diff options
| author | Alex Vakulenko <avakulenko@google.com> | 2016-04-01 09:49:22 -0700 |
|---|---|---|
| committer | Alex Vakulenko <avakulenko@google.com> | 2016-04-01 09:51:09 -0700 |
| commit | 9fd3adb463ad97dfb41f2265e8836af32a37d2c2 (patch) | |
| tree | 5329c96cd2853d914b3f37a059a359eb3bba525c | |
| parent | d5cf72a7a1de0e785c295b63dd2446b34399a935 (diff) | |
| parent | dcc0dca657396fdc57a1f4d7ee1fca66f059cb1e (diff) | |
| download | libweave-9fd3adb463ad97dfb41f2265e8836af32a37d2c2.tar.gz | |
Merge remote-tracking branch 'weave/master' into 'weave/aosp-master'
This merge includes the following commits:
dcc0dca libweave: Convert the blacklist trait into a standard one
5fe0ac0 examples: refactor README file
3127cb3 Tweak the README.md with respect to the repo subdirectories.
38a2aef libweave: Remove release() calls on scoped_ptr
bf79a9e libweave: Port base/ changes from Chromium project
b9bbdc6 libweave: Fix compile errors on Chrome OS
5c2870f libevhtp: build checked out copy
3e4d883 add integrated cross-compile & qemu support
7fb7903 examples/daemon/ledflasher: switch to onOff
0011064 readme: add more tips/links
d70a965 gtest/gmock: build checked out copy
8bf4757 Fail setup/start if device already registered
cd7a3a2 Add HTTPS port into TXT record
5ddd991 Return 'component' in command JSON
00180aa Avoid Revocation list overflow
b506696 Bound j2000 timestamps into [0, <int32_t>::max()] interval
e733c38 Take into account scope from /privet/v3/auth call
8b897af examples/daemon: fix ledflasher param names
978e712 Implement local_discovery_enabled setting
e4b8ccf Remove ObserverList from cloud delegate
1c6837f Remove CloudDelegate::Observer::OnDeviceInfoChanged
63feef5 Fixed issues configuring devices with date before 2000.
86e8f63 Update READMEs to reflect cloud_id name in Cloud Services for deviceId Added git add <files> in README for Making Changes
80c65d2 VERISON: Initial commit on master - 1.4.1
98af48b Update macaroon lib
fafbc5d Insert the description of the change.
5e94dc8 Applied clang-format
b741d64 Fix crash when device in access point mode
0d3062e Implement minimalRole for state definitions
c7fab18 Update _accessRevocationList trait
b18bead Fixes to event_http_server for examples
a07bbc7 Add provider::Wifi::GetConnectedSsid
c96ee4e examples/lock: fix lock trait
f7bfb6a examples/speaker: fix volume trait
2419a2a Update local auth info if server side information does not match
b7e0996 Simplify few comparison helpers
497559b Moved previous comment to README file
Change-Id: Ibd3ca55fb07b816b60489e458f5e905d260c683f
115 files changed, 2330 insertions, 2558 deletions
@@ -1,7 +1,6 @@ *~ /out/ -/third_party/include -/third_party/lib +/third_party/cross /third_party/libevhtp /third_party/googletest gomacc.lock diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 0bb7f26..66a9132 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -16,6 +16,7 @@ Bertrand Simonnet <bsimonnet@google.com> Chris Sosa <sosa@google.com> Christopher Wiley <wiley@google.com> Daniel Erat <derat@google.com> +David Paul <davepaul@google.com> David Zeuthen <zeuthen@google.com> Gene Gutnik <gene@google.com> Gerry Fan <gfan@google.com> @@ -15,7 +15,8 @@ INCLUDES := \ -I. \ -Iinclude \ -Ithird_party/chromium \ - -Ithird_party/include \ + -Ithird_party/googletest/googletest/include \ + -Ithird_party/googletest/googlemock/include \ -Ithird_party/libuweave \ -Ithird_party/modp_b64/modp_b64 @@ -75,7 +76,7 @@ DEFS_TEST := \ out/$(BUILD_MODE)/libweave.so : out/$(BUILD_MODE)/libweave_common.a $(CXX) -shared -Wl,-soname=libweave.so -o $@ -Wl,--whole-archive $^ -Wl,--no-whole-archive -lcrypto -lexpat -lpthread -lrt -include file_lists.mk third_party/third_party.mk examples/examples.mk tests.mk +include cross.mk file_lists.mk third_party/third_party.mk examples/examples.mk tests.mk ### # src/ @@ -90,7 +91,10 @@ out/$(BUILD_MODE)/libweave_common.a : $(weave_obj_files) $(third_party_chromium_ rm -f $@ $(AR) crsT $@ $^ -all : out/$(BUILD_MODE)/libweave.so all-examples out/$(BUILD_MODE)/libweave_exports_testrunner out/$(BUILD_MODE)/libweave_testrunner +all-libs : out/$(BUILD_MODE)/libweave.so +all-tests : out/$(BUILD_MODE)/libweave_exports_testrunner out/$(BUILD_MODE)/libweave_testrunner + +all : all-libs all-examples all-tests clean : rm -rf out @@ -18,7 +18,8 @@ mkdir ~/bin PATH=~/bin:$PATH ``` -Download the Repo tool and ensure that it is executable: +Download the [Repo tool](https://gerrit.googlesource.com/git-repo) and ensure +that it is executable: ``` curl https://storage.googleapis.com/git-repo-downloads/repo > ~/bin/repo @@ -28,11 +29,15 @@ chmod a+x ~/bin/repo # Checkout code ``` +mkdir ~/weave +cd ~/weave repo init -u https://weave.googlesource.com/weave/manifest repo sync ``` -# Directory structure +This checks out libweave and its dependencies into the ~/weave directory. + +# libweave Directory structure | Path | Description | |--------------------------|------------------------------------| @@ -78,8 +83,8 @@ sudo apt-get install \ ### For tests - cmake - - gtest (included; see third_party/get_gtest.sh) - - gmock (included; see third_party/get_gtest.sh) + - gtest (included; see third_party/googletest/googletest/) + - gmock (included; see third_party/googletest/googlemock/) ### For examples @@ -87,12 +92,14 @@ sudo apt-get install \ - hostapd - libavahi-client-dev - libcurl4-openssl-dev - - libevhtp (included; see third_party/get_libevhtp.sh) + - libevhtp (included; see third_party/libevhtp/) - libevent-dev # Compiling +From the `libweave` directory: + The `make --jobs/-j` flag is encouraged, to speed up build time. For example ``` @@ -103,7 +110,7 @@ which happens to be the same as ``` make all -j -```` +``` ### Build library @@ -119,6 +126,24 @@ make all-examples See [the examples README](/examples/daemon/README.md) for details. +### Cross-compiling + +The build supports transparently downloading & using a few cross-compilers. +Just add `cross-<arch>` to the command line in addition to the target you +want to actually build. + +This will cross-compile for an armv7 (hard float) target: + +``` +make cross-arm all-libs +``` + +This will cross-compile for a mips (little endian) target: + +``` +make cross-mipsel all-libs +``` + # Testing ### Run tests @@ -134,8 +159,22 @@ or make testall ``` +### Cross-testing + +The build supports using qemu to run non-native tests. + +This will run armv7 tests through qemu: + +``` +make cross-arm testall +``` + # Making changes +The [Android Developing site](https://source.android.com/source/developing.html) +has a lot of good tips for working with git and repo in general. The tips below +are meant as a quick cheat sheet rather than diving deep into relevant topics. + ### Configure git Make sure to have correct user in local or global config e.g.: @@ -164,4 +203,4 @@ repo upload . ### Request code review -Go to the url from the output of "repo upload" and add reviewers. +Go to the url from the output of `repo upload` and add reviewers. @@ -0,0 +1 @@ +1.4.0 diff --git a/cross.mk b/cross.mk new file mode 100644 index 0000000..e833ca9 --- /dev/null +++ b/cross.mk @@ -0,0 +1,40 @@ +# Copyright 2016 The Weave Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# Logic to easily run cross-compiling tests. + +CROSS_ROOT := $(PWD)/third_party/cross +CROSS_FLAGS := +DOWNLOAD_CROSS_TOOLCHAINS := no +QEMU_BASE := $(CROSS_ROOT)/app-emulation/qemu/usr/bin + +define cross-setup-gcc +.PHONY: $(1) +ifneq (,$$(findstring $(1),$$(MAKECMDGOALS))) +DOWNLOAD_CROSS_TOOLCHAINS := yes +CHOST := $(2) +BOARD := $(3) +CROSS := $$(CROSS_ROOT)/$$(CHOST)/bin/$$(CHOST)- +CC := $$(CROSS)gcc +CXX := $$(CROSS)g++ +AR := $$(CROSS)ar +CROSS_FLAGS += $(5) +QEMU := $$(QEMU_BASE)/$(4) -L $$(CROSS_ROOT)/$$(BOARD) +endif +endef + +# Whitespace matters with arguments, so we can't make this more readable :/. +$(eval $(call cross-setup-gcc,cross-arm,armv7a-cros-linux-gnueabi,arm-generic-full,qemu-arm,-mhard-float)) +$(eval $(call cross-setup-gcc,cross-mipsel,mipsel-cros-linux-gnu,mipsel-o32-generic-full,qemu-mipsel)) +$(eval $(call cross-setup-gcc,cross-x86,i686-pc-linux-gnu,x86-generic-full,qemu-i386)) +$(eval $(call cross-setup-gcc,cross-x86_64,x86_64-cros-linux-gnu,amd64-generic-full,qemu-x86_64)) + +ifeq ($(DOWNLOAD_CROSS_TOOLCHAINS),yes) +ifeq (,$(wildcard third_party/cross/$(BOARD))) +CROSS_FETCH_OUT := $(shell ./third_party/get_cross.sh >&2) +endif +CROSS_FLAGS += --sysroot $(CROSS_ROOT)/$(BOARD) +CC += $(CROSS_FLAGS) +CXX += $(CROSS_FLAGS) +endif diff --git a/examples/daemon/README.md b/examples/daemon/README.md index 4585b5b..b6a7664 100644 --- a/examples/daemon/README.md +++ b/examples/daemon/README.md @@ -1,122 +1,93 @@ -# Overview +# libweave daemon examples -The wrapper implements OS dependent services for libweave +This directory contains examples implementation of `libweave` device daemons. -# Building +## Build -### Build daemon examples - -The example binaries land in the out/Debug/ directory build all of them at once: +- build all examples ``` make all-examples ``` -...or one at a time. +- build only the light daemon ``` make out/Debug/weave_daemon_light ``` -# Prepare Host OS +## Pre-requisites + +- enable user-service-publishing in avahi daemon + +set `disable-user-service-publishing=no` in `/etc/avahi/avahi-daemon.conf` -### Enable user-service-publishing in avahi daemon -Set disable-user-service-publishing=no in /etc/avahi/avahi-daemon.conf +- restart avahi daemon -#### restart avahi ``` sudo service avahi-daemon restart ``` -# Control device with the cloud +## Provisioning -### Generate registration ticket -- Go to [OAuth 2.0 Playground](https://developers.google.com/oauthplayground/) -- "Step 1": Paste https://www.googleapis.com/auth/clouddevices and click to "Authorize APIs" -- "Step 2": Click "Exchange authorization code for tokens" -- "Step 3": Fill the form: - * HTTP Method: POST - * Request URI: https://www.googleapis.com/weave/v1/registrationTickets - * Enter request body: ```{"userEmail": "me"}``` - * Click "Send the request", a ticket id will be returned in +### Generate registration tickets +- go to the [OAuth 2.0 Playground](https://developers.google.com/oauthplayground/) + - `Step 1`: enter the Weave API scope `https://www.googleapis.com/auth/weave.app` and click to `Authorize APIs` + - `Step 2`: click `Exchange authorization code for tokens` + - `Step 3`: + - set `HTTP Method`: `POST` + - set `Request URI`: `https://www.googleapis.com/weave/v1/registrationTickets` + - click `Enter request body`: `{"userEmail": "me"}` + - click `Send the request` + - The `Response` contains a new `registrationTicket` resource. ``` - { - "userEmail": "user@google.com", - "kind": "weave#registrationTicket", - "expirationTimeMs": "1443204934855", - "deviceId": "0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d", - "creationTimeMs": "1443204694855", - "id": "93019287-6b26-04a0-22ee-d55ad23a4226" - } +{ + "userEmail": "user@google.com", + "kind": "weave#registrationTicket", + "expirationTimeMs": "1443204934855", + "deviceId": "0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d", + "creationTimeMs": "1443204694855", + "id": "93019287-6b26-04a0-22ee-d55ad23a4226" +} ``` -- Note: the ticket "id" is not used within 240 sec, it will be expired. +- Note: the ticket expires after a few minutes -### Register device to cloud - -- Copy the ticket "id" generated above: ```93019287-6b26-04a0-22ee-d55ad23a4226``` -- Go to terminal, register and start the daemon with - -``` - sudo out/Debug/weave_daemon_sample --registration_ticket=93019287-6b26-04a0-22ee-d55ad23a4226 -``` +### Provision the device -- See something like: +- start the daemon with the `registrationTicket` id. ``` - Publishing service - Saving settings to /var/lib/weave/weave_settings.json +sudo out/Debug/weave_daemon_sample --registration_ticket=93019287-6b26-04a0-22ee-d55ad23a4226 ``` -- Note: in second and future runs, --registration_ticket options is not necessary anymore -- Get your device id with +- the daemon outputs the path to its configuration file and its deviceId ``` - sudo cat /var/lib/weave/weave_settings.json +Saving settings to /var/lib/weave/weave_settings_XXXXX_config.json +Device registered: 0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d ``` -- See something like: +- the device id matches the `cloud_id` in the configuration ``` - ... - "device_id": 0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d - ... +$ sudo grep "cloud_id" /var/lib/weave/weave_settings_[XXXXX]_config.json + "cloud_id": 0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d ``` -- Use this device_id for future communication with your device. It does not expire. -- Verify device is up with Weave Device Managers on -[Android](https://play.google.com/apps/testing/com.google.android.apps.weave.management), -[Chrome](https://chrome.google.com/webstore/detail/weave-device-manager/pcdgflbjckpjmlofgopidgdfonmnodfm) -or [Weave Developpers Console](https://weave.google.com/console/) - -### Send Command to the Daemon - -- Go to [OAuth 2.0 Playground](https://developers.google.com/oauthplayground/) -- "Step 1": Paste https://www.googleapis.com/auth/clouddevices and click to "Authorize APIs" -- "Step 2": Click "Exchange authorization code for tokens" -- "Step 3": Fill the form: - * HTTP Method: POST - * Request URI: https://www.googleapis.com/weave/v1/commands - * Enter request body: +- verify that that the device is online in the + [Weave Developers Console](https://weave.google.com/console/), + the [Weave Android app](https://play.google.com/apps/testing/com.google.android.apps.weave.management) + or + the [Weave Chrome app](https://chrome.google.com/webstore/detail/weave-device-manager/pcdgflbjckpjmlofgopidgdfonmnodfm). -``` - { - "deviceId": "0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d", - "name": "_sample.hello", - "component": "sample", - "parameters": { "name": "cloud user" } - } - -``` +### Send Commands -- "Send the request", you command will be "queued" as its "state" -- Verify the command execution observing daemon console logs -- Verify the command usign [Weave Developpers Console](https://weave.google.com/console/) -- Verify the command history with [OAuth 2.0 Playground](https://developers.google.com/oauthplayground/) -- "Step 1": Paste https://www.googleapis.com/auth/clouddevices and click to "Authorize APIs" -- "Step 2": Click "Exchange authorization code for tokens" -- "Step 3": Fill the form: - * HTTP Method: GET - * Request URI: https://www.googleapis.com/weave/v1/commands?deviceId=0f8a5ff5-1ef0-ec39-f9d8-66d1caeb9e3d - * Click "Send the request", you get all of the commands executed on your device. +- go to the [Weave Developers Console](https://weave.google.com/console/), +- click `Your devices` +- select your device to show the `Device details` page. +- in the `Available commands` click `_sample.hello` +- set command parameters +- click `RUN COMMAND` +- verify the command is handled correctly by looking at the daemon logs. diff --git a/examples/daemon/common/daemon.h b/examples/daemon/common/daemon.h index 22591a2..23a08d2 100644 --- a/examples/daemon/common/daemon.h +++ b/examples/daemon/common/daemon.h @@ -114,10 +114,8 @@ class Daemon { private: static void OnRegisterDeviceDone(weave::Device* device, weave::ErrorPtr error) { - if (error) - LOG(ERROR) << "Fail to register device: " << error->GetMessage(); - else - LOG(INFO) << "Device registered: " << device->GetSettings().cloud_id; + CHECK(!error) << "Registration failed device: " << error->GetMessage(); + LOG(INFO) << "Device registered: " << device->GetSettings().cloud_id; } std::unique_ptr<weave::examples::EventTaskRunner> task_runner_; diff --git a/examples/daemon/ledflasher/ledflasher.cc b/examples/daemon/ledflasher/ledflasher.cc index 369b003..88403a2 100644 --- a/examples/daemon/ledflasher/ledflasher.cc +++ b/examples/daemon/ledflasher/ledflasher.cc @@ -12,45 +12,32 @@ #include <bitset> namespace { -// Supported LED count on this device const size_t kLedCount = 3; const char kTraits[] = R"({ - "_ledflasher": { + "onOff": { "commands": { - "set": { + "setConfig": { "minimalRole": "user", "parameters": { - "led": { - "type": "integer", - "minimum": 1, - "maximum": 3 - }, - "on": { "type": "boolean" } - } - }, - "toggle": { - "minimalRole": "user", - "parameters": { - "led": { - "type": "integer", - "minimum": 1, - "maximum": 3 + "state": { + "type": "string", + "enum": [ "on", "off" ] } } } }, "state": { - "leds": { - "type": "array", - "items": { "type": "boolean" } + "state": { + "isRequired": true, + "type": "string", + "enum": [ "on", "off" ] } } } })"; -const char kComponent[] = "ledflasher"; - +const char kLedComponentPrefix[] = "led"; } // namespace // LedFlasherHandler is a complete command handler example that shows @@ -62,41 +49,38 @@ class LedFlasherHandler { device_ = device; device->AddTraitDefinitionsFromJson(kTraits); - CHECK(device->AddComponent(kComponent, {"_ledflasher"}, nullptr)); - UpdateLedState(); - - device->AddCommandHandler( - kComponent, "_ledflasher.toggle", - base::Bind(&LedFlasherHandler::OnFlasherToggleCommand, - weak_ptr_factory_.GetWeakPtr())); - device->AddCommandHandler( - kComponent, "_ledflasher.set", - base::Bind(&LedFlasherHandler::OnFlasherSetCommand, - weak_ptr_factory_.GetWeakPtr())); + for (size_t led_index = 0; led_index < led_states_.size(); led_index++) { + std::string component_name = + kLedComponentPrefix + std::to_string(led_index + 1); + CHECK(device->AddComponent(component_name, {"onOff"}, nullptr)); + device->AddCommandHandler( + component_name, "onOff.setConfig", + base::Bind(&LedFlasherHandler::OnOnOffSetConfig, + weak_ptr_factory_.GetWeakPtr(), led_index)); + device->SetStateProperty( + component_name, "onOff.state", + base::StringValue{led_states_[led_index] ? "on" : "off"}, nullptr); + } } private: - void OnFlasherSetCommand(const std::weak_ptr<weave::Command>& command) { + void OnOnOffSetConfig(size_t led_index, + const std::weak_ptr<weave::Command>& command) { auto cmd = command.lock(); if (!cmd) return; LOG(INFO) << "received command: " << cmd->GetName(); - int32_t led_index = 0; const auto& params = cmd->GetParameters(); - bool cmd_value = false; - if (params.GetInteger("_led", &led_index) && - params.GetBoolean("_on", &cmd_value)) { - // Display this command in terminal - LOG(INFO) << cmd->GetName() << " _led: " << led_index - << ", _on: " << (cmd_value ? "true" : "false"); - - led_index--; - int new_state = cmd_value ? 1 : 0; - int cur_state = led_status_[led_index]; - led_status_[led_index] = new_state; - - if (cmd_value != cur_state) { - UpdateLedState(); + std::string state; + if (params.GetString("state", &state)) { + LOG(INFO) << cmd->GetName() << " led: " << led_index + << " state: " << state; + int current_state = led_states_[led_index]; + int new_state = (state == "on") ? 1 : 0; + led_states_[led_index] = new_state; + if (new_state != current_state) { + device_->SetStateProperty(cmd->GetComponent(), "onOff.State", + base::StringValue{state}, nullptr); } cmd->Complete({}, nullptr); return; @@ -107,41 +91,9 @@ class LedFlasherHandler { cmd->Abort(error.get(), nullptr); } - void OnFlasherToggleCommand(const std::weak_ptr<weave::Command>& command) { - auto cmd = command.lock(); - if (!cmd) - return; - LOG(INFO) << "received command: " << cmd->GetName(); - const auto& params = cmd->GetParameters(); - int32_t led_index = 0; - if (params.GetInteger("_led", &led_index)) { - LOG(INFO) << cmd->GetName() << " _led: " << led_index; - led_index--; - led_status_[led_index] = ~led_status_[led_index]; - - UpdateLedState(); - cmd->Complete({}, nullptr); - return; - } - weave::ErrorPtr error; - weave::Error::AddTo(&error, FROM_HERE, "invalid_parameter_value", - "Invalid parameters"); - cmd->Abort(error.get(), nullptr); - } - - void UpdateLedState() { - base::ListValue list; - for (uint32_t i = 0; i < led_status_.size(); i++) - list.AppendBoolean(led_status_[i] ? true : false); - - device_->SetStateProperty(kComponent, "_ledflasher.leds", list, nullptr); - } - weave::Device* device_{nullptr}; - // Simulate LED status on this device so client app could explore - // Each bit represents one device, indexing from LSB - std::bitset<kLedCount> led_status_{0}; + std::bitset<kLedCount> led_states_{0}; base::WeakPtrFactory<LedFlasherHandler> weak_ptr_factory_{this}; }; diff --git a/examples/daemon/light/light.cc b/examples/daemon/light/light.cc index 07c0231..66167b4 100644 --- a/examples/daemon/light/light.cc +++ b/examples/daemon/light/light.cc @@ -288,7 +288,7 @@ class LightHandler { std::unique_ptr<base::DictionaryValue> colorXy(new base::DictionaryValue()); colorXy->SetDouble("colorX", color_X_); colorXy->SetDouble("colorY", color_Y_); - state.Set("colorXy.colorSetting", colorXy.release()); + state.Set("colorXy.colorSetting", std::move(colorXy)); device_->SetStateProperties(kComponent, state, nullptr); } diff --git a/examples/daemon/lock/lock.cc b/examples/daemon/lock/lock.cc index a2da9cf..32ce12d 100644 --- a/examples/daemon/lock/lock.cc +++ b/examples/daemon/lock/lock.cc @@ -38,7 +38,7 @@ const char kTraits[] = R"({ "enum": [ "locked", "unlocked" ] } }, - "errors": ["batteryTooLow", "jammed", "lockingNotSupported"] + "errors": [ "jammed", "lockingNotSupported" ] } }, "state": { diff --git a/examples/daemon/oven/oven.cc b/examples/daemon/oven/oven.cc index daf1971..f92c838 100644 --- a/examples/daemon/oven/oven.cc +++ b/examples/daemon/oven/oven.cc @@ -227,11 +227,13 @@ class OvenHandler { state.SetString("temperatureSensor.units", units_); state.SetDouble("temperatureSensor.value", current_temperature_); - state.Set("temperatureSensor.supportedUnits", supportedUnits.DeepCopy()); + state.Set("temperatureSensor.supportedUnits", + supportedUnits.CreateDeepCopy()); state.SetString("temperatureSetting.units", units_); state.SetDouble("temperatureSetting.tempSetting", target_temperature_); - state.Set("temperatureSetting.supportedUnits", supportedUnits.DeepCopy()); + state.Set("temperatureSetting.supportedUnits", + supportedUnits.CreateDeepCopy()); state.SetDouble("temperatureSetting.maxTempSetting", kMaxTemp); state.SetDouble("temperatureSetting.minTempSetting", kMinTemp); diff --git a/examples/daemon/speaker/speaker.cc b/examples/daemon/speaker/speaker.cc index 35eee2f..56da840 100644 --- a/examples/daemon/speaker/speaker.cc +++ b/examples/daemon/speaker/speaker.cc @@ -19,7 +19,7 @@ const char kTraits[] = R"({ "parameters": { "state": { "type": "string", - "enum": [ "on", "standby" ] + "enum": [ "on", "off" ] } } } @@ -27,7 +27,7 @@ const char kTraits[] = R"({ "state": { "state": { "type": "string", - "enum": [ "on", "standby" ], + "enum": [ "on", "off" ], "isRequired": true } } @@ -149,7 +149,7 @@ class SpeakerHandler { void UpdateSpeakerState() { base::DictionaryValue state; - state.SetString("onOff.state", speaker_status_ ? "on" : "standby"); + state.SetString("onOff.state", speaker_status_ ? "on" : "off"); state.SetBoolean("volume.isMuted", isMuted_status_); state.SetInteger("volume.volume", volume_value_); device_->SetStateProperties(kComponent, state, nullptr); diff --git a/examples/examples.mk b/examples/examples.mk index af15d5c..48a1210 100644 --- a/examples/examples.mk +++ b/examples/examples.mk @@ -10,9 +10,15 @@ examples_provider_obj_files := $(EXAMPLES_PROVIDER_SRC_FILES:%.cc=out/$(BUILD_MO USE_INTERNAL_LIBEVHTP ?= 1 ifeq (1, $(USE_INTERNAL_LIBEVHTP)) -$(examples_provider_obj_files) : third_party/include/evhtp.h +LIBEVHTP_INCLUDES = -Ithird_party/libevhtp -I$(dir $(third_party_libevhtp_header)) +LIBEVHTP_HEADERS = $(third_party_libevhtp_header) +else +LIBEVHTP_INCLUDES = +LIBEVHTP_HEADERS = endif +$(examples_provider_obj_files) : $(LIBEVHTP_HEADERS) +$(examples_provider_obj_files) : INCLUDES += $(LIBEVHTP_INCLUDES) $(examples_provider_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_$(BUILD_MODE)) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< @@ -31,17 +37,14 @@ EXAMPLES_DAEMON_SRC_FILES := \ examples_daemon_obj_files := $(EXAMPLES_DAEMON_SRC_FILES:%.cc=out/$(BUILD_MODE)/%.o) -ifeq (1, $(USE_INTERNAL_LIBEVHTP)) -$(examples_daemon_obj_files) : third_party/include/evhtp.h -endif - +$(examples_daemon_obj_files) : $(LIBEVHTP_HEADERS) +$(examples_daemon_obj_files) : INCLUDES += $(LIBEVHTP_INCLUDES) $(examples_daemon_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_$(BUILD_MODE)) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< daemon_common_flags := \ -Wl,-rpath=out/$(BUILD_MODE)/ \ - -Lthird_party/lib \ -levent \ -levent_openssl \ -lpthread \ @@ -55,7 +58,7 @@ daemon_common_flags := \ daemon_deps := out/$(BUILD_MODE)/examples_provider.a out/$(BUILD_MODE)/libweave.so ifeq (1, $(USE_INTERNAL_LIBEVHTP)) -daemon_deps += third_party/lib/libevhtp.a +daemon_deps += $(third_party_libevhtp_lib) else daemon_common_flags += -levhtp endif diff --git a/examples/provider/README.md b/examples/provider/README.md new file mode 100644 index 0000000..607fad4 --- /dev/null +++ b/examples/provider/README.md @@ -0,0 +1,34 @@ +# libweave provider examples + +This directory contains example implementations of `weave` system providers. + +## Providers +- `avahi_client.cc` + - implements: `weave::providerDnsServiceDiscovery` + - build-depends: libavahi-client + - run-depends: `avahi-daemon` +- `bluez_client.cc` + - not-implemented +- `curl_http_client.cc` + - implements: `weave::provider::HttpClient` + - build-depends: libcurl +- `event_http_server.cc` + - implements: `weave::provider::HttpServer` + - build-depends: libevhtp +- `event_network.cc` + - implements: `weave::provider::Network` + - build-depends: libevent +- `event_task_runner.cc` + - implements: `weave::provider::TaskRunner` + - build-depends: libevent +- `file_config_store.cc` + - implements: `weave::provider::ConfigStore` +- `wifi_manager.cc` + - implements: `weave::provider::Wifi` + - build-depends: `weave::examples::EventNetworkImpl` + - run-depends: `network-manager`, `dnsmasq`, `hostapd` + +Note: +- The example providers are based on `libevent` and should be portable between most GNU/Linux distributions. +- `weave::examples::WifiImpl` currently shells out to system + command tools like `nmcli`, `dnsmasq`, `ifconfig` and `hostpad`. diff --git a/examples/provider/avahi_client.cc b/examples/provider/avahi_client.cc index 27fae10..ddd4630 100644 --- a/examples/provider/avahi_client.cc +++ b/examples/provider/avahi_client.cc @@ -75,10 +75,10 @@ void AvahiClient::PublishService(const std::string& service_type, service_type.c_str(), nullptr, txt_list.get()); CHECK_GE(ret, 0) << avahi_strerror(ret); } else { + avahi_entry_group_reset(group_.get()); prev_port_ = port; prev_type_ = service_type; - avahi_entry_group_reset(group_.get()); CHECK(avahi_entry_group_is_empty(group_.get())); ret = avahi_entry_group_add_service_strlst( @@ -91,6 +91,9 @@ void AvahiClient::PublishService(const std::string& service_type, } void AvahiClient::StopPublishing(const std::string& service_name) { + prev_port_ = 0; + prev_type_.clear(); + CHECK(group_); avahi_entry_group_reset(group_.get()); } diff --git a/examples/provider/event_http_server.cc b/examples/provider/event_http_server.cc index c058401..3d64d15 100644 --- a/examples/provider/event_http_server.cc +++ b/examples/provider/event_http_server.cc @@ -30,8 +30,7 @@ std::string GetSslError() { class HttpServerImpl::RequestImpl : public Request { public: RequestImpl(EventPtr<evhtp_request_t> req) : req_(std::move(req)) { - evbuf_t* input_buffer = - bufferevent_get_input(evhtp_request_get_bev(req_.get())); + evbuf_t* input_buffer = req_.get()->buffer_in; data_.resize(evbuffer_get_length(input_buffer)); evbuffer_remove(input_buffer, &data_[0], data_.size()); } @@ -56,6 +55,9 @@ class HttpServerImpl::RequestImpl : public Request { evbuffer_add(buf.get(), data.data(), data.size()); evhtp_header_key_add(req_->headers_out, "Content-Type", 0); evhtp_header_val_add(req_->headers_out, mime_type.c_str(), 1); + evhtp_header_key_add(req_->headers_out, "Content-Length", 0); + std::string content_length = std::to_string(evbuffer_get_length(buf.get())); + evhtp_header_val_add(req_->headers_out, content_length.c_str(), 1); evhtp_send_reply_start(req_.get(), status_code); evhtp_send_reply_body(req_.get(), buf.get()); evhtp_send_reply_end(req_.get()); @@ -141,7 +143,7 @@ void HttpServerImpl::NotFound(evhtp_request_t* req) { void HttpServerImpl::ProcessRequest(evhtp_request_t* req) { std::unique_ptr<RequestImpl> request{new RequestImpl{EventPtr<evhtp_request_t>{req}}}; std::string path = request->GetPath(); - auto it = handlers_.find(path); + auto it = handlers_.find(std::make_pair(path, req->htp)); if (it != handlers_.end()) { return it->second.Run(std::move(request)); } @@ -155,17 +157,27 @@ void HttpServerImpl::ProcessRequestCallback(evhtp_request_t* req, void* arg) { void HttpServerImpl::AddHttpRequestHandler( const std::string& path, const RequestHandlerCallback& callback) { - handlers_.insert(std::make_pair(path, callback)); + handlers_[std::make_pair(path, httpd_.get())] = callback; evhtp_set_cb(httpd_.get(), path.c_str(), &ProcessRequestCallback, this); } void HttpServerImpl::AddHttpsRequestHandler( const std::string& path, const RequestHandlerCallback& callback) { - handlers_.insert(std::make_pair(path, callback)); + handlers_[std::make_pair(path, httpsd_.get())] = callback; evhtp_set_cb(httpsd_.get(), path.c_str(), &ProcessRequestCallback, this); } +void HttpServerImpl::RemoveHttpRequestHandler(const std::string& path) { + handlers_.erase(std::make_pair(path, httpd_.get())); + evhtp_set_cb(httpd_.get(), path.c_str(), nullptr, nullptr); +} + +void HttpServerImpl::RemoveHttpsRequestHandler(const std::string& path) { + handlers_.erase(std::make_pair(path, httpsd_.get())); + evhtp_set_cb(httpsd_.get(), path.c_str(), nullptr, nullptr); +} + void HttpServerImpl::ProcessReply(std::shared_ptr<RequestImpl> request, int status_code, const std::string& data, diff --git a/examples/provider/event_http_server.h b/examples/provider/event_http_server.h index 8bb5dd9..bf8e6d4 100644 --- a/examples/provider/event_http_server.h +++ b/examples/provider/event_http_server.h @@ -33,6 +33,8 @@ class HttpServerImpl : public provider::HttpServer { const RequestHandlerCallback& callback) override; void AddHttpsRequestHandler(const std::string& path_prefix, const RequestHandlerCallback& callback) override; + void RemoveHttpRequestHandler(const std::string& path) override; + void RemoveHttpsRequestHandler(const std::string& path) override; uint16_t GetHttpPort() const override; uint16_t GetHttpsPort() const override; base::TimeDelta GetRequestTimeout() const override; @@ -48,7 +50,8 @@ class HttpServerImpl : public provider::HttpServer { const std::string& mime_type); void NotFound(evhtp_request_t* req); - std::map<std::string, RequestHandlerCallback> handlers_; + std::map<std::pair<std::string, const evhtp_t*>, RequestHandlerCallback> + handlers_; std::vector<uint8_t> cert_fingerprint_; EventTaskRunner* task_runner_{nullptr}; diff --git a/examples/provider/wifi_manager.cc b/examples/provider/wifi_manager.cc index 9337d23..27bbfd6 100644 --- a/examples/provider/wifi_manager.cc +++ b/examples/provider/wifi_manager.cc @@ -64,11 +64,53 @@ std::string FindWirelessInterface() { return ""; } +std::string GetSsid(const std::string& interface) { + int sockf_d = socket(AF_INET, SOCK_DGRAM, 0); + CHECK_GE(sockf_d, 0) << strerror(errno); + iwreq wreq = {}; + CHECK_LE(interface.size(), sizeof(wreq.ifr_name)); + strncpy(wreq.ifr_name, interface.c_str(), sizeof(wreq.ifr_name)); + std::string essid(' ', IW_ESSID_MAX_SIZE + 1); + wreq.u.essid.pointer = &essid[0]; + wreq.u.essid.length = essid.size(); + if (ioctl(sockf_d, SIOCGIWESSID, &wreq) >= 0) + essid.resize(wreq.u.essid.length); + else + essid.clear(); + close(sockf_d); + return essid; +} + +bool CheckFreq(const std::string& interface, double start, double end) { + int sockf_d = socket(AF_INET, SOCK_DGRAM, 0); + CHECK_GE(sockf_d, 0) << strerror(errno); + iwreq wreq = {}; + CHECK_LE(interface.size(), sizeof(wreq.ifr_name)); + strncpy(wreq.ifr_name, interface.c_str(), sizeof(wreq.ifr_name)); + + iw_range range = {}; + wreq.u.data.pointer = ⦥ + wreq.u.data.length = sizeof(range); + + bool result = false; + if (ioctl(sockf_d, SIOCGIWRANGE, &wreq) >= 0) { + for (size_t i = 0; !result && i < range.num_frequency; ++i) { + double freq = range.freq[i].m * std::pow(10., range.freq[i].e); + if (start <= freq && freq <= end) + result = true; + } + } + + close(sockf_d); + return result; +} + } // namespace WifiImpl::WifiImpl(provider::TaskRunner* task_runner, EventNetworkImpl* network) : task_runner_{task_runner}, network_{network}, iface_{FindWirelessInterface()} { CHECK(!iface_.empty()) << "WiFi interface not found"; + CHECK(IsWifi24Supported() || IsWifi50Supported()); CHECK_EQ(0u, getuid()) << "\nWiFi manager expects root access to control WiFi capabilities"; StopAccessPoint(); @@ -85,19 +127,7 @@ void WifiImpl::TryToConnect(const std::string& ssid, if (pid) { int status = 0; if (pid == waitpid(pid, &status, WNOWAIT)) { - int sockf_d = socket(AF_INET, SOCK_DGRAM, 0); - CHECK_GE(sockf_d, 0) << strerror(errno); - - iwreq wreq = {}; - strncpy(wreq.ifr_name, iface_.c_str(), sizeof(wreq.ifr_name)); - std::string essid(' ', IW_ESSID_MAX_SIZE + 1); - wreq.u.essid.pointer = &essid[0]; - wreq.u.essid.length = essid.size(); - CHECK_GE(ioctl(sockf_d, SIOCGIWESSID, &wreq), 0) << strerror(errno); - essid.resize(wreq.u.essid.length); - close(sockf_d); - - if (ssid == essid) + if (ssid == GetSsid(iface_)) return task_runner_->PostDelayedTask(FROM_HERE, base::Bind(callback, nullptr), {}); pid = 0; // Try again. @@ -129,8 +159,7 @@ void WifiImpl::Connect(const std::string& ssid, const std::string& passphrase, const DoneCallback& callback) { network_->SetSimulateOffline(false); - CHECK(!hostapd_started_); - if (hostapd_started_) { + if (!hostapd_ssid_.empty()) { ErrorPtr error; Error::AddTo(&error, FROM_HERE, "busy", "Running Access Point."); task_runner_->PostDelayedTask( @@ -143,8 +172,7 @@ void WifiImpl::Connect(const std::string& ssid, } void WifiImpl::StartAccessPoint(const std::string& ssid) { - if (hostapd_started_) - return; + CHECK(hostapd_ssid_.empty()); // Release wifi interface. CHECK_EQ(0, ForkCmdAndWait("nmcli", {"nm", "wifi", "off"})); @@ -160,7 +188,7 @@ void WifiImpl::StartAccessPoint(const std::string& ssid) { } CHECK_EQ(0, ForkCmdAndWait("hostapd", {"-B", "-K", hostapd_conf})); - hostapd_started_ = true; + hostapd_ssid_ = ssid; for (size_t i = 0; i < 10; ++i) { if (0 == ForkCmdAndWait("ifconfig", {iface_, "192.168.76.1/24"})) @@ -186,12 +214,24 @@ void WifiImpl::StopAccessPoint() { base::IgnoreResult(ForkCmdAndWait("pkill", {"-f", "dnsmasq.*/tmp/weave"})); base::IgnoreResult(ForkCmdAndWait("pkill", {"-f", "hostapd.*/tmp/weave"})); CHECK_EQ(0, ForkCmdAndWait("nmcli", {"nm", "wifi", "on"})); - hostapd_started_ = false; + hostapd_ssid_.clear(); } bool WifiImpl::HasWifiCapability() { return !FindWirelessInterface().empty(); } +bool WifiImpl::IsWifi24Supported() const { + return CheckFreq(iface_, 2.4e9, 2.5e9); +}; + +bool WifiImpl::IsWifi50Supported() const { + return CheckFreq(iface_, 4.9e9, 5.9e9); +}; + +std::string WifiImpl::GetConnectedSsid() const { + return GetSsid(iface_); +} + } // namespace examples } // namespace weave diff --git a/examples/provider/wifi_manager.h b/examples/provider/wifi_manager.h index 72f54df..a119d7f 100644 --- a/examples/provider/wifi_manager.h +++ b/examples/provider/wifi_manager.h @@ -35,8 +35,9 @@ class WifiImpl : public provider::Wifi { const DoneCallback& callback) override; void StartAccessPoint(const std::string& ssid) override; void StopAccessPoint() override; - bool IsWifi24Supported() const override {return true;}; - bool IsWifi50Supported() const override {return false;}; + bool IsWifi24Supported() const override; + bool IsWifi50Supported() const override; + std::string GetConnectedSsid() const override; static bool HasWifiCapability(); @@ -46,7 +47,7 @@ class WifiImpl : public provider::Wifi { int pid, base::Time until, const DoneCallback& callback); - bool hostapd_started_{false}; + std::string hostapd_ssid_; provider::TaskRunner* task_runner_{nullptr}; EventNetworkImpl* network_{nullptr}; base::WeakPtrFactory<WifiImpl> weak_ptr_factory_{this}; diff --git a/file_lists.mk b/file_lists.mk index e13553f..6345939 100644 --- a/file_lists.mk +++ b/file_lists.mk @@ -130,7 +130,6 @@ THIRD_PARTY_CHROMIUM_BASE_UNITTEST_SRC_FILES := \ third_party/chromium/base/json/string_escape_unittest.cc \ third_party/chromium/base/logging_unittest.cc \ third_party/chromium/base/memory/ref_counted_unittest.cc \ - third_party/chromium/base/memory/scoped_ptr_unittest.cc \ third_party/chromium/base/memory/weak_ptr_unittest.cc \ third_party/chromium/base/numerics/safe_numerics_unittest.cc \ third_party/chromium/base/observer_list_unittest.cc \ diff --git a/include/weave/provider/http_server.h b/include/weave/provider/http_server.h index 1c28d63..33aff35 100644 --- a/include/weave/provider/http_server.h +++ b/include/weave/provider/http_server.h @@ -132,6 +132,9 @@ class HttpServer { const std::string& path, const RequestHandlerCallback& callback) = 0; + virtual void RemoveHttpRequestHandler(const std::string& path) = 0; + virtual void RemoveHttpsRequestHandler(const std::string& path) = 0; + virtual uint16_t GetHttpPort() const = 0; virtual uint16_t GetHttpsPort() const = 0; virtual std::vector<uint8_t> GetHttpsCertificateFingerprint() const = 0; diff --git a/include/weave/provider/network.h b/include/weave/provider/network.h index 0fb147d..a73b75e 100644 --- a/include/weave/provider/network.h +++ b/include/weave/provider/network.h @@ -14,6 +14,28 @@ namespace weave { namespace provider { +// This interface should be implemented by the user of libweave and +// provided during device creation in Device::Create(...) +// libweave will use this interface to create and interact with network +// sockets, implementing XMPP push notifications channel. +// +// There are 2 main parts of this interface. One part is used to check network +// connection status and signup for notification if connection status changes. +// Implementation of GetConnectionState() should return current network +// connection state (enum Network::State). +// Implementation of AddConnectionChangedCallback() should remember callback +// and invoke it network connection status changes. +// +// Second part of the interface is used to create new secure (SSL) socket and +// provide functionality to read/write data into sockets. +// Implementation of OpenSslSocket() should create SSL socket using whatever +// underlying mechanism is available on the current platform. It should also +// wrap read / write / disconnect functionality into Stream interface +// (include/weave/stream.h) and call OpenSslSocketCallback() callback +// with pointer to the Stream interface implementation. +// Reading data from the socket will be done through InputStream::Read() +// function. Writing to socket - through OutputStream::Write(). + // Interface with methods to detect network connectivity and opening network // connections. class Network { diff --git a/include/weave/provider/task_runner.h b/include/weave/provider/task_runner.h index 095910b..28ad42c 100644 --- a/include/weave/provider/task_runner.h +++ b/include/weave/provider/task_runner.h @@ -16,6 +16,27 @@ namespace weave { namespace provider { +// This interface should be implemented by the user of libweave and +// provided during device creation in Device::Create(...) +// libweave will use this interface to schedule task execution. +// +// libweave is a single threaded library that uses task scheduling (similar +// to message loop) to perform asynchronous tasks. libweave itself does not +// implement message loop, and rely on user to provide implementation +// for its platform. +// +// Implementation of PostDelayedTask(...) should add specified task (callback) +// after all current tasks in the queue with delay=0. If there are tasks in the +// queue with delay > 0, new task may be scheduled before or between existing +// tasks. Position of the new task in the queue depends on remaining delay for +// existing tasks and specified delay. Normally, new task should be scheduled +// after the last task with "remaining delay" <= "new task delay". This will +// guarantee that all tasks with delay=0 will be executed in the same order +// they are put in the task queue. +// +// If delay is specified, task should be invoked no sooner then timeout is +// reached (it might be delayed due to other tasks in the queue). + // Interface with methods to post tasks into platform-specific message loop of // the current thread. class TaskRunner { diff --git a/include/weave/provider/test/mock_http_server.h b/include/weave/provider/test/mock_http_server.h index 995a8d8..e6b1f55 100644 --- a/include/weave/provider/test/mock_http_server.h +++ b/include/weave/provider/test/mock_http_server.h @@ -22,6 +22,8 @@ class MockHttpServer : public HttpServer { void(const std::string&, const RequestHandlerCallback&)); MOCK_METHOD2(AddHttpsRequestHandler, void(const std::string&, const RequestHandlerCallback&)); + MOCK_METHOD1(RemoveHttpRequestHandler, void(const std::string&)); + MOCK_METHOD1(RemoveHttpsRequestHandler, void(const std::string&)); MOCK_CONST_METHOD0(GetHttpPort, uint16_t()); MOCK_CONST_METHOD0(GetHttpsPort, uint16_t()); MOCK_CONST_METHOD0(GetHttpsCertificateFingerprint, std::vector<uint8_t>()); diff --git a/include/weave/provider/test/mock_wifi.h b/include/weave/provider/test/mock_wifi.h index 7ede55e..8fff0a3 100644 --- a/include/weave/provider/test/mock_wifi.h +++ b/include/weave/provider/test/mock_wifi.h @@ -25,6 +25,7 @@ class MockWifi : public Wifi { MOCK_METHOD0(StopAccessPoint, void()); MOCK_CONST_METHOD0(IsWifi24Supported, bool()); MOCK_CONST_METHOD0(IsWifi50Supported, bool()); + MOCK_CONST_METHOD0(GetConnectedSsid, std::string()); }; } // namespace test diff --git a/include/weave/provider/wifi.h b/include/weave/provider/wifi.h index 9c7cfca..033728f 100644 --- a/include/weave/provider/wifi.h +++ b/include/weave/provider/wifi.h @@ -13,6 +13,34 @@ namespace weave { namespace provider { +// This interface should be implemented by the user of libweave and +// provided during device creation in Device::Create(...) +// libweave will use this interface to get WiFi capabilities and +// configure WiFi on the device. +// +// If device does not support WiFi (e.g. Ethernet device), user code +// should supply nullptr pointer for WiFi interface at the device creation +// time. +// +// Implementation of Connect(...) method should connect to specified +// WiFi access point (identified by ssid) using supplied passphrase. +// If WiFi access point allows open connection, passphrase will contain +// empty string. libweave should be notified when connection is established +// through callback invocation. +// +// Implementation of StartAccessPoint(...) should start open WiFi access point +// according to WiFi specification using specified ssid. New AP should be +// available on 2.4Ghz WiFi band, and be open (allow connection without +// passphrase). +// Implementation of StopAccessPoint() should stop previously started WiFi +// access point. +// +// Implementations of IsWifi24Supported() and IsWifi50Supported() should +// return if true if respectively 2.4Ghz and 5.0Ghz WiFi bands are supported. +// +// Implementation of GetConnectedSsid() should return SSID of the WiFi network +// device is currently connected to. + // Interface with methods to control WiFi capability of the device. class Wifi { public: @@ -31,6 +59,9 @@ class Wifi { virtual bool IsWifi24Supported() const = 0; virtual bool IsWifi50Supported() const = 0; + // Get SSID of the network device is connected. + virtual std::string GetConnectedSsid() const = 0; + protected: virtual ~Wifi() {} }; diff --git a/src/access_api_handler.cc b/src/access_api_handler.cc index dcfae45..906fe25 100644 --- a/src/access_api_handler.cc +++ b/src/access_api_handler.cc @@ -18,13 +18,13 @@ namespace weave { namespace { const char kComponent[] = "accessControl"; -const char kTrait[] = "_accessRevocationList"; -const char kStateCapacity[] = "_accessRevocationList.capacity"; +const char kTrait[] = "blacklist"; +const char kStateCapacity[] = "blacklist.capacity"; const char kUserId[] = "userId"; const char kApplicationId[] = "applicationId"; const char kExpirationTime[] = "expirationTime"; const char kRevocationTimestamp[] = "revocationTimestamp"; -const char kRevocationList[] = "revocationListEntries"; +const char kBlacklistEntries[] = "blacklistEntries"; bool GetIds(const base::DictionaryValue& parameters, std::vector<uint8_t>* user_id_decoded, @@ -55,9 +55,9 @@ AccessApiHandler::AccessApiHandler(Device* device, AccessRevocationManager* manager) : device_{device}, manager_{manager} { device_->AddTraitDefinitionsFromJson(R"({ - "_accessRevocationList": { + "blacklist": { "commands": { - "revoke": { + "add": { "minimalRole": "owner", "parameters": { "userId": { @@ -78,7 +78,7 @@ AccessApiHandler::AccessApiHandler(Device* device, "minimalRole": "owner", "parameters": {}, "results": { - "revocationEntriesList": { + "blacklistEntries": { "type": "array", "items": { "type": "object", @@ -114,10 +114,10 @@ AccessApiHandler::AccessApiHandler(Device* device, UpdateState(); device_->AddCommandHandler( - kComponent, "_accessRevocationList.revoke", + kComponent, "blacklist.add", base::Bind(&AccessApiHandler::Block, weak_ptr_factory_.GetWeakPtr())); device_->AddCommandHandler( - kComponent, "_accessRevocationList.list", + kComponent, "blacklist.list", base::Bind(&AccessApiHandler::List, weak_ptr_factory_.GetWeakPtr())); } @@ -176,11 +176,11 @@ void AccessApiHandler::List(const std::weak_ptr<Command>& cmd) { std::unique_ptr<base::DictionaryValue> entry{new base::DictionaryValue}; entry->SetString(kUserId, Base64Encode(e.user_id)); entry->SetString(kApplicationId, Base64Encode(e.app_id)); - entries->Append(entry.release()); + entries->Append(std::move(entry)); } base::DictionaryValue result; - result.Set(kRevocationList, entries.release()); + result.Set(kBlacklistEntries, std::move(entries)); command->Complete(result, nullptr); } diff --git a/src/access_api_handler.h b/src/access_api_handler.h index 16eaa6f..b84bfff 100644 --- a/src/access_api_handler.h +++ b/src/access_api_handler.h @@ -16,12 +16,12 @@ class AccessRevocationManager; class Command; class Device; -// Handles commands for '_accessRevocationList' trait. +// Handles commands for 'blacklist' trait. // Objects of the class subscribe for notification from CommandManager and // execute incoming commands. // Handled commands: -// _accessRevocationList.revoke -// _accessRevocationList.list +// blacklist.add +// blacklist.list class AccessApiHandler final { public: AccessApiHandler(Device* device, AccessRevocationManager* manager); diff --git a/src/access_api_handler_unittest.cc b/src/access_api_handler_unittest.cc index fda0748..d7bb4e6 100644 --- a/src/access_api_handler_unittest.cc +++ b/src/access_api_handler_unittest.cc @@ -44,8 +44,7 @@ class AccessApiHandlerTest : public ::testing::Test { })); EXPECT_CALL(device_, - AddCommandHandler(_, AnyOf("_accessRevocationList.revoke", - "_accessRevocationList.list"), + AddCommandHandler(_, AnyOf("blacklist.add", "blacklist.list"), _)) .WillRepeatedly( Invoke(&component_manager_, &ComponentManager::AddCommandHandler)); @@ -71,14 +70,14 @@ class AccessApiHandlerTest : public ::testing::Test { std::unique_ptr<base::DictionaryValue> GetState() { std::string path = - component_manager_.FindComponentWithTrait("_accessRevocationList"); + component_manager_.FindComponentWithTrait("blacklist"); EXPECT_FALSE(path.empty()); const auto* component = component_manager_.FindComponent(path, nullptr); EXPECT_TRUE(component); const base::DictionaryValue* state = nullptr; EXPECT_TRUE( - component->GetDictionary("state._accessRevocationList", &state)); - return std::unique_ptr<base::DictionaryValue>{state->DeepCopy()}; + component->GetDictionary("state.blacklist", &state)); + return state->CreateDeepCopy(); } StrictMock<provider::test::FakeTaskRunner> task_runner_; @@ -91,11 +90,11 @@ class AccessApiHandlerTest : public ::testing::Test { TEST_F(AccessApiHandlerTest, Initialization) { const base::DictionaryValue* trait = nullptr; ASSERT_TRUE(component_manager_.GetTraits().GetDictionary( - "_accessRevocationList", &trait)); + "blacklist", &trait)); auto expected = R"({ "commands": { - "revoke": { + "add": { "minimalRole": "owner", "parameters": { "userId": { @@ -116,7 +115,7 @@ TEST_F(AccessApiHandlerTest, Initialization) { "minimalRole": "owner", "parameters": {}, "results": { - "revocationEntriesList": { + "blacklistEntries": { "type": "array", "items": { "type": "object", @@ -168,7 +167,7 @@ TEST_F(AccessApiHandlerTest, Revoke) { EXPECT_CALL(access_manager_, GetSize()).WillRepeatedly(Return(1)); AddCommand(R"({ - 'name' : '_accessRevocationList.revoke', + 'name' : 'blacklist.add', 'component': 'accessControl', 'parameters': { 'userId': 'AQID', @@ -199,7 +198,7 @@ TEST_F(AccessApiHandlerTest, List) { EXPECT_CALL(access_manager_, GetSize()).WillRepeatedly(Return(4)); auto expected = R"({ - "revocationListEntries": [ { + "blacklistEntries": [ { "applicationId": "FRYX", "userId": "CwwN" }, { @@ -209,7 +208,7 @@ TEST_F(AccessApiHandlerTest, List) { })"; const auto& results = AddCommand(R"({ - 'name' : '_accessRevocationList.list', + 'name' : 'blacklist.list', 'component': 'accessControl', 'parameters': { } diff --git a/src/access_revocation_manager.h b/src/access_revocation_manager.h index 6d5bf7b..ba2bcca 100644 --- a/src/access_revocation_manager.h +++ b/src/access_revocation_manager.h @@ -52,8 +52,10 @@ class AccessRevocationManager { inline bool operator==(const AccessRevocationManager::Entry& l, const AccessRevocationManager::Entry& r) { - return l.revocation == r.revocation && l.expiration == r.expiration && - l.user_id == r.user_id && l.app_id == r.app_id; + auto make_tuple = [](const AccessRevocationManager::Entry& e) { + return std::tie(e.revocation, e.expiration, e.user_id, e.app_id); + }; + return make_tuple(l) == make_tuple(r); } inline bool operator!=(const AccessRevocationManager::Entry& l, diff --git a/src/access_revocation_manager_impl.cc b/src/access_revocation_manager_impl.cc index f039114..035e1ad 100644 --- a/src/access_revocation_manager_impl.cc +++ b/src/access_revocation_manager_impl.cc @@ -83,12 +83,37 @@ void AccessRevocationManagerImpl::Save(const DoneCallback& callback) { store_->SaveSettings(kConfigFileName, json, callback); } -void AccessRevocationManagerImpl::RemoveExpired() { +void AccessRevocationManagerImpl::Shrink() { + base::Time oldest[2] = {base::Time::Max(), base::Time::Max()}; for (auto i = begin(entries_); i != end(entries_);) { if (i->expiration <= clock_->Now()) i = entries_.erase(i); - else + else { + // Non-strict comparison to ensure counting same timestamps as different. + if (i->revocation <= oldest[0]) { + oldest[1] = oldest[0]; + oldest[0] = i->revocation; + } else { + oldest[1] = std::min(oldest[1], i->revocation); + } ++i; + } + } + CHECK_GT(capacity_, 1u); + if (entries_.size() >= capacity_) { + // List is full so we are going to remove oldest entries from the list. + for (auto i = begin(entries_); i != end(entries_);) { + if (i->revocation <= oldest[1]) + i = entries_.erase(i); + else { + ++i; + } + } + // And replace with a single rule to block everything older. + Entry all_blocking_entry; + all_blocking_entry.expiration = base::Time::Max(); + all_blocking_entry.revocation = oldest[1]; + entries_.insert(all_blocking_entry); } } @@ -99,8 +124,6 @@ void AccessRevocationManagerImpl::AddEntryAddedCallback( void AccessRevocationManagerImpl::Block(const Entry& entry, const DoneCallback& callback) { - // Iterating is OK as Save below is more expensive. - RemoveExpired(); if (entry.expiration <= clock_->Now()) { if (!callback.is_null()) { ErrorPtr error; @@ -110,15 +133,10 @@ void AccessRevocationManagerImpl::Block(const Entry& entry, } return; } - if (entries_.size() >= capacity_) { - if (!callback.is_null()) { - ErrorPtr error; - Error::AddTo(&error, FROM_HERE, "blacklist_is_full", - "Unable to store more entries"); - callback.Run(std::move(error)); - } - return; - } + + // Iterating is OK as Save below is more expensive. + Shrink(); + CHECK_LT(entries_.size(), capacity_); auto existing = entries_.find(entry); if (existing != entries_.end()) { diff --git a/src/access_revocation_manager_impl.h b/src/access_revocation_manager_impl.h index a911128..9aed36a 100644 --- a/src/access_revocation_manager_impl.h +++ b/src/access_revocation_manager_impl.h @@ -36,15 +36,14 @@ class AccessRevocationManagerImpl : public AccessRevocationManager { private: void Load(); void Save(const DoneCallback& callback); - void RemoveExpired(); + void Shrink(); struct EntryIdsLess { bool operator()(const Entry& l, const Entry& r) const { - if (l.user_id < r.user_id) - return true; - if (l.user_id > r.user_id) - return false; - return l.app_id < r.app_id; + auto make_tuple = [](const AccessRevocationManager::Entry& e) { + return std::tie(e.user_id, e.app_id); + }; + return make_tuple(l) < make_tuple(r); } }; diff --git a/src/access_revocation_manager_impl_unittest.cc b/src/access_revocation_manager_impl_unittest.cc index b0b774b..afc63fe 100644 --- a/src/access_revocation_manager_impl_unittest.cc +++ b/src/access_revocation_manager_impl_unittest.cc @@ -112,29 +112,46 @@ TEST_F(AccessRevocationManagerImplTest, BlockExpired) { })); } -TEST_F(AccessRevocationManagerImplTest, BlockListIsFull) { +TEST_F(AccessRevocationManagerImplTest, BlockListOverflow) { + EXPECT_CALL(config_store_, LoadSettings("black_list")).WillOnce(Return("")); + manager_.reset(new AccessRevocationManagerImpl{&config_store_, 10, &clock_}); + EXPECT_CALL(config_store_, SaveSettings("black_list", _, _)) .WillRepeatedly(testing::WithArgs<1, 2>(testing::Invoke( [](const std::string& json, const DoneCallback& callback) { if (!callback.is_null()) callback.Run(nullptr); }))); - for (size_t i = manager_->GetSize(); i < manager_->GetCapacity(); ++i) { - manager_->Block( - {{99, static_cast<uint8_t>(i / 256), static_cast<uint8_t>(i % 256)}, - {8, 8, 8}, - base::Time::FromTimeT(1419970000), - base::Time::FromTimeT(1419990000)}, - {}); - EXPECT_EQ(i + 1, manager_->GetSize()); + + EXPECT_EQ(0u, manager_->GetSize()); + + // Trigger overflow several times. + for (size_t i = 0; i < manager_->GetCapacity() + 3; ++i) { + bool callback_called = false; + manager_->Block({{99, static_cast<uint8_t>(i), static_cast<uint8_t>(i)}, + {8, 8, 8}, + base::Time::FromTimeT(1419970000 + i), + base::Time::FromTimeT(1419990000)}, + base::Bind([&callback_called](ErrorPtr error) { + callback_called = true; + EXPECT_FALSE(error); + })); + EXPECT_TRUE(callback_called); + } + EXPECT_EQ(manager_->GetCapacity(), manager_->GetSize()); + + // We didn't block these ids, so we can use this to check if all_blocking + // issue is set for correct revocation time. + EXPECT_TRUE(manager_->IsBlocked({1}, {2}, base::Time::FromTimeT(1419970003))); + EXPECT_FALSE( + manager_->IsBlocked({1}, {2}, base::Time::FromTimeT(1419970004))); + + // Check if all blocking rules still work. + for (size_t i = 0; i < manager_->GetCapacity() + 3; ++i) { + EXPECT_TRUE(manager_->IsBlocked( + {99, static_cast<uint8_t>(i), static_cast<uint8_t>(i)}, {8, 8, 8}, + base::Time::FromTimeT(1419970000 + i))); } - manager_->Block({{99}, - {8, 8, 8}, - base::Time::FromTimeT(1419970000), - base::Time::FromTimeT(1419990000)}, - base::Bind([](ErrorPtr error) { - EXPECT_TRUE(error->HasError("blacklist_is_full")); - })); } TEST_F(AccessRevocationManagerImplTest, IsBlockedIdsNotMacth) { diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc index 2a202d1..12fb3b6 100644 --- a/src/base_api_handler_unittest.cc +++ b/src/base_api_handler_unittest.cc @@ -84,7 +84,7 @@ class BaseApiHandlerTest : public ::testing::Test { CHECK(component); const base::DictionaryValue* base_state = nullptr; if (component->GetDictionary("state.base", &base_state)) - state.reset(base_state->DeepCopy()); + state = base_state->CreateDeepCopy(); else state.reset(new base::DictionaryValue); return state; diff --git a/src/commands/cloud_command_proxy.cc b/src/commands/cloud_command_proxy.cc index f8f8d1f..c12e833 100644 --- a/src/commands/cloud_command_proxy.cc +++ b/src/commands/cloud_command_proxy.cc @@ -35,8 +35,8 @@ void CloudCommandProxy::OnErrorChanged() { std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue}; patch->Set(commands::attributes::kCommand_Error, command_instance_->GetError() - ? ErrorInfoToJson(*command_instance_->GetError()).release() - : base::Value::CreateNullValue().release()); + ? ErrorInfoToJson(*command_instance_->GetError()) + : base::Value::CreateNullValue()); QueueCommandUpdate(std::move(patch)); } diff --git a/src/commands/cloud_command_proxy_unittest.cc b/src/commands/cloud_command_proxy_unittest.cc index e0bd4fc..3c65cd1 100644 --- a/src/commands/cloud_command_proxy_unittest.cc +++ b/src/commands/cloud_command_proxy_unittest.cc @@ -77,9 +77,7 @@ class CloudCommandProxyWrapper : public CloudCommandProxy { task_runner}, destruct_callback_{destruct_callback} {} - ~CloudCommandProxyWrapper() { - destruct_callback_.Run(); - } + ~CloudCommandProxyWrapper() { destruct_callback_.Run(); } private: base::Closure destruct_callback_; diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc index 590bbb1..1e7e16f 100644 --- a/src/commands/command_instance.cc +++ b/src/commands/command_instance.cc @@ -156,7 +156,7 @@ std::unique_ptr<base::DictionaryValue> GetCommandParameters( "Property '%s' must be a JSON object", commands::attributes::kCommand_Parameters); } - params.reset(params_dict->DeepCopy()); + params = params_dict->CreateDeepCopy(); } else { // "parameters" are not specified. Assume empty param list. params.reset(new base::DictionaryValue); @@ -220,14 +220,15 @@ std::unique_ptr<base::DictionaryValue> CommandInstance::ToJson() const { json->SetString(commands::attributes::kCommand_Id, id_); json->SetString(commands::attributes::kCommand_Name, name_); - json->Set(commands::attributes::kCommand_Parameters, parameters_.DeepCopy()); - json->Set(commands::attributes::kCommand_Progress, progress_.DeepCopy()); - json->Set(commands::attributes::kCommand_Results, results_.DeepCopy()); + json->SetString(commands::attributes::kCommand_Component, component_); + json->Set(commands::attributes::kCommand_Parameters, + parameters_.CreateDeepCopy()); + json->Set(commands::attributes::kCommand_Progress, + progress_.CreateDeepCopy()); + json->Set(commands::attributes::kCommand_Results, results_.CreateDeepCopy()); json->SetString(commands::attributes::kCommand_State, EnumToString(state_)); - if (error_) { - json->Set(commands::attributes::kCommand_Error, - ErrorInfoToJson(*error_).release()); - } + if (error_) + json->Set(commands::attributes::kCommand_Error, ErrorInfoToJson(*error_)); return json; } diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc index 803d5b4..e06332f 100644 --- a/src/commands/command_instance_unittest.cc +++ b/src/commands/command_instance_unittest.cc @@ -103,6 +103,7 @@ TEST(CommandInstanceTest, FromJson_ParamsNotObject) { TEST(CommandInstanceTest, ToJson) { auto json = CreateDictionaryValue(R"({ + 'component': 'testComponent', 'name': 'robot.jump', 'parameters': { 'height': 53, @@ -121,6 +122,7 @@ TEST(CommandInstanceTest, ToJson) { nullptr)); json->MergeDictionary(CreateDictionaryValue(R"({ + 'component': 'testComponent', 'id': 'testId', 'progress': {'progress': 15}, 'state': 'done', @@ -135,6 +137,7 @@ TEST(CommandInstanceTest, ToJson) { TEST(CommandInstanceTest, ToJsonError) { auto json = CreateDictionaryValue(R"({ + 'component': 'testComponent', 'name': 'base.reboot', 'parameters': {} })"); @@ -147,6 +150,7 @@ TEST(CommandInstanceTest, ToJsonError) { instance->Abort(error.get(), nullptr); json->MergeDictionary(CreateDictionaryValue(R"({ + 'component': 'testComponent', 'id': 'testId', 'state': 'aborted', 'progress': {}, diff --git a/src/commands/command_queue.cc b/src/commands/command_queue.cc index f0d2228..8faae35 100644 --- a/src/commands/command_queue.cc +++ b/src/commands/command_queue.cc @@ -124,9 +124,8 @@ void CommandQueue::Cleanup(const base::Time& cutoff_time) { void CommandQueue::ScheduleCleanup(base::TimeDelta delay) { task_runner_->PostDelayedTask( - FROM_HERE, - base::Bind(&CommandQueue::PerformScheduledCleanup, - weak_ptr_factory_.GetWeakPtr()), + FROM_HERE, base::Bind(&CommandQueue::PerformScheduledCleanup, + weak_ptr_factory_.GetWeakPtr()), delay); } diff --git a/src/component_manager.h b/src/component_manager.h index cea5569..1a3d05f 100644 --- a/src/component_manager.h +++ b/src/component_manager.h @@ -154,9 +154,14 @@ class ComponentManager { const std::string& command_name) const = 0; // Checks the minimum required user role for a given command. - virtual bool GetMinimalRole(const std::string& command_name, - UserRole* minimal_role, - ErrorPtr* error) const = 0; + virtual bool GetCommandMinimalRole(const std::string& command_name, + UserRole* minimal_role, + ErrorPtr* error) const = 0; + + // Checks the minimum required user role for a given state property. + virtual bool GetStateMinimalRole(const std::string& state_property_name, + UserRole* minimal_role, + ErrorPtr* error) const = 0; // Returns the full JSON dictionary containing trait definitions. virtual const base::DictionaryValue& GetTraits() const = 0; @@ -164,6 +169,11 @@ class ComponentManager { // Returns the full JSON dictionary containing component instances. virtual const base::DictionaryValue& GetComponents() const = 0; + // Returns a JSON dictionary containing component instances with state + // properties visible to a user of the given |role|. + virtual std::unique_ptr<base::DictionaryValue> GetComponentsForUserRole( + UserRole role) const = 0; + // Component state manipulation methods. virtual bool SetStateProperties(const std::string& component_path, const base::DictionaryValue& dict, diff --git a/src/component_manager_impl.cc b/src/component_manager_impl.cc index 3ea1f46..805e57a 100644 --- a/src/component_manager_impl.cc +++ b/src/component_manager_impl.cc @@ -27,6 +27,59 @@ const EnumToStringMap<UserRole>::Map kMap[] = { {UserRole::kOwner, "owner"}, {UserRole::kManager, "manager"}, }; + +void RemoveInaccessibleState(const ComponentManagerImpl* manager, + base::DictionaryValue* component, + UserRole role) { + std::vector<std::string> state_props_to_remove; + base::DictionaryValue* state = nullptr; + if (component->GetDictionary("state", &state)) { + for (base::DictionaryValue::Iterator it_trait(*state); !it_trait.IsAtEnd(); + it_trait.Advance()) { + const base::DictionaryValue* trait = nullptr; + CHECK(it_trait.value().GetAsDictionary(&trait)); + for (base::DictionaryValue::Iterator it_prop(*trait); !it_prop.IsAtEnd(); + it_prop.Advance()) { + std::string prop_name = base::StringPrintf( + "%s.%s", it_trait.key().c_str(), it_prop.key().c_str()); + UserRole minimal_role; + if (manager->GetStateMinimalRole(prop_name, &minimal_role, nullptr) && + minimal_role > role) { + state_props_to_remove.push_back(prop_name); + } + } + } + } + // Now remove any inaccessible properties from the state collection. + for (const std::string& path : state_props_to_remove) { + // Remove starting from component level in order for "state" to be removed + // if no sub-properties remain. + CHECK(component->RemovePath(base::StringPrintf("state.%s", path.c_str()), + nullptr)); + } + + // If this component has any sub-components, filter them too. + base::DictionaryValue* sub_components = nullptr; + if (component->GetDictionary("components", &sub_components)) { + for (base::DictionaryValue::Iterator it_component(*sub_components); + !it_component.IsAtEnd(); it_component.Advance()) { + base::Value* sub_component = nullptr; + CHECK(sub_components->Get(it_component.key(), &sub_component)); + if (sub_component->GetType() == base::Value::TYPE_LIST) { + base::ListValue* component_array = nullptr; + CHECK(sub_component->GetAsList(&component_array)); + for (base::Value* item : *component_array) { + CHECK(item->GetAsDictionary(&component)); + RemoveInaccessibleState(manager, component, role); + } + } else if (sub_component->GetType() == base::Value::TYPE_DICTIONARY) { + CHECK(sub_component->GetAsDictionary(&component)); + RemoveInaccessibleState(manager, component, role); + } + } + } +} + } // anonymous namespace template <> @@ -67,8 +120,8 @@ bool ComponentManagerImpl::AddComponent(const std::string& path, std::unique_ptr<base::DictionaryValue> dict{new base::DictionaryValue}; std::unique_ptr<base::ListValue> traits_list{new base::ListValue}; traits_list->AppendStrings(traits); - dict->Set("traits", traits_list.release()); - root->SetWithoutPathExpansion(name, dict.release()); + dict->Set("traits", std::move(traits_list)); + root->SetWithoutPathExpansion(name, std::move(dict)); for (const auto& cb : on_componet_tree_changed_) cb.Run(); return true; @@ -93,8 +146,8 @@ bool ComponentManagerImpl::AddComponentArrayItem( std::unique_ptr<base::DictionaryValue> dict{new base::DictionaryValue}; std::unique_ptr<base::ListValue> traits_list{new base::ListValue}; traits_list->AppendStrings(traits); - dict->Set("traits", traits_list.release()); - array_value->Append(dict.release()); + dict->Set("traits", std::move(traits_list)); + array_value->Append(std::move(dict)); for (const auto& cb : on_componet_tree_changed_) cb.Run(); return true; @@ -180,7 +233,7 @@ bool ComponentManagerImpl::LoadTraits(const base::DictionaryValue& dict, break; } } else { - traits_.Set(it.key(), it.value().DeepCopy()); + traits_.Set(it.key(), it.value().CreateDeepCopy()); modified = true; } } @@ -230,7 +283,7 @@ std::unique_ptr<CommandInstance> ComponentManagerImpl::ParseCommandInstance( return nullptr; UserRole minimal_role; - if (!GetMinimalRole(command_instance->GetName(), &minimal_role, error)) + if (!GetCommandMinimalRole(command_instance->GetName(), &minimal_role, error)) return nullptr; if (role < minimal_role) { @@ -346,9 +399,24 @@ const base::DictionaryValue* ComponentManagerImpl::FindCommandDefinition( return definition; } -bool ComponentManagerImpl::GetMinimalRole(const std::string& command_name, - UserRole* minimal_role, - ErrorPtr* error) const { +const base::DictionaryValue* ComponentManagerImpl::FindStateDefinition( + const std::string& state_property_name) const { + const base::DictionaryValue* definition = nullptr; + std::vector<std::string> components = + Split(state_property_name, ".", true, false); + // Make sure the |state_property_name| came in form of trait_name.state_name. + if (components.size() != 2) + return definition; + std::string key = base::StringPrintf("%s.state.%s", components[0].c_str(), + components[1].c_str()); + traits_.GetDictionary(key, &definition); + return definition; +} + +bool ComponentManagerImpl::GetCommandMinimalRole( + const std::string& command_name, + UserRole* minimal_role, + ErrorPtr* error) const { const base::DictionaryValue* command = FindCommandDefinition(command_name); if (!command) { return Error::AddToPrintf( @@ -363,12 +431,47 @@ bool ComponentManagerImpl::GetMinimalRole(const std::string& command_name, return true; } +bool ComponentManagerImpl::GetStateMinimalRole( + const std::string& state_property_name, + UserRole* minimal_role, + ErrorPtr* error) const { + const base::DictionaryValue* state = FindStateDefinition(state_property_name); + if (!state) { + return Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, + "State definition for '%s' not found", + state_property_name.c_str()); + } + std::string value; + if (state->GetString(kMinimalRole, &value)) { + CHECK(StringToEnum(value, minimal_role)); + } else { + *minimal_role = UserRole::kUser; + } + return true; +} + void ComponentManagerImpl::AddStateChangedCallback( const base::Closure& callback) { on_state_changed_.push_back(callback); callback.Run(); // Force to read current state. } +std::unique_ptr<base::DictionaryValue> +ComponentManagerImpl::GetComponentsForUserRole(UserRole role) const { + auto components = components_.CreateDeepCopy(); + // Build a list of all state properties that are inaccessible to the given + // user. These properties will be removed from the components collection + // returned from this method. + for (base::DictionaryValue::Iterator it_component(components_); + !it_component.IsAtEnd(); it_component.Advance()) { + base::DictionaryValue* component = nullptr; + CHECK(components->GetDictionary(it_component.key(), &component)); + RemoveInaccessibleState(this, component, role); + } + + return components; +} + bool ComponentManagerImpl::SetStateProperties(const std::string& component_path, const base::DictionaryValue& dict, ErrorPtr* error) { @@ -447,7 +550,7 @@ bool ComponentManagerImpl::SetStateProperty(const std::string& component_path, error, FROM_HERE, errors::commands::kPropertyMissing, "State property name not specified in '%s'", name.c_str()); } - dict.Set(name, value.DeepCopy()); + dict.Set(name, value.CreateDeepCopy()); return SetStateProperties(component_path, dict, error); } @@ -484,7 +587,7 @@ ComponentManager::Token ComponentManagerImpl::AddServerStateUpdatedCallback( const base::Callback<void(UpdateID)>& callback) { if (state_change_queues_.empty()) callback.Run(GetLastStateChangeId()); - return Token{on_server_state_updated_.Add(callback).release()}; + return Token{on_server_state_updated_.Add(callback)}; } std::string ComponentManagerImpl::FindComponentWithTrait( diff --git a/src/component_manager_impl.h b/src/component_manager_impl.h index 5b8201a..f7995ed 100644 --- a/src/component_manager_impl.h +++ b/src/component_manager_impl.h @@ -115,10 +115,20 @@ class ComponentManagerImpl final : public ComponentManager { const base::DictionaryValue* FindCommandDefinition( const std::string& command_name) const override; + // Finds a state definition, where |state_property_name| is in the form of + // "trait.state". + const base::DictionaryValue* FindStateDefinition( + const std::string& state_property_name) const; + // Checks the minimum required user role for a given command. - bool GetMinimalRole(const std::string& command_name, - UserRole* minimal_role, - ErrorPtr* error) const override; + bool GetCommandMinimalRole(const std::string& command_name, + UserRole* minimal_role, + ErrorPtr* error) const override; + + // Checks the minimum required user role for a given state. + bool GetStateMinimalRole(const std::string& state_property_name, + UserRole* minimal_role, + ErrorPtr* error) const override; // Returns the full JSON dictionary containing trait definitions. const base::DictionaryValue& GetTraits() const override { return traits_; } @@ -128,6 +138,11 @@ class ComponentManagerImpl final : public ComponentManager { return components_; } + // Returns a JSON dictionary containing component instances with state + // properties visible to a user of the given |role|. + std::unique_ptr<base::DictionaryValue> GetComponentsForUserRole( + UserRole role) const override; + // Component state manipulation methods. bool SetStateProperties(const std::string& component_path, const base::DictionaryValue& dict, diff --git a/src/component_manager_unittest.cc b/src/component_manager_unittest.cc index 291ace8..cdd089e 100644 --- a/src/component_manager_unittest.cc +++ b/src/component_manager_unittest.cc @@ -404,7 +404,7 @@ TEST_F(ComponentManagerTest, FindCommandDefinition) { manager_.FindTraitDefinition("trait1.command1.parameters")); } -TEST_F(ComponentManagerTest, GetMinimalRole) { +TEST_F(ComponentManagerTest, GetCommandMinimalRole) { const char kTraits[] = R"({ "trait1": { "commands": { @@ -423,19 +423,63 @@ TEST_F(ComponentManagerTest, GetMinimalRole) { ASSERT_TRUE(manager_.LoadTraits(*json, nullptr)); UserRole role; - ASSERT_TRUE(manager_.GetMinimalRole("trait1.command1", &role, nullptr)); + ASSERT_TRUE( + manager_.GetCommandMinimalRole("trait1.command1", &role, nullptr)); EXPECT_EQ(UserRole::kUser, role); - ASSERT_TRUE(manager_.GetMinimalRole("trait1.command2", &role, nullptr)); + ASSERT_TRUE( + manager_.GetCommandMinimalRole("trait1.command2", &role, nullptr)); EXPECT_EQ(UserRole::kViewer, role); - ASSERT_TRUE(manager_.GetMinimalRole("trait2.command1", &role, nullptr)); + ASSERT_TRUE( + manager_.GetCommandMinimalRole("trait2.command1", &role, nullptr)); EXPECT_EQ(UserRole::kManager, role); - ASSERT_TRUE(manager_.GetMinimalRole("trait2.command2", &role, nullptr)); + ASSERT_TRUE( + manager_.GetCommandMinimalRole("trait2.command2", &role, nullptr)); EXPECT_EQ(UserRole::kOwner, role); - EXPECT_FALSE(manager_.GetMinimalRole("trait1.command3", &role, nullptr)); + EXPECT_FALSE( + manager_.GetCommandMinimalRole("trait1.command3", &role, nullptr)); +} + +TEST_F(ComponentManagerTest, GetStateMinimalRole) { + const char kTraits[] = R"({ + "trait1": { + "state": { + "property1": {"type": "integer"}, + "property2": {"type": "boolean", "minimalRole": "viewer"}, + "property3": {"type": "integer", "minimalRole": "user"} + } + }, + "trait2": { + "state": { + "property1": {"type": "string", "minimalRole": "manager"}, + "property2": {"type": "integer", "minimalRole": "owner"} + } + } + })"; + auto json = CreateDictionaryValue(kTraits); + ASSERT_TRUE(manager_.LoadTraits(*json, nullptr)); + + UserRole role; + ASSERT_TRUE(manager_.GetStateMinimalRole("trait1.property1", &role, nullptr)); + EXPECT_EQ(UserRole::kUser, role); + + ASSERT_TRUE(manager_.GetStateMinimalRole("trait1.property2", &role, nullptr)); + EXPECT_EQ(UserRole::kViewer, role); + + ASSERT_TRUE(manager_.GetStateMinimalRole("trait1.property3", &role, nullptr)); + EXPECT_EQ(UserRole::kUser, role); + + ASSERT_TRUE(manager_.GetStateMinimalRole("trait2.property1", &role, nullptr)); + EXPECT_EQ(UserRole::kManager, role); + + ASSERT_TRUE(manager_.GetStateMinimalRole("trait2.property2", &role, nullptr)); + EXPECT_EQ(UserRole::kOwner, role); + + ASSERT_FALSE( + manager_.GetStateMinimalRole("trait2.property3", &role, nullptr)); } TEST_F(ComponentManagerTest, AddComponent) { @@ -528,8 +572,8 @@ TEST_F(ComponentManagerTest, AddComponentArrayItem) { TEST_F(ComponentManagerTest, RemoveComponent) { CreateTestComponentTree(&manager_); - EXPECT_TRUE(manager_.RemoveComponent("comp1.comp2[1].comp3", "comp4", - nullptr)); + EXPECT_TRUE( + manager_.RemoveComponent("comp1.comp2[1].comp3", "comp4", nullptr)); const char kExpected1[] = R"({ "comp1": { "traits": [ "t1" ], @@ -1267,4 +1311,216 @@ TEST_F(ComponentManagerTest, TestMockComponentManager) { test::MockComponentManager mock; } +TEST_F(ComponentManagerTest, GetComponentsForUserRole) { + const char kTraits[] = R"({ + "trait1": { + "state": { + "prop1": { "type": "string", "minimalRole": "viewer" }, + "prop2": { "type": "string", "minimalRole": "user" } + } + }, + "trait2": { + "state": { + "prop3": { "type": "string", "minimalRole": "manager" }, + "prop4": { "type": "string", "minimalRole": "owner" } + } + }, + "trait3": { + "state": { + "prop5": { "type": "string" } + } + } + })"; + auto json = CreateDictionaryValue(kTraits); + ASSERT_TRUE(manager_.LoadTraits(*json, nullptr)); + ASSERT_TRUE( + manager_.AddComponent("", "comp1", {"trait1", "trait2"}, nullptr)); + ASSERT_TRUE(manager_.AddComponent("", "comp2", {"trait2"}, nullptr)); + ASSERT_TRUE( + manager_.AddComponentArrayItem("comp2", "comp3", {"trait3"}, nullptr)); + ASSERT_TRUE(manager_.AddComponent("comp2", "comp4", {"trait3"}, nullptr)); + + const char kComp1Properties[] = R"({ + "trait1": { "prop1": "foo", "prop2": "bar" }, + "trait2": { "prop3": "baz", "prop4": "quux" } + })"; + ASSERT_TRUE( + manager_.SetStatePropertiesFromJson("comp1", kComp1Properties, nullptr)); + + const char kComp2Properties[] = R"({ + "trait2": { "prop3": "foo", "prop4": "bar" } + })"; + ASSERT_TRUE( + manager_.SetStatePropertiesFromJson("comp2", kComp2Properties, nullptr)); + + const char kComp3Properties[] = R"({ + "trait3": { "prop5": "foo" } + })"; + ASSERT_TRUE(manager_.SetStatePropertiesFromJson("comp2.comp3[0]", + kComp3Properties, nullptr)); + + const char kComp4Properties[] = R"({ + "trait3": { "prop5": "bar" } + })"; + ASSERT_TRUE(manager_.SetStatePropertiesFromJson("comp2.comp4", + kComp4Properties, nullptr)); + + const char kExpected1[] = R"({ + "comp1": { + "traits": [ "trait1", "trait2" ], + "state": { + "trait1": { + "prop1": "foo", + "prop2": "bar" + }, + "trait2": { + "prop3": "baz", + "prop4": "quux" + } + } + }, + "comp2": { + "traits": [ "trait2" ], + "state": { + "trait2": { + "prop3": "foo", + "prop4": "bar" + } + }, + "components": { + "comp3": [ + { + "traits": [ "trait3" ], + "state": { + "trait3": { + "prop5": "foo" + } + } + } + ], + "comp4": { + "traits": [ "trait3" ], + "state": { + "trait3": { + "prop5": "bar" + } + } + } + } + } + })"; + EXPECT_JSON_EQ(kExpected1, manager_.GetComponents()); + + EXPECT_JSON_EQ(kExpected1, + *manager_.GetComponentsForUserRole(UserRole::kOwner)); + + const char kExpected2[] = R"({ + "comp1": { + "traits": [ "trait1", "trait2" ], + "state": { + "trait1": { + "prop1": "foo", + "prop2": "bar" + }, + "trait2": { + "prop3": "baz" + } + } + }, + "comp2": { + "traits": [ "trait2" ], + "state": { + "trait2": { + "prop3": "foo" + } + }, + "components": { + "comp3": [ + { + "traits": [ "trait3" ], + "state": { + "trait3": { + "prop5": "foo" + } + } + } + ], + "comp4": { + "traits": [ "trait3" ], + "state": { + "trait3": { + "prop5": "bar" + } + } + } + } + } + })"; + EXPECT_JSON_EQ(kExpected2, + *manager_.GetComponentsForUserRole(UserRole::kManager)); + + const char kExpected3[] = R"({ + "comp1": { + "traits": [ "trait1", "trait2" ], + "state": { + "trait1": { + "prop1": "foo", + "prop2": "bar" + } + } + }, + "comp2": { + "traits": [ "trait2" ], + "components": { + "comp3": [ + { + "traits": [ "trait3" ], + "state": { + "trait3": { + "prop5": "foo" + } + } + } + ], + "comp4": { + "traits": [ "trait3" ], + "state": { + "trait3": { + "prop5": "bar" + } + } + } + } + } + })"; + EXPECT_JSON_EQ(kExpected3, + *manager_.GetComponentsForUserRole(UserRole::kUser)); + + const char kExpected4[] = R"({ + "comp1": { + "traits": [ "trait1", "trait2" ], + "state": { + "trait1": { + "prop1": "foo" + } + } + }, + "comp2": { + "traits": [ "trait2" ], + "components": { + "comp3": [ + { + "traits": [ "trait3" ] + } + ], + "comp4": { + "traits": [ "trait3" ] + } + } + } + })"; + EXPECT_JSON_EQ(kExpected4, + *manager_.GetComponentsForUserRole(UserRole::kViewer)); +} + } // namespace weave diff --git a/src/config.cc b/src/config.cc index cd28de9..108c6aa 100644 --- a/src/config.cc +++ b/src/config.cc @@ -4,6 +4,8 @@ #include "src/config.h" + +#include <algorithm> #include <set> #include <base/bind.h> diff --git a/src/config_unittest.cc b/src/config_unittest.cc index bb2743a..3c020f4 100644 --- a/src/config_unittest.cc +++ b/src/config_unittest.cc @@ -25,9 +25,7 @@ const char kConfigName[] = "config"; class ConfigTest : public ::testing::Test { protected: - void SetUp() override { - Reload(); - } + void SetUp() override { Reload(); } void Reload() { EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); diff --git a/src/device_manager.cc b/src/device_manager.cc index b6b91a9..3f8e4d7 100644 --- a/src/device_manager.cc +++ b/src/device_manager.cc @@ -30,7 +30,12 @@ DeviceManager::DeviceManager(provider::ConfigStore* config_store, provider::HttpServer* http_server, provider::Wifi* wifi, provider::Bluetooth* bluetooth) - : config_{new Config{config_store}}, + : task_runner_{task_runner}, + network_{network}, + dns_sd_{dns_sd}, + http_server_{http_server}, + wifi_{wifi}, + config_{new Config{config_store}}, component_manager_{new ComponentManagerImpl{task_runner}} { if (http_server) { access_revocation_manager_.reset( @@ -50,7 +55,9 @@ DeviceManager::DeviceManager(provider::ConfigStore* config_store, device_info_->Start(); if (http_server) { - StartPrivet(task_runner, network, dns_sd, http_server, wifi, bluetooth); + StartPrivet(); + AddSettingsChangedCallback(base::Bind(&DeviceManager::OnSettingsChanged, + weak_ptr_factory_.GetWeakPtr())); } else { CHECK(!dns_sd); } @@ -71,17 +78,18 @@ Config* DeviceManager::GetConfig() { return device_info_->GetMutableConfig(); } -void DeviceManager::StartPrivet(provider::TaskRunner* task_runner, - provider::Network* network, - provider::DnsServiceDiscovery* dns_sd, - provider::HttpServer* http_server, - provider::Wifi* wifi, - provider::Bluetooth* bluetooth) { - privet_.reset(new privet::Manager{task_runner}); - privet_->Start(network, dns_sd, http_server, wifi, auth_manager_.get(), +void DeviceManager::StartPrivet() { + if (privet_) + return; + privet_.reset(new privet::Manager{task_runner_}); + privet_->Start(network_, dns_sd_, http_server_, wifi_, auth_manager_.get(), device_info_.get(), component_manager_.get()); } +void DeviceManager::StopPrivet() { + privet_.reset(); +} + GcdState DeviceManager::GetGcdState() const { return device_info_->GetGcdState(); } @@ -188,6 +196,14 @@ void DeviceManager::AddPairingChangedCallbacks( privet_->AddOnPairingChangedCallbacks(begin_callback, end_callback); } +void DeviceManager::OnSettingsChanged(const Settings& settings) { + if (settings.local_discovery_enabled && http_server_) { + StartPrivet(); + } else { + StopPrivet(); + } +} + std::unique_ptr<Device> Device::Create(provider::ConfigStore* config_store, provider::TaskRunner* task_runner, provider::HttpClient* http_client, diff --git a/src/device_manager.h b/src/device_manager.h index 545f293..c41a627 100644 --- a/src/device_manager.h +++ b/src/device_manager.h @@ -81,12 +81,15 @@ class DeviceManager final : public Device { Config* GetConfig(); private: - void StartPrivet(provider::TaskRunner* task_runner, - provider::Network* network, - provider::DnsServiceDiscovery* dns_sd, - provider::HttpServer* http_server, - provider::Wifi* wifi, - provider::Bluetooth* bluetooth); + void StartPrivet(); + void StopPrivet(); + void OnSettingsChanged(const Settings& settings); + + provider::TaskRunner* task_runner_{nullptr}; + provider::Network* network_{nullptr}; + provider::DnsServiceDiscovery* dns_sd_{nullptr}; + provider::HttpServer* http_server_{nullptr}; + provider::Wifi* wifi_{nullptr}; std::unique_ptr<Config> config_; std::unique_ptr<privet::AuthManager> auth_manager_; diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index 3ae1321..a278e63 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc @@ -212,17 +212,13 @@ std::unique_ptr<base::DictionaryValue> ParseJsonResponse( error_message.c_str(), json.c_str()); return std::unique_ptr<base::DictionaryValue>(); } - base::DictionaryValue* dict_value = nullptr; - if (!value->GetAsDictionary(&dict_value)) { + auto dict_value = base::DictionaryValue::From(std::move(value)); + if (!dict_value) { Error::AddToPrintf(error, FROM_HERE, errors::json::kObjectExpected, "Response is not a valid JSON object: '%s'", json.c_str()); - return std::unique_ptr<base::DictionaryValue>(); - } else { - // |value| is now owned by |dict_value|, so release the scoped_ptr now. - base::IgnoreResult(value.release()); } - return std::unique_ptr<base::DictionaryValue>(dict_value); + return dict_value; } bool IsSuccessful(const HttpClient::Response& response) { @@ -425,7 +421,12 @@ void DeviceRegistrationInfo::OnRefreshAccessTokenDone( StartNotificationChannel(); } - SendAuthInfo(); + if (GetSettings().root_client_token_owner != RootClientTokenOwner::kCloud) { + // Avoid re-claiming if device is already claimed by the Cloud. Cloud is + // allowed to re-claim device at any time. However this will invalidate all + // issued tokens. + SendAuthInfo(); + } callback.Run(nullptr); } @@ -495,9 +496,10 @@ DeviceRegistrationInfo::BuildDeviceResource() const { } else { channel->SetString("supportedType", "pull"); } - resource->Set("channel", channel.release()); - resource->Set("traits", component_manager_->GetTraits().DeepCopy()); - resource->Set("components", component_manager_->GetComponents().DeepCopy()); + resource->Set("channel", std::move(channel)); + resource->Set("traits", component_manager_->GetTraits().CreateDeepCopy()); + resource->Set("components", + component_manager_->GetComponents().CreateDeepCopy()); return resource; } @@ -570,7 +572,7 @@ void DeviceRegistrationInfo::RegisterDevice(RegistrationData registration_data, base::DictionaryValue req_json; req_json.SetString("id", registration_data.ticket_id); req_json.SetString("oauthClientId", registration_data.client_id); - req_json.Set("deviceDraft", device_draft.release()); + req_json.Set("deviceDraft", std::move(device_draft)); auto url = BuildUrl(registration_data.service_url, "registrationTickets/" + registration_data.ticket_id, @@ -899,7 +901,7 @@ void DeviceRegistrationInfo::NotifyCommandAborted(const std::string& command_id, EnumToString(Command::State::kAborted)); if (error) { command_patch.Set(commands::attributes::kCommand_Error, - ErrorInfoToJson(*error).release()); + ErrorInfoToJson(*error)); } UpdateCommand(command_id, command_patch, base::Bind(&IgnoreCloudError)); } @@ -954,12 +956,7 @@ void DeviceRegistrationInfo::SendAuthInfo() { if (!auth_manager_ || auth_info_update_inprogress_) return; - if (GetSettings().root_client_token_owner == RootClientTokenOwner::kCloud) { - // Avoid re-claiming if device is already claimed by the Cloud. Cloud is - // allowed to re-claim device at any time. However this will invalidate all - // issued tokens. - return; - } + LOG(INFO) << "Updating local auth info"; auth_info_update_inprogress_ = true; @@ -976,7 +973,7 @@ void DeviceRegistrationInfo::SendAuthInfo() { auth->SetString("clientToken", token_base64); auth->SetString("certFingerprint", fingerprint); std::unique_ptr<base::DictionaryValue> root{new base::DictionaryValue}; - root->Set("localAuthInfo", auth.release()); + root->Set("localAuthInfo", std::move(auth)); std::string url = GetDeviceUrl("upsertLocalAuthInfo", {}); DoCloudRequest(HttpClient::Method::kPost, url, root.get(), @@ -1028,6 +1025,18 @@ void DeviceRegistrationInfo::OnUpdateDeviceResourceDone( if (error) return OnUpdateDeviceResourceError(std::move(error)); UpdateDeviceInfoTimestamp(device_info); + + if (auth_manager_) { + std::string fingerprint_base64; + std::vector<uint8_t> fingerprint; + if (!device_info.GetString("certFingerprint", &fingerprint_base64) || + !Base64Decode(fingerprint_base64, &fingerprint) || + fingerprint != auth_manager_->GetCertificateFingerprint()) { + LOG(WARNING) << "Local auth info from server is invalid"; + SendAuthInfo(); + } + } + // Make a copy of the callback list so that if the callback triggers another // call to UpdateDeviceResource(), we do not modify the list we are iterating // over. @@ -1129,7 +1138,7 @@ void DeviceRegistrationInfo::ProcessInitialCommandList( continue; } - std::unique_ptr<base::DictionaryValue> cmd_copy{command_dict->DeepCopy()}; + auto cmd_copy = command_dict->CreateDeepCopy(); cmd_copy->SetString("state", "aborted"); // TODO(wiley) We could consider handling this error case more gracefully. DoCloudRequest(HttpClient::Method::kPut, @@ -1201,14 +1210,14 @@ void DeviceRegistrationInfo::PublishStateUpdates() { patch->SetString("timeMs", std::to_string(state_change.timestamp.ToJavaTime())); patch->SetString("component", state_change.component); - patch->Set("patch", state_change.changed_properties.release()); - patches->Append(patch.release()); + patch->Set("patch", std::move(state_change.changed_properties)); + patches->Append(std::move(patch)); } base::DictionaryValue body; body.SetString("requestTimeMs", std::to_string(base::Time::Now().ToJavaTime())); - body.Set("patches", patches.release()); + body.Set("patches", std::move(patches)); device_state_update_pending_ = true; DoCloudRequest(HttpClient::Method::kPost, GetDeviceUrl("patchState"), &body, diff --git a/src/device_registration_info.h b/src/device_registration_info.h index a488bae..db08ef9 100644 --- a/src/device_registration_info.h +++ b/src/device_registration_info.h @@ -115,6 +115,9 @@ class DeviceRegistrationInfo : public NotificationDelegate, GcdState GetGcdState() const { return gcd_state_; } + // Checks whether we have credentials generated during registration. + bool HaveRegistrationCredentials() const; + private: friend class DeviceRegistrationInfoTest; @@ -124,8 +127,6 @@ class DeviceRegistrationInfo : public NotificationDelegate, return weak_factory_.GetWeakPtr(); } - // Checks whether we have credentials generated during registration. - bool HaveRegistrationCredentials() const; // Calls HaveRegistrationCredentials() and logs an error if no credentials // are available. bool VerifyRegistrationCredentials(ErrorPtr* error) const; diff --git a/src/device_registration_info_unittest.cc b/src/device_registration_info_unittest.cc index adc5175..757b1c9 100644 --- a/src/device_registration_info_unittest.cc +++ b/src/device_registration_info_unittest.cc @@ -207,10 +207,10 @@ class DeviceRegistrationInfoTest : public ::testing::Test { std::unique_ptr<Config> config_; test::MockClock clock_; privet::AuthManager auth_{ - {68, 52, 36, 95, 74, 89, 25, 2, 31, 5, 65, 87, 64, 32, 17, 26, 8, 73, 57, - 16, 33, 82, 71, 10, 72, 62, 45, 1, 77, 97, 70, 24}, - {21, 6, 58, 4, 66, 13, 14, 60, 55, 22, 11, 38, 96, 40, 81, 90, 3, 51, 50, - 23, 56, 76, 47, 46, 27, 69, 20, 80, 88, 93, 15, 61}, + {68, 52, 36, 95, 74, 89, 25, 2, 31, 5, 65, 87, 64, 32, 17, 26, + 8, 73, 57, 16, 33, 82, 71, 10, 72, 62, 45, 1, 77, 97, 70, 24}, + {21, 6, 58, 4, 66, 13, 14, 60, 55, 22, 11, 38, 96, 40, 81, 90, + 3, 51, 50, 23, 56, 76, 47, 46, 27, 69, 20, 80, 88, 93, 15, 61}, {}, &clock_}; std::unique_ptr<DeviceRegistrationInfo> dev_reg_; diff --git a/src/privet/auth_manager.cc b/src/privet/auth_manager.cc index d905a45..52d3a43 100644 --- a/src/privet/auth_manager.cc +++ b/src/privet/auth_manager.cc @@ -447,7 +447,14 @@ std::vector<uint8_t> AuthManager::GetRootClientAuthToken( } base::Time AuthManager::Now() const { - return clock_->Now(); + base::Time now = clock_->Now(); + static const base::Time k2010 = base::Time::FromTimeT(1262304000); + if (now >= k2010) + return now; + // Slowdown time before 1 Jan 2010. This will increase expiration time of + // access tokens but allow to handle dates which can not be handled by + // macaroon library. + return k2010 - base::TimeDelta::FromSeconds((k2010 - now).InSeconds() / 10); } bool AuthManager::IsValidAuthToken(const std::vector<uint8_t>& token, diff --git a/src/privet/auth_manager_unittest.cc b/src/privet/auth_manager_unittest.cc index 648027d..ba1862c 100644 --- a/src/privet/auth_manager_unittest.cc +++ b/src/privet/auth_manager_unittest.cc @@ -222,6 +222,15 @@ TEST_F(AuthManagerTest, AccessTokenAfterReset) { EXPECT_TRUE(auth_.ParseAccessToken(token2, &user_info, nullptr)); } +TEST_F(AuthManagerTest, AccessTokenBeforeJ2000) { + EXPECT_CALL(clock_, Now()) + .WillRepeatedly(Return(base::Time::FromTimeT(5678))); + UserInfo user_info; + auto token1 = auth_.CreateAccessToken( + UserInfo{AuthScope::kViewer, TestUserId{"555"}}, {}); + EXPECT_TRUE(auth_.ParseAccessToken(token1, &user_info, nullptr)); +} + TEST_F(AuthManagerTest, GetRootClientAuthToken) { EXPECT_EQ("WCCDQxkgAUYIGhudoQBCDABQX3fPR5zsPnrs9aOSvS7/eQ==", Base64Encode( diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc index f565687..f9d53c3 100644 --- a/src/privet/cloud_delegate.cc +++ b/src/privet/cloud_delegate.cc @@ -26,6 +26,8 @@ namespace privet { namespace { +const char kErrorAlreayRegistered[] = "already_registered"; + const BackoffEntry::Policy register_backoff_policy = {0, 1000, 2.0, 0.2, 5000, -1, false}; @@ -46,23 +48,13 @@ class CloudDelegateImpl : public CloudDelegate { : task_runner_{task_runner}, device_{device}, component_manager_{component_manager} { - device_->GetMutableConfig()->AddOnChangedCallback(base::Bind( - &CloudDelegateImpl::OnConfigChanged, weak_factory_.GetWeakPtr())); device_->AddGcdStateChangedCallback(base::Bind( &CloudDelegateImpl::OnRegistrationChanged, weak_factory_.GetWeakPtr())); - component_manager_->AddTraitDefChangedCallback( - base::Bind(&CloudDelegateImpl::NotifyOnTraitDefsChanged, - weak_factory_.GetWeakPtr())); component_manager_->AddCommandAddedCallback(base::Bind( &CloudDelegateImpl::OnCommandAdded, weak_factory_.GetWeakPtr())); component_manager_->AddCommandRemovedCallback(base::Bind( &CloudDelegateImpl::OnCommandRemoved, weak_factory_.GetWeakPtr())); - component_manager_->AddStateChangedCallback(base::Bind( - &CloudDelegateImpl::NotifyOnStateChanged, weak_factory_.GetWeakPtr())); - component_manager_->AddComponentTreeChangedCallback( - base::Bind(&CloudDelegateImpl::NotifyOnComponentTreeChanged, - weak_factory_.GetWeakPtr())); } ~CloudDelegateImpl() override = default; @@ -113,6 +105,12 @@ class CloudDelegateImpl : public CloudDelegate { bool Setup(const RegistrationData& registration_data, ErrorPtr* error) override { VLOG(1) << "GCD Setup started. "; + if (device_->HaveRegistrationCredentials()) { + Error::AddTo(error, FROM_HERE, kErrorAlreayRegistered, + "Unable to register already registered device"); + return false; + } + // Set (or reset) the retry counter, since we are starting a new // registration process. registation_retry_count_ = kMaxDeviceRegistrationRetries; @@ -151,8 +149,12 @@ class CloudDelegateImpl : public CloudDelegate { return device_->GetSettings().xmpp_endpoint; } - const base::DictionaryValue& GetComponents() const override { - return component_manager_->GetComponents(); + std::unique_ptr<base::DictionaryValue> GetComponentsForUser( + const UserInfo& user_info) const override { + UserRole role; + std::string str_scope = EnumToString(user_info.scope()); + CHECK(StringToEnum(str_scope, &role)); + return component_manager_->GetComponentsForUserRole(role); } const base::DictionaryValue* FindComponent(const std::string& path, @@ -218,18 +220,28 @@ class CloudDelegateImpl : public CloudDelegate { base::ListValue list_value; for (const auto& it : command_owners_) { - if (CanAccessCommand(it.second, user_info, nullptr)) { - list_value.Append( - component_manager_->FindCommand(it.first)->ToJson().release()); - } + if (CanAccessCommand(it.second, user_info, nullptr)) + list_value.Append(component_manager_->FindCommand(it.first)->ToJson()); } base::DictionaryValue commands_json; - commands_json.Set("commands", list_value.DeepCopy()); + commands_json.Set("commands", list_value.CreateDeepCopy()); callback.Run(commands_json, nullptr); } + void AddOnTraitsChangedCallback(const base::Closure& callback) override { + component_manager_->AddTraitDefChangedCallback(callback); + } + + void AddOnStateChangedCallback(const base::Closure& callback) override { + component_manager_->AddStateChangedCallback(callback); + } + + void AddOnComponentsChangeCallback(const base::Closure& callback) override { + component_manager_->AddComponentTreeChangedCallback(callback); + } + private: void OnCommandAdded(Command* command) { // Set to "" for any new unknown command. @@ -240,8 +252,6 @@ class CloudDelegateImpl : public CloudDelegate { CHECK(command_owners_.erase(command->GetID())); } - void OnConfigChanged(const Settings&) { NotifyOnDeviceInfoChanged(); } - void OnRegistrationChanged(GcdState status) { if (status == GcdState::kUnconfigured || status == GcdState::kInvalidCredentials) { @@ -258,7 +268,6 @@ class CloudDelegateImpl : public CloudDelegate { EnumToString(status).c_str()); connection_state_ = ConnectionState{std::move(error)}; } - NotifyOnDeviceInfoChanged(); } void OnRegisterSuccess(const std::string& cloud_id) { @@ -374,21 +383,5 @@ std::unique_ptr<CloudDelegate> CloudDelegate::CreateDefault( new CloudDelegateImpl{task_runner, device, component_manager}}; } -void CloudDelegate::NotifyOnDeviceInfoChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnDeviceInfoChanged()); -} - -void CloudDelegate::NotifyOnTraitDefsChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnTraitDefsChanged()); -} - -void CloudDelegate::NotifyOnComponentTreeChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnComponentTreeChanged()); -} - -void CloudDelegate::NotifyOnStateChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnStateChanged()); -} - } // namespace privet } // namespace weave diff --git a/src/privet/cloud_delegate.h b/src/privet/cloud_delegate.h index 43b8904..6d0ed48 100644 --- a/src/privet/cloud_delegate.h +++ b/src/privet/cloud_delegate.h @@ -11,7 +11,6 @@ #include <base/callback.h> #include <base/memory/ref_counted.h> -#include <base/observer_list.h> #include <weave/device.h> #include "src/privet/privet_types.h" @@ -43,16 +42,6 @@ class CloudDelegate { base::Callback<void(const base::DictionaryValue& commands, ErrorPtr error)>; - class Observer { - public: - virtual ~Observer() {} - - virtual void OnDeviceInfoChanged() {} - virtual void OnTraitDefsChanged() {} - virtual void OnStateChanged() {} - virtual void OnComponentTreeChanged() {} - }; - // Returns the ID of the device. virtual std::string GetDeviceId() const = 0; @@ -100,8 +89,10 @@ class CloudDelegate { virtual std::string GetServiceUrl() const = 0; virtual std::string GetXmppEndpoint() const = 0; - // Returns dictionary with component tree. - virtual const base::DictionaryValue& GetComponents() const = 0; + // Returns dictionary with component tree. The components contain only the + // state visible to the given user. + virtual std::unique_ptr<base::DictionaryValue> GetComponentsForUser( + const UserInfo& user_info) const = 0; // Finds a component at the given path. Return nullptr in case of an error. virtual const base::DictionaryValue* FindComponent(const std::string& path, @@ -129,24 +120,15 @@ class CloudDelegate { virtual void ListCommands(const UserInfo& user_info, const CommandDoneCallback& callback) = 0; - void AddObserver(Observer* observer) { observer_list_.AddObserver(observer); } - void RemoveObserver(Observer* observer) { - observer_list_.RemoveObserver(observer); - } - - void NotifyOnDeviceInfoChanged(); - void NotifyOnTraitDefsChanged(); - void NotifyOnStateChanged(); - void NotifyOnComponentTreeChanged(); + virtual void AddOnTraitsChangedCallback(const base::Closure& callback) = 0; + virtual void AddOnStateChangedCallback(const base::Closure& callback) = 0; + virtual void AddOnComponentsChangeCallback(const base::Closure& callback) = 0; // Create default instance. static std::unique_ptr<CloudDelegate> CreateDefault( provider::TaskRunner* task_runner, DeviceRegistrationInfo* device, ComponentManager* component_manager); - - private: - base::ObserverList<Observer> observer_list_; }; } // namespace privet diff --git a/src/privet/mock_delegates.h b/src/privet/mock_delegates.h index f04fb37..5338e1b 100644 --- a/src/privet/mock_delegates.h +++ b/src/privet/mock_delegates.h @@ -20,8 +20,10 @@ #include "src/privet/wifi_delegate.h" using testing::_; +using testing::AtLeast; using testing::Return; using testing::ReturnRef; +using testing::SaveArg; using testing::SetArgPointee; namespace weave { @@ -188,7 +190,8 @@ class MockCloudDelegate : public CloudDelegate { MOCK_CONST_METHOD0(GetOAuthUrl, std::string()); MOCK_CONST_METHOD0(GetServiceUrl, std::string()); MOCK_CONST_METHOD0(GetXmppEndpoint, std::string()); - MOCK_CONST_METHOD0(GetComponents, const base::DictionaryValue&()); + MOCK_CONST_METHOD1(MockGetComponentsForUser, + const base::DictionaryValue&(const UserInfo&)); MOCK_CONST_METHOD2(FindComponent, const base::DictionaryValue*(const std::string& path, ErrorPtr* error)); @@ -206,6 +209,9 @@ class MockCloudDelegate : public CloudDelegate { const UserInfo&, const CommandDoneCallback&)); MOCK_METHOD2(ListCommands, void(const UserInfo&, const CommandDoneCallback&)); + MOCK_METHOD1(AddOnTraitsChangedCallback, void(const base::Closure&)); + MOCK_METHOD1(AddOnStateChangedCallback, void(const base::Closure&)); + MOCK_METHOD1(AddOnComponentsChangeCallback, void(const base::Closure&)); MockCloudDelegate() { EXPECT_CALL(*this, GetDeviceId()).WillRepeatedly(Return("TestId")); @@ -228,13 +234,37 @@ class MockCloudDelegate : public CloudDelegate { EXPECT_CALL(*this, GetCloudId()).WillRepeatedly(Return("TestCloudId")); test_dict_.Set("test", new base::DictionaryValue); EXPECT_CALL(*this, GetTraits()).WillRepeatedly(ReturnRef(test_dict_)); - EXPECT_CALL(*this, GetComponents()).WillRepeatedly(ReturnRef(test_dict_)); + EXPECT_CALL(*this, MockGetComponentsForUser(_)) + .WillRepeatedly(ReturnRef(test_dict_)); EXPECT_CALL(*this, FindComponent(_, _)).Times(0); + + EXPECT_CALL(*this, AddOnTraitsChangedCallback(_)) + .WillRepeatedly(SaveArg<0>(&on_traits_changed_)); + EXPECT_CALL(*this, AddOnStateChangedCallback(_)) + .WillRepeatedly(SaveArg<0>(&on_state_changed_)); + EXPECT_CALL(*this, AddOnComponentsChangeCallback(_)) + .WillRepeatedly(SaveArg<0>(&on_components_changed_)); } + void NotifyOnTraitDefsChanged() { on_traits_changed_.Run(); } + + void NotifyOnComponentTreeChanged() { on_components_changed_.Run(); } + + void NotifyOnStateChanged() { on_state_changed_.Run(); } + ConnectionState connection_state_{ConnectionState::kOnline}; SetupState setup_state_{SetupState::kNone}; base::DictionaryValue test_dict_; + + base::Closure on_traits_changed_; + base::Closure on_state_changed_; + base::Closure on_components_changed_; + + private: + std::unique_ptr<base::DictionaryValue> GetComponentsForUser( + const UserInfo& user_info) const override { + return MockGetComponentsForUser(user_info).CreateDeepCopy(); + } }; } // namespace privet diff --git a/src/privet/privet_handler.cc b/src/privet/privet_handler.cc index 83d5ef3..42a1c23 100644 --- a/src/privet/privet_handler.cc +++ b/src/privet/privet_handler.cc @@ -177,9 +177,9 @@ std::unique_ptr<base::DictionaryValue> ErrorToJson(const Error& error) { it->GetLocation().file_name.c_str(), it->GetLocation().line_number, nullptr}; inner->SetString(kErrorDebugInfoKey, location.ToString()); - errors->Append(inner.release()); + errors->Append(std::move(inner)); } - output->Set(kErrorDebugInfoKey, errors.release()); + output->Set(kErrorDebugInfoKey, std::move(errors)); return output; } @@ -190,7 +190,7 @@ void SetStateProperties(const T& state, base::DictionaryValue* parent) { return; } parent->SetString(kStatusKey, kStatusErrorValue); - parent->Set(kErrorKey, ErrorToJson(*state.error()).release()); + parent->Set(kErrorKey, ErrorToJson(*state.error())); } void ReturnError(const Error& error, @@ -203,7 +203,7 @@ void ReturnError(const Error& error, } } std::unique_ptr<base::DictionaryValue> output{new base::DictionaryValue}; - output->Set(kErrorKey, ErrorToJson(error).release()); + output->Set(kErrorKey, ErrorToJson(error)); callback.Run(code, *output); } @@ -260,17 +260,17 @@ std::unique_ptr<base::DictionaryValue> CreateInfoAuthSection( std::unique_ptr<base::ListValue> pairing_types(new base::ListValue()); for (PairingType type : security.GetPairingTypes()) pairing_types->AppendString(EnumToString(type)); - auth->Set(kPairingKey, pairing_types.release()); + auth->Set(kPairingKey, std::move(pairing_types)); std::unique_ptr<base::ListValue> auth_types(new base::ListValue()); for (AuthType type : security.GetAuthTypes()) auth_types->AppendString(EnumToString(type)); - auth->Set(kAuthModeKey, auth_types.release()); + auth->Set(kAuthModeKey, std::move(auth_types)); std::unique_ptr<base::ListValue> crypto_types(new base::ListValue()); for (CryptoType type : security.GetCryptoTypes()) crypto_types->AppendString(EnumToString(type)); - auth->Set(kCryptoKey, crypto_types.release()); + auth->Set(kCryptoKey, std::move(crypto_types)); return auth; } @@ -282,7 +282,7 @@ std::unique_ptr<base::DictionaryValue> CreateWifiSection( std::unique_ptr<base::ListValue> capabilities(new base::ListValue()); for (WifiType type : wifi.GetTypes()) capabilities->AppendString(EnumToString(type)); - result->Set(kInfoWifiCapabilitiesKey, capabilities.release()); + result->Set(kInfoWifiCapabilitiesKey, std::move(capabilities)); result->SetString(kInfoWifiSsidKey, wifi.GetCurrentlyConnectedSsid()); @@ -340,9 +340,9 @@ std::unique_ptr<base::DictionaryValue> CloneComponent( const base::DictionaryValue* sub_components = nullptr; CHECK(it.value().GetAsDictionary(&sub_components)); clone->SetWithoutPathExpansion( - it.key(), CloneComponentTree(*sub_components, filter).release()); + it.key(), CloneComponentTree(*sub_components, filter)); } else { - clone->SetWithoutPathExpansion(it.key(), it.value().DeepCopy()); + clone->SetWithoutPathExpansion(it.key(), it.value().CreateDeepCopy()); } } } @@ -350,8 +350,8 @@ std::unique_ptr<base::DictionaryValue> CloneComponent( } // Clones a dictionary containing a bunch of component JSON objects in a manner -// similar to that of DeepCopy(). Calls CloneComponent() on each instance of -// the component sub-object. +// similar to that of CreateDeepCopy(). Calls CloneComponent() on each instance +// of the component sub-object. std::unique_ptr<base::DictionaryValue> CloneComponentTree( const base::DictionaryValue& parent, const std::set<std::string>& filter) { @@ -361,7 +361,7 @@ std::unique_ptr<base::DictionaryValue> CloneComponentTree( const base::DictionaryValue* component = nullptr; CHECK(it.value().GetAsDictionary(&component)); clone->SetWithoutPathExpansion( - it.key(), CloneComponent(*component, filter).release()); + it.key(), CloneComponent(*component, filter)); } return clone; } @@ -398,7 +398,13 @@ PrivetHandler::PrivetHandler(CloudDelegate* cloud, CHECK(device_); CHECK(security_); CHECK(clock_); - cloud_observer_.Add(cloud_); + + cloud_->AddOnTraitsChangedCallback(base::Bind( + &PrivetHandler::OnTraitDefsChanged, weak_ptr_factory_.GetWeakPtr())); + cloud_->AddOnStateChangedCallback(base::Bind(&PrivetHandler::OnStateChanged, + weak_ptr_factory_.GetWeakPtr())); + cloud_->AddOnComponentsChangeCallback(base::Bind( + &PrivetHandler::OnComponentTreeChanged, weak_ptr_factory_.GetWeakPtr())); AddHandler("/privet/info", &PrivetHandler::HandleInfo, AuthScope::kNone); AddHandler("/privet/v3/pairing/start", &PrivetHandler::HandlePairingStart, @@ -559,23 +565,21 @@ void PrivetHandler::HandleInfo(const base::DictionaryValue&, output.SetString(kLocationKey, location); output.SetString(kInfoModelIdKey, model_id); - output.Set(kInfoModelManifestKey, CreateManifestSection(*cloud_).release()); + output.Set(kInfoModelManifestKey, CreateManifestSection(*cloud_)); output.Set( kInfoServicesKey, - ToValue(std::vector<std::string>{GetDeviceUiKind(cloud_->GetModelId())}) - .release()); + ToValue(std::vector<std::string>{GetDeviceUiKind(cloud_->GetModelId())})); output.Set( kInfoAuthenticationKey, - CreateInfoAuthSection(*security_, GetAnonymousMaxScope(*cloud_, wifi_)) - .release()); + CreateInfoAuthSection(*security_, GetAnonymousMaxScope(*cloud_, wifi_))); - output.Set(kInfoEndpointsKey, CreateEndpointsSection(*device_).release()); + output.Set(kInfoEndpointsKey, CreateEndpointsSection(*device_)); if (wifi_) - output.Set(kWifiKey, CreateWifiSection(*wifi_).release()); + output.Set(kWifiKey, CreateWifiSection(*wifi_)); - output.Set(kGcdKey, CreateGcdSection(*cloud_).release()); + output.Set(kGcdKey, CreateGcdSection(*cloud_)); output.SetDouble(kInfoTimeKey, clock_->Now().ToJsTime()); output.SetString(kInfoSessionIdKey, security_->CreateSessionId()); @@ -707,6 +711,8 @@ void PrivetHandler::HandleAuth(const base::DictionaryValue& input, return ReturnError(*error, callback); } + CHECK_LE(access_token_scope, desired_scope); + if (access_token_scope < acceptable_scope) { Error::AddToPrintf(&error, FROM_HERE, errors::kAccessDenied, "Scope '%s' is not allowed", @@ -829,8 +835,9 @@ void PrivetHandler::HandleSetupStart(const base::DictionaryValue& input, return ReturnError(*error, callback); if (!registration_data.ticket_id.empty() && - !cloud_->Setup(registration_data, &error)) + !cloud_->Setup(registration_data, &error)) { return ReturnError(*error, callback); + } ReplyWithSetupStatus(callback); } @@ -873,7 +880,7 @@ void PrivetHandler::HandleTraits(const base::DictionaryValue& input, const UserInfo& user_info, const RequestCallback& callback) { base::DictionaryValue output; - output.Set(kTraitsKey, cloud_->GetTraits().DeepCopy()); + output.Set(kTraitsKey, cloud_->GetTraits().CreateDeepCopy()); output.SetString(kFingerprintKey, std::to_string(traits_fingerprint_)); callback.Run(http::kOk, output); @@ -904,12 +911,13 @@ void PrivetHandler::HandleComponents(const base::DictionaryValue& input, components.reset(new base::DictionaryValue); // Get the last element of the path and use it as a dictionary key here. auto parts = Split(path, ".", true, false); - components->Set(parts.back(), CloneComponent(*component, filter).release()); + components->Set(parts.back(), CloneComponent(*component, filter)); } else { - components = CloneComponentTree(cloud_->GetComponents(), filter); + components = + CloneComponentTree(*cloud_->GetComponentsForUser(user_info), filter); } base::DictionaryValue output; - output.Set(kComponentsKey, components.release()); + output.Set(kComponentsKey, std::move(components)); output.SetString(kFingerprintKey, std::to_string(components_fingerprint_)); callback.Run(http::kOk, output); diff --git a/src/privet/privet_handler.h b/src/privet/privet_handler.h index 4eba329..7b212e4 100644 --- a/src/privet/privet_handler.h +++ b/src/privet/privet_handler.h @@ -11,7 +11,6 @@ #include <base/macros.h> #include <base/memory/weak_ptr.h> -#include <base/scoped_observer.h> #include <base/time/default_clock.h> #include "src/privet/cloud_delegate.h" @@ -31,7 +30,7 @@ class WifiDelegate; // Privet V3 HTTP/HTTPS requests handler. // API details at https://developers.google.com/cloud-devices/ -class PrivetHandler : public CloudDelegate::Observer { +class PrivetHandler { public: // Callback to handle requests asynchronously. // |status| is HTTP status code. @@ -45,11 +44,7 @@ class PrivetHandler : public CloudDelegate::Observer { SecurityDelegate* pairing, WifiDelegate* wifi, base::Clock* clock = nullptr); - ~PrivetHandler() override; - - void OnTraitDefsChanged() override; - void OnStateChanged() override; - void OnComponentTreeChanged() override; + ~PrivetHandler(); std::vector<std::string> GetHttpPaths() const; std::vector<std::string> GetHttpsPaths() const; @@ -138,6 +133,10 @@ class PrivetHandler : public CloudDelegate::Observer { void ReplyToUpdateRequest(const RequestCallback& callback) const; void OnUpdateRequestTimeout(int update_request_id); + void OnTraitDefsChanged(); + void OnStateChanged(); + void OnComponentTreeChanged(); + CloudDelegate* cloud_{nullptr}; DeviceDelegate* device_{nullptr}; SecurityDelegate* security_{nullptr}; @@ -165,7 +164,6 @@ class PrivetHandler : public CloudDelegate::Observer { uint64_t state_fingerprint_{1}; uint64_t traits_fingerprint_{1}; uint64_t components_fingerprint_{1}; - ScopedObserver<CloudDelegate, CloudDelegate::Observer> cloud_observer_{this}; base::WeakPtrFactory<PrivetHandler> weak_ptr_factory_{this}; diff --git a/src/privet/privet_handler_unittest.cc b/src/privet/privet_handler_unittest.cc index ecf4797..0c9f158 100644 --- a/src/privet/privet_handler_unittest.cc +++ b/src/privet/privet_handler_unittest.cc @@ -41,8 +41,7 @@ void LoadTestJson(const std::string& test_json, std::string message; std::unique_ptr<base::Value> value( base::JSONReader::ReadAndReturnError(test_json, base::JSON_PARSE_RFC, - &error, &message) - .release()); + &error, &message)); EXPECT_TRUE(value.get()) << "\nError: " << message << "\n" << test_json; base::DictionaryValue* dictionary_ptr = nullptr; if (value->GetAsDictionary(&dictionary_ptr)) @@ -75,7 +74,7 @@ bool IsEqualError(const CodeWithReason& expected, std::unique_ptr<base::DictionaryValue> StripDebugErrorDetails( const std::string& path_to_error_object, const base::DictionaryValue& value) { - std::unique_ptr<base::DictionaryValue> result{value.DeepCopy()}; + auto result = value.CreateDeepCopy(); base::DictionaryValue* error_dict = nullptr; EXPECT_TRUE(result->GetDictionary(path_to_error_object, &error_dict)); scoped_ptr<base::Value> dummy; @@ -484,6 +483,40 @@ TEST_F(PrivetHandlerTest, AuthLocalHighScope) { HandleRequest("/privet/v3/auth", kInput)); } +TEST_F(PrivetHandlerTest, ComponentsForUser) { + auth_header_ = "Privet 123"; + const UserInfo kOwner{AuthScope::kOwner, TestUserId{"1"}}; + const UserInfo kManager{AuthScope::kManager, TestUserId{"2"}}; + const UserInfo kUser{AuthScope::kUser, TestUserId{"3"}}; + const UserInfo kViewer{AuthScope::kViewer, TestUserId{"4"}}; + const base::DictionaryValue components; + const std::string expected = R"({"components": {}, "fingerprint": "1"})"; + + EXPECT_CALL(security_, ParseAccessToken(_, _, _)) + .WillOnce(DoAll(SetArgPointee<1>(kOwner), Return(true))); + EXPECT_CALL(cloud_, MockGetComponentsForUser(kOwner)) + .WillOnce(ReturnRef(components)); + EXPECT_JSON_EQ(expected, HandleRequest("/privet/v3/components", "{}")); + + EXPECT_CALL(security_, ParseAccessToken(_, _, _)) + .WillOnce(DoAll(SetArgPointee<1>(kManager), Return(true))); + EXPECT_CALL(cloud_, MockGetComponentsForUser(kManager)) + .WillOnce(ReturnRef(components)); + EXPECT_JSON_EQ(expected, HandleRequest("/privet/v3/components", "{}")); + + EXPECT_CALL(security_, ParseAccessToken(_, _, _)) + .WillOnce(DoAll(SetArgPointee<1>(kUser), Return(true))); + EXPECT_CALL(cloud_, MockGetComponentsForUser(kUser)) + .WillOnce(ReturnRef(components)); + EXPECT_JSON_EQ(expected, HandleRequest("/privet/v3/components", "{}")); + + EXPECT_CALL(security_, ParseAccessToken(_, _, _)) + .WillOnce(DoAll(SetArgPointee<1>(kViewer), Return(true))); + EXPECT_CALL(cloud_, MockGetComponentsForUser(kViewer)) + .WillOnce(ReturnRef(components)); + EXPECT_JSON_EQ(expected, HandleRequest("/privet/v3/components", "{}")); +} + class PrivetHandlerTestWithAuth : public PrivetHandlerTest { public: void SetUp() override { @@ -780,7 +813,8 @@ TEST_F(PrivetHandlerTestWithAuth, ComponentsWithFiltersAndPaths) { base::DictionaryValue components; LoadTestJson(kComponents, &components); EXPECT_CALL(cloud_, FindComponent(_, _)).WillRepeatedly(Return(nullptr)); - EXPECT_CALL(cloud_, GetComponents()).WillRepeatedly(ReturnRef(components)); + EXPECT_CALL(cloud_, MockGetComponentsForUser(_)) + .WillRepeatedly(ReturnRef(components)); const char kExpected1[] = R"({ "components": { "comp1": { diff --git a/src/privet/privet_manager.cc b/src/privet/privet_manager.cc index 9c717ce..1858168 100644 --- a/src/privet/privet_manager.cc +++ b/src/privet/privet_manager.cc @@ -12,7 +12,6 @@ #include <base/json/json_reader.h> #include <base/json/json_writer.h> #include <base/memory/weak_ptr.h> -#include <base/scoped_observer.h> #include <base/strings/string_number_conversions.h> #include <base/values.h> #include <weave/provider/network.h> @@ -41,7 +40,17 @@ using provider::Wifi; Manager::Manager(TaskRunner* task_runner) : task_runner_{task_runner} {} -Manager::~Manager() {} +Manager::~Manager() { + if (privet_handler_) { + for (const auto& path : privet_handler_->GetHttpsPaths()) { + http_server_->RemoveHttpsRequestHandler(path); + } + + for (const auto& path : privet_handler_->GetHttpPaths()) { + http_server_->RemoveHttpRequestHandler(path); + } + } +} void Manager::Start(Network* network, DnsServiceDiscovery* dns_sd, @@ -50,6 +59,8 @@ void Manager::Start(Network* network, AuthManager* auth_manager, DeviceRegistrationInfo* device, ComponentManager* component_manager) { + http_server_ = http_server; + CHECK(http_server_); CHECK(auth_manager); CHECK(device); @@ -58,7 +69,6 @@ void Manager::Start(Network* network, http_server->GetRequestTimeout()); cloud_ = CloudDelegate::CreateDefault(task_runner_, device, component_manager); - cloud_observer_.Add(cloud_.get()); security_.reset(new SecurityManager(device->GetMutableConfig(), auth_manager, task_runner_)); @@ -92,6 +102,9 @@ void Manager::Start(Network* network, path, base::Bind(&Manager::PrivetRequestHandler, weak_ptr_factory_.GetWeakPtr())); } + + device->GetMutableConfig()->AddOnChangedCallback(base::Bind( + &Manager::OnDeviceInfoChanged, weak_ptr_factory_.GetWeakPtr())); } std::string Manager::GetCurrentlyConnectedSsid() const { @@ -106,7 +119,7 @@ void Manager::AddOnPairingChangedCallbacks( security_->RegisterPairingListeners(on_start, on_end); } -void Manager::OnDeviceInfoChanged() { +void Manager::OnDeviceInfoChanged(const weave::Settings&) { OnChanged(); } diff --git a/src/privet/privet_manager.h b/src/privet/privet_manager.h index 06eb89a..0d95f7a 100644 --- a/src/privet/privet_manager.h +++ b/src/privet/privet_manager.h @@ -11,7 +11,6 @@ #include <vector> #include <base/memory/weak_ptr.h> -#include <base/scoped_observer.h> #include <weave/device.h> #include "src/privet/cloud_delegate.h" @@ -41,10 +40,10 @@ class PrivetHandler; class Publisher; class SecurityManager; -class Manager : public CloudDelegate::Observer { +class Manager { public: explicit Manager(provider::TaskRunner* task_runner); - ~Manager() override; + ~Manager(); void Start(provider::Network* network, provider::DnsServiceDiscovery* dns_sd, @@ -61,8 +60,7 @@ class Manager : public CloudDelegate::Observer { const Device::PairingEndCallback& end_callback); private: - // CloudDelegate::Observer - void OnDeviceInfoChanged() override; + void OnDeviceInfoChanged(const weave::Settings&); void PrivetRequestHandler( std::unique_ptr<provider::HttpServer::Request> request); @@ -80,6 +78,7 @@ class Manager : public CloudDelegate::Observer { void OnConnectivityChanged(); provider::TaskRunner* task_runner_{nullptr}; + provider::HttpServer* http_server_{nullptr}; std::unique_ptr<CloudDelegate> cloud_; std::unique_ptr<DeviceDelegate> device_; std::unique_ptr<SecurityManager> security_; @@ -87,8 +86,6 @@ class Manager : public CloudDelegate::Observer { std::unique_ptr<Publisher> publisher_; std::unique_ptr<PrivetHandler> privet_handler_; - ScopedObserver<CloudDelegate, CloudDelegate::Observer> cloud_observer_{this}; - base::WeakPtrFactory<Manager> weak_ptr_factory_{this}; DISALLOW_COPY_AND_ASSIGN(Manager); }; diff --git a/src/privet/privet_types.h b/src/privet/privet_types.h index 0f51862..44be96f 100644 --- a/src/privet/privet_types.h +++ b/src/privet/privet_types.h @@ -62,6 +62,14 @@ class UserInfo { AuthScope scope() const { return scope_; } const UserAppId& id() const { return id_; } + bool operator==(const UserInfo& rhs) const { + return scope_ == rhs.scope_ && id_ == rhs.id_; + } + + bool operator!=(const UserInfo& rhs) const { + return scope_ != rhs.scope_ || id_ != rhs.id_; + } + private: AuthScope scope_; UserAppId id_; diff --git a/src/privet/publisher.cc b/src/privet/publisher.cc index 55c9fe4..db26b26 100644 --- a/src/privet/publisher.cc +++ b/src/privet/publisher.cc @@ -65,6 +65,10 @@ void Publisher::ExposeService() { {"flags=" + WifiSsidGenerator{cloud_, wifi_}.GenerateFlags()}, }; + if (device_->GetHttpsEnpoint().first > 0) + txt_record.emplace_back("https=" + + std::to_string(device_->GetHttpsEnpoint().first)); + if (!cloud_->GetCloudId().empty()) txt_record.emplace_back("gcd_id=" + cloud_->GetCloudId()); diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index 3b08613..3c11935 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc @@ -144,9 +144,11 @@ bool SecurityManager::CreateAccessTokenImpl( return disabled_mode(error); const base::TimeDelta kTtl = base::TimeDelta::FromSeconds(kAccessTokenExpirationSeconds); - return auth_manager_->CreateAccessTokenFromAuth( + bool result = auth_manager_->CreateAccessTokenFromAuth( auth_code, kTtl, access_token, access_token_scope, access_token_ttl, error); + *access_token_scope = std::min(*access_token_scope, desired_scope); + return result; } return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthMode, diff --git a/src/privet/security_manager_unittest.cc b/src/privet/security_manager_unittest.cc index f596de9..526921c 100644 --- a/src/privet/security_manager_unittest.cc +++ b/src/privet/security_manager_unittest.cc @@ -138,15 +138,15 @@ class SecurityManagerTest : public testing::Test { SecurityManagerConfigStore config_store_; Config config_{&config_store_}; AuthManager auth_manager_{ - {22, 47, 23, 77, 42, 98, 96, 25, 83, 16, 9, 14, 91, 44, 15, 75, 60, 62, - 10, 18, 82, 35, 88, 100, 30, 45, 7, 46, 67, 84, 58, 85}, + {22, 47, 23, 77, 42, 98, 96, 25, 83, 16, 9, 14, 91, 44, 15, 75, + 60, 62, 10, 18, 82, 35, 88, 100, 30, 45, 7, 46, 67, 84, 58, 85}, { - 59, 47, 77, 247, 129, 187, 188, 158, 172, 105, 246, 93, 102, 83, 8, - 138, 176, 141, 37, 63, 223, 40, 153, 121, 134, 23, 120, 106, 24, 205, - 7, 135, + 59, 47, 77, 247, 129, 187, 188, 158, 172, 105, 246, + 93, 102, 83, 8, 138, 176, 141, 37, 63, 223, 40, + 153, 121, 134, 23, 120, 106, 24, 205, 7, 135, }, - {22, 47, 23, 77, 42, 98, 96, 25, 83, 16, 9, 14, 91, 44, 15, 75, 60, 62, - 10, 18, 82, 35, 88, 100, 30, 45, 7, 46, 67, 84, 58, 85}, + {22, 47, 23, 77, 42, 98, 96, 25, 83, 16, 9, 14, 91, 44, 15, 75, + 60, 62, 10, 18, 82, 35, 88, 100, 30, 45, 7, 46, 67, 84, 58, 85}, &clock_}; SecurityManager security_{&config_, &auth_manager_, &task_runner_}; diff --git a/src/privet/wifi_bootstrap_manager.cc b/src/privet/wifi_bootstrap_manager.cc index ce2016a..3fe0da1 100644 --- a/src/privet/wifi_bootstrap_manager.cc +++ b/src/privet/wifi_bootstrap_manager.cc @@ -195,8 +195,7 @@ bool WifiBootstrapManager::ConfigureCredentials(const std::string& ssid, } std::string WifiBootstrapManager::GetCurrentlyConnectedSsid() const { - // TODO(vitalybuka): Get from shill, if possible. - return config_->GetSettings().last_configured_ssid; + return wifi_->GetConnectedSsid(); } std::string WifiBootstrapManager::GetHostedSsid() const { diff --git a/src/states/state_change_queue.cc b/src/states/state_change_queue.cc index effe7f3..dcb19df 100644 --- a/src/states/state_change_queue.cc +++ b/src/states/state_change_queue.cc @@ -21,7 +21,7 @@ bool StateChangeQueue::NotifyPropertiesUpdated( if (stored_changes) stored_changes->MergeDictionary(&changed_properties); else - stored_changes.reset(changed_properties.DeepCopy()); + stored_changes = changed_properties.CreateDeepCopy(); while (state_changes_.size() > max_queue_size_) { // Queue is full. diff --git a/src/test/mock_component_manager.h b/src/test/mock_component_manager.h index 2c1d695..de6ffee 100644 --- a/src/test/mock_component_manager.h +++ b/src/test/mock_component_manager.h @@ -65,12 +65,18 @@ class MockComponentManager : public ComponentManager { MOCK_CONST_METHOD1( FindCommandDefinition, const base::DictionaryValue*(const std::string& command_name)); - MOCK_CONST_METHOD3(GetMinimalRole, + MOCK_CONST_METHOD3(GetCommandMinimalRole, bool(const std::string& command_name, UserRole* minimal_role, ErrorPtr* error)); + MOCK_CONST_METHOD3(GetStateMinimalRole, + bool(const std::string& state_property_name, + UserRole* minimal_role, + ErrorPtr* error)); MOCK_CONST_METHOD0(GetTraits, const base::DictionaryValue&()); MOCK_CONST_METHOD0(GetComponents, const base::DictionaryValue&()); + MOCK_CONST_METHOD1(MockGetComponentsForUserRole, + base::DictionaryValue*(UserRole)); MOCK_METHOD3(SetStateProperties, bool(const std::string& component_path, const base::DictionaryValue& dict, @@ -118,6 +124,11 @@ class MockComponentManager : public ComponentManager { const base::Callback<void(UpdateID)>& callback) override { return Token{MockAddServerStateUpdatedCallback(callback)}; } + std::unique_ptr<base::DictionaryValue> GetComponentsForUserRole( + UserRole role) const override { + return std::unique_ptr<base::DictionaryValue>{ + MockGetComponentsForUserRole(role)}; + } }; } // namespace test diff --git a/src/test/unittest_utils.cc b/src/test/unittest_utils.cc index df8ccc0..effadae 100644 --- a/src/test/unittest_utils.cc +++ b/src/test/unittest_utils.cc @@ -4,6 +4,8 @@ #include <weave/test/unittest_utils.h> +#include <algorithm> + #include <base/json/json_reader.h> #include <base/json/json_writer.h> #include <base/logging.h> @@ -19,8 +21,7 @@ std::unique_ptr<base::Value> CreateValue(const std::string& json) { std::string message; std::unique_ptr<base::Value> value{ base::JSONReader::ReadAndReturnError(json2, base::JSON_PARSE_RFC, &error, - &message) - .release()}; + &message)}; CHECK(value) << "Failed to load JSON: " << message << ", " << json; return value; } @@ -34,12 +35,10 @@ std::string ValueToString(const base::Value& value) { std::unique_ptr<base::DictionaryValue> CreateDictionaryValue( const std::string& json) { - std::unique_ptr<base::Value> value = CreateValue(json); - base::DictionaryValue* dict = nullptr; - value->GetAsDictionary(&dict); + std::unique_ptr<base::DictionaryValue> dict = + base::DictionaryValue::From(CreateValue(json)); CHECK(dict) << "Value is not dictionary: " << json; - value.release(); - return std::unique_ptr<base::DictionaryValue>(dict); + return dict; } } // namespace test diff --git a/src/utils.cc b/src/utils.cc index 5d1c3e3..d74c81e 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -4,6 +4,8 @@ #include "src/utils.h" +#include <limits> + #include <base/bind_helpers.h> #include <base/json/json_reader.h> @@ -50,17 +52,12 @@ std::unique_ptr<base::DictionaryValue> LoadJsonDict( json_string.size(), error_message.c_str()); return result; } - base::DictionaryValue* dict_value = nullptr; - if (!value->GetAsDictionary(&dict_value)) { + result = base::DictionaryValue::From(std::move(value)); + if (!result) { Error::AddToPrintf(error, FROM_HERE, errors::json::kObjectExpected, "JSON string '%s' is not a JSON object", LimitString(json_string, kMaxStrLen).c_str()); - return result; - } else { - // |value| is now owned by |dict_value|. - base::IgnoreResult(value.release()); } - result.reset(dict_value); return result; } @@ -72,10 +69,14 @@ std::unique_ptr<base::DictionaryValue> ErrorInfoToJson(const Error& error) { } uint32_t ToJ2000Time(const base::Time& time) { - return std::max(time.ToTimeT(), kJ2000ToTimeT) - kJ2000ToTimeT; + return std::min<int64_t>( + std::numeric_limits<int32_t>::max(), + std::max<int64_t>(kJ2000ToTimeT, time.ToTimeT()) - kJ2000ToTimeT); } base::Time FromJ2000Time(uint32_t time) { + if (time >= static_cast<uint32_t>(std::numeric_limits<int32_t>::max())) + return base::Time::Max(); return base::Time::FromTimeT(time + kJ2000ToTimeT); } diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc index 3b28001..eb65149 100644 --- a/src/weave_unittest.cc +++ b/src/weave_unittest.cc @@ -263,6 +263,14 @@ class WeaveTest : public ::testing::Test { const provider::HttpServer::RequestHandlerCallback& cb) { https_handlers_[path_prefix] = cb; })); + EXPECT_CALL(http_server_, RemoveHttpRequestHandler(_)) + .WillRepeatedly(Invoke([this](const std::string& path_prefix) { + http_handlers_.erase(path_prefix); + })); + EXPECT_CALL(http_server_, RemoveHttpsRequestHandler(_)) + .WillRepeatedly(Invoke([this](const std::string& path_prefix) { + https_handlers_.erase(path_prefix); + })); } void InitDefaultExpectations() { @@ -396,14 +404,14 @@ TEST_F(WeaveBasicTest, Register) { auto draft = CreateDictionaryValue(kDeviceResource); auto response = CreateDictionaryValue(kRegistrationResponse); - response->Set("deviceDraft", draft->DeepCopy()); + response->Set("deviceDraft", draft->CreateDeepCopy()); ExpectRequest(HttpClient::Method::kPatch, "https://www.googleapis.com/weave/v1/registrationTickets/" "TICKET_ID?key=TEST_API_KEY", ValueToString(*response)); response = CreateDictionaryValue(kRegistrationFinalResponse); - response->Set("deviceDraft", draft->DeepCopy()); + response->Set("deviceDraft", draft->CreateDeepCopy()); ExpectRequest(HttpClient::Method::kPost, "https://www.googleapis.com/weave/v1/registrationTickets/" "TICKET_ID/finalize?key=TEST_API_KEY", @@ -12,10 +12,11 @@ TEST_ENV ?= ifeq (1, $(CLANG)) TEST_ENV += ASAN_SYMBOLIZER_PATH=$(shell which llvm-symbolizer-3.6) endif +TEST_ENV += $(QEMU) weave_test_obj_files := $(WEAVE_TEST_SRC_FILES:%.cc=out/$(BUILD_MODE)/%.o) -$(weave_test_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party/include/gtest/gtest.h +$(weave_test_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_$(BUILD_MODE)) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< @@ -24,7 +25,7 @@ out/$(BUILD_MODE)/libweave-test.a : $(weave_test_obj_files) weave_unittest_obj_files := $(WEAVE_UNITTEST_SRC_FILES:%.cc=out/$(BUILD_MODE)/%.o) -$(weave_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party/include/gtest/gtest.h +$(weave_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_TEST) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< @@ -34,9 +35,9 @@ out/$(BUILD_MODE)/libweave_testrunner : \ $(third_party_chromium_base_unittest_obj_files) \ out/$(BUILD_MODE)/libweave_common.a \ out/$(BUILD_MODE)/libweave-test.a \ - third_party/lib/gmock.a \ - third_party/lib/gtest.a - $(CXX) -o $@ $^ $(CFLAGS) -lcrypto -lexpat -lpthread -lrt -Lthird_party/lib + $(third_party_gtest_lib) \ + $(third_party_gmock_lib) + $(CXX) -o $@ $^ $(CFLAGS) -lcrypto -lexpat -lpthread -lrt test : out/$(BUILD_MODE)/libweave_testrunner $(TEST_ENV) $< $(TEST_FLAGS) @@ -46,7 +47,7 @@ test : out/$(BUILD_MODE)/libweave_testrunner weave_exports_unittest_obj_files := $(WEAVE_EXPORTS_UNITTEST_SRC_FILES:%.cc=out/$(BUILD_MODE)/%.o) -$(weave_exports_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party/include/gtest/gtest.h +$(weave_exports_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_TEST) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< @@ -55,9 +56,9 @@ out/$(BUILD_MODE)/libweave_exports_testrunner : \ out/$(BUILD_MODE)/libweave.so \ out/$(BUILD_MODE)/libweave-test.a \ out/$(BUILD_MODE)/src/test/weave_testrunner.o \ - third_party/lib/gmock.a \ - third_party/lib/gtest.a - $(CXX) -o $@ $^ $(CFLAGS) -lcrypto -lexpat -lpthread -lrt -Lthird_party/lib -Wl,-rpath=out/$(BUILD_MODE)/ + $(third_party_gtest_lib) \ + $(third_party_gmock_lib) + $(CXX) -o $@ $^ $(CFLAGS) -lcrypto -lexpat -lpthread -lrt -Wl,-rpath=out/$(BUILD_MODE)/ export-test : out/$(BUILD_MODE)/libweave_exports_testrunner $(TEST_ENV) $< $(TEST_FLAGS) diff --git a/third_party/chromium/base/bind.h b/third_party/chromium/base/bind.h index 770e457..46dbb91 100644 --- a/third_party/chromium/base/bind.h +++ b/third_party/chromium/base/bind.h @@ -6,7 +6,6 @@ #define BASE_BIND_H_ #include "base/bind_internal.h" -#include "base/callback_internal.h" // ----------------------------------------------------------------------------- // Usage documentation @@ -47,14 +46,27 @@ namespace base { +namespace internal { + +// Don't use Alias Template directly here to avoid a compile error on MSVC2013. +template <typename Functor, typename... Args> +struct MakeUnboundRunTypeImpl { + using Type = + typename BindState< + typename FunctorTraits<Functor>::RunnableType, + typename FunctorTraits<Functor>::RunType, + Args...>::UnboundRunType; +}; + +} // namespace internal + +template <typename Functor, typename... Args> +using MakeUnboundRunType = + typename internal::MakeUnboundRunTypeImpl<Functor, Args...>::Type; + template <typename Functor, typename... Args> -base::Callback< - typename internal::BindState< - typename internal::FunctorTraits<Functor>::RunnableType, - typename internal::FunctorTraits<Functor>::RunType, - typename internal::CallbackParamTraits<Args>::StorageType...> - ::UnboundRunType> -Bind(Functor functor, const Args&... args) { +base::Callback<MakeUnboundRunType<Functor, Args...>> +Bind(Functor functor, Args&&... args) { // Type aliases for how to store and run the functor. using RunnableType = typename internal::FunctorTraits<Functor>::RunnableType; using RunType = typename internal::FunctorTraits<Functor>::RunType; @@ -88,12 +100,11 @@ Bind(Functor functor, const Args&... args) { !internal::HasRefCountedParamAsRawPtr<is_method, Args...>::value, "a parameter is a refcounted type and needs scoped_refptr"); - using BindState = internal::BindState< - RunnableType, RunType, - typename internal::CallbackParamTraits<Args>::StorageType...>; + using BindState = internal::BindState<RunnableType, RunType, Args...>; return Callback<typename BindState::UnboundRunType>( - new BindState(internal::MakeRunnable(functor), args...)); + new BindState(internal::MakeRunnable(functor), + std::forward<Args>(args)...)); } } // namespace base diff --git a/third_party/chromium/base/bind_helpers.h b/third_party/chromium/base/bind_helpers.h index 23e2cc9..5a4524a 100644 --- a/third_party/chromium/base/bind_helpers.h +++ b/third_party/chromium/base/bind_helpers.h @@ -153,7 +153,6 @@ #include "base/callback.h" #include "base/memory/weak_ptr.h" -#include "base/template_util.h" #include "build/build_config.h" namespace base { @@ -381,7 +380,7 @@ class PassedWrapper { : is_valid_(true), scoper_(std::move(scoper)) {} PassedWrapper(const PassedWrapper& other) : is_valid_(other.is_valid_), scoper_(std::move(other.scoper_)) {} - T Pass() const { + T Take() const { CHECK(is_valid_); is_valid_ = false; return std::move(scoper_); @@ -394,98 +393,39 @@ class PassedWrapper { // Unwrap the stored parameters for the wrappers above. template <typename T> -struct UnwrapTraits { - using ForwardType = const T&; - static ForwardType Unwrap(const T& o) { return o; } -}; +const T& Unwrap(const T& o) { + return o; +} template <typename T> -struct UnwrapTraits<UnretainedWrapper<T> > { - using ForwardType = T*; - static ForwardType Unwrap(UnretainedWrapper<T> unretained) { - return unretained.get(); - } -}; +T* Unwrap(UnretainedWrapper<T> unretained) { + return unretained.get(); +} template <typename T> -struct UnwrapTraits<ConstRefWrapper<T> > { - using ForwardType = const T&; - static ForwardType Unwrap(ConstRefWrapper<T> const_ref) { - return const_ref.get(); - } -}; +const T& Unwrap(ConstRefWrapper<T> const_ref) { + return const_ref.get(); +} template <typename T> -struct UnwrapTraits<scoped_refptr<T> > { - using ForwardType = T*; - static ForwardType Unwrap(const scoped_refptr<T>& o) { return o.get(); } -}; +T* Unwrap(const scoped_refptr<T>& o) { + return o.get(); +} template <typename T> -struct UnwrapTraits<WeakPtr<T> > { - using ForwardType = const WeakPtr<T>&; - static ForwardType Unwrap(const WeakPtr<T>& o) { return o; } -}; +const WeakPtr<T>& Unwrap(const WeakPtr<T>& o) { + return o; +} template <typename T> -struct UnwrapTraits<OwnedWrapper<T> > { - using ForwardType = T*; - static ForwardType Unwrap(const OwnedWrapper<T>& o) { - return o.get(); - } -}; +T* Unwrap(const OwnedWrapper<T>& o) { + return o.get(); +} template <typename T> -struct UnwrapTraits<PassedWrapper<T> > { - using ForwardType = T; - static T Unwrap(PassedWrapper<T>& o) { - return o.Pass(); - } -}; - -// Utility for handling different refcounting semantics in the Bind() -// function. -template <bool is_method, typename... T> -struct MaybeScopedRefPtr; - -template <bool is_method> -struct MaybeScopedRefPtr<is_method> { - MaybeScopedRefPtr() {} -}; - -template <typename T, typename... Rest> -struct MaybeScopedRefPtr<false, T, Rest...> { - MaybeScopedRefPtr(const T&, const Rest&...) {} -}; - -template <typename T, size_t n, typename... Rest> -struct MaybeScopedRefPtr<false, T[n], Rest...> { - MaybeScopedRefPtr(const T*, const Rest&...) {} -}; - -template <typename T, typename... Rest> -struct MaybeScopedRefPtr<true, T, Rest...> { - MaybeScopedRefPtr(const T& /* o */, const Rest&...) {} -}; - -template <typename T, typename... Rest> -struct MaybeScopedRefPtr<true, T*, Rest...> { - MaybeScopedRefPtr(T* o, const Rest&...) : ref_(o) {} - scoped_refptr<T> ref_; -}; - -// No need to additionally AddRef() and Release() since we are storing a -// scoped_refptr<> inside the storage object already. -template <typename T, typename... Rest> -struct MaybeScopedRefPtr<true, scoped_refptr<T>, Rest...> { - MaybeScopedRefPtr(const scoped_refptr<T>&, const Rest&...) {} -}; - -template <typename T, typename... Rest> -struct MaybeScopedRefPtr<true, const T*, Rest...> { - MaybeScopedRefPtr(const T* o, const Rest&...) : ref_(o) {} - scoped_refptr<const T> ref_; -}; +T Unwrap(PassedWrapper<T>& o) { + return o.Take(); +} // IsWeakMethod is a helper that determine if we are binding a WeakPtr<> to a // method. It is used internally by Bind() to select the correct @@ -625,15 +565,12 @@ static inline internal::OwnedWrapper<T> Owned(T* o) { // Both versions of Passed() prevent T from being an lvalue reference. The first // via use of enable_if, and the second takes a T* which will not bind to T&. template <typename T, - typename std::enable_if<internal::IsMoveOnlyType<T>::value && - !std::is_lvalue_reference<T>::value>::type* = + typename std::enable_if<!std::is_lvalue_reference<T>::value>::type* = nullptr> static inline internal::PassedWrapper<T> Passed(T&& scoper) { return internal::PassedWrapper<T>(std::move(scoper)); } -template <typename T, - typename std::enable_if<internal::IsMoveOnlyType<T>::value>::type* = - nullptr> +template <typename T> static inline internal::PassedWrapper<T> Passed(T* scoper) { return internal::PassedWrapper<T>(std::move(*scoper)); } diff --git a/third_party/chromium/base/bind_internal.h b/third_party/chromium/base/bind_internal.h index 50c3e24..199467c 100644 --- a/third_party/chromium/base/bind_internal.h +++ b/third_party/chromium/base/bind_internal.h @@ -104,7 +104,8 @@ template <bool is_method, typename... Args> struct BindsArrayToFirstArg : std::false_type {}; template <typename T, typename... Args> -struct BindsArrayToFirstArg<true, T, Args...> : std::is_array<T> {}; +struct BindsArrayToFirstArg<true, T, Args...> + : std::is_array<typename std::remove_reference<T>::type> {}; // HasRefCountedParamAsRawPtr is the same to HasRefCountedTypeAsRawPtr except // when |is_method| is true HasRefCountedParamAsRawPtr skips the first argument. @@ -153,8 +154,9 @@ class RunnableAdapter<R(*)(Args...)> { : function_(function) { } - R Run(typename CallbackParamTraits<Args>::ForwardType... args) { - return function_(CallbackForward(args)...); + template <typename... RunArgs> + R Run(RunArgs&&... args) { + return function_(std::forward<RunArgs>(args)...); } private: @@ -174,8 +176,9 @@ class RunnableAdapter<R(T::*)(Args...)> { : method_(method) { } - R Run(T* object, typename CallbackParamTraits<Args>::ForwardType... args) { - return (object->*method_)(CallbackForward(args)...); + template <typename... RunArgs> + R Run(T* object, RunArgs&&... args) { + return (object->*method_)(std::forward<RunArgs>(args)...); } private: @@ -193,9 +196,9 @@ class RunnableAdapter<R(T::*)(Args...) const> { : method_(method) { } - R Run(const T* object, - typename CallbackParamTraits<Args>::ForwardType... args) { - return (object->*method_)(CallbackForward(args)...); + template <typename... RunArgs> + R Run(const T* object, RunArgs&&... args) { + return (object->*method_)(std::forward<RunArgs>(args)...); } private: @@ -280,38 +283,42 @@ MakeRunnable(const Callback<T>& t) { // // WeakCalls similarly need special syntax that is applied to the first // argument to check if they should no-op themselves. -template <bool IsWeakCall, typename ReturnType, typename Runnable, - typename ArgsType> +template <bool IsWeakCall, typename ReturnType, typename Runnable> struct InvokeHelper; -template <typename ReturnType, typename Runnable, typename... Args> -struct InvokeHelper<false, ReturnType, Runnable, TypeList<Args...>> { - static ReturnType MakeItSo(Runnable runnable, Args... args) { - return runnable.Run(CallbackForward(args)...); +template <typename ReturnType, typename Runnable> +struct InvokeHelper<false, ReturnType, Runnable> { + template <typename... RunArgs> + static ReturnType MakeItSo(Runnable runnable, RunArgs&&... args) { + return runnable.Run(std::forward<RunArgs>(args)...); } }; -template <typename Runnable, typename... Args> -struct InvokeHelper<false, void, Runnable, TypeList<Args...>> { - static void MakeItSo(Runnable runnable, Args... args) { - runnable.Run(CallbackForward(args)...); +template <typename Runnable> +struct InvokeHelper<false, void, Runnable> { + template <typename... RunArgs> + static void MakeItSo(Runnable runnable, RunArgs&&... args) { + runnable.Run(std::forward<RunArgs>(args)...); } }; -template <typename Runnable, typename BoundWeakPtr, typename... Args> -struct InvokeHelper<true, void, Runnable, TypeList<BoundWeakPtr, Args...>> { - static void MakeItSo(Runnable runnable, BoundWeakPtr weak_ptr, Args... args) { +template <typename Runnable> +struct InvokeHelper<true, void, Runnable> { + template <typename BoundWeakPtr, typename... RunArgs> + static void MakeItSo(Runnable runnable, + BoundWeakPtr weak_ptr, + RunArgs&&... args) { if (!weak_ptr.get()) { return; } - runnable.Run(weak_ptr.get(), CallbackForward(args)...); + runnable.Run(weak_ptr.get(), std::forward<RunArgs>(args)...); } }; #if !defined(_MSC_VER) -template <typename ReturnType, typename Runnable, typename ArgsType> -struct InvokeHelper<true, ReturnType, Runnable, ArgsType> { +template <typename ReturnType, typename Runnable> +struct InvokeHelper<true, ReturnType, Runnable> { // WeakCalls are only supported for functions with a void return type. // Otherwise, the function result would be undefined if the the WeakPtr<> // is invalidated. @@ -324,33 +331,48 @@ struct InvokeHelper<true, ReturnType, Runnable, ArgsType> { // Invoker<> // // See description at the top of the file. -template <typename BoundIndices, - typename StorageType, typename Unwrappers, +template <typename BoundIndices, typename StorageType, typename InvokeHelperType, typename UnboundForwardRunType> struct Invoker; template <size_t... bound_indices, typename StorageType, - typename... Unwrappers, typename InvokeHelperType, typename R, - typename... UnboundForwardArgs> + typename... UnboundArgs> struct Invoker<IndexSequence<bound_indices...>, - StorageType, TypeList<Unwrappers...>, - InvokeHelperType, R(UnboundForwardArgs...)> { - static R Run(BindStateBase* base, - UnboundForwardArgs... unbound_args) { + StorageType, + InvokeHelperType, + R(UnboundArgs...)> { + static R Run(BindStateBase* base, UnboundArgs&&... unbound_args) { StorageType* storage = static_cast<StorageType*>(base); // Local references to make debugger stepping easier. If in a debugger, // you really want to warp ahead and step through the // InvokeHelper<>::MakeItSo() call below. return InvokeHelperType::MakeItSo( - storage->runnable_, - Unwrappers::Unwrap(get<bound_indices>(storage->bound_args_))..., - CallbackForward(unbound_args)...); + storage->runnable_, Unwrap(get<bound_indices>(storage->bound_args_))..., + std::forward<UnboundArgs>(unbound_args)...); } }; +// Used to implement MakeArgsStorage. +template <bool is_method, typename... BoundArgs> +struct MakeArgsStorageImpl { + using Type = std::tuple<BoundArgs...>; +}; + +template <typename Obj, typename... BoundArgs> +struct MakeArgsStorageImpl<true, Obj*, BoundArgs...> { + using Type = std::tuple<scoped_refptr<Obj>, BoundArgs...>; +}; + +// Constructs a tuple type to store BoundArgs into BindState. +// This wraps the first argument into a scoped_refptr if |is_method| is true and +// the first argument is a raw pointer. +// Other arguments are adjusted for store and packed into a tuple. +template <bool is_method, typename... BoundArgs> +using MakeArgsStorage = typename MakeArgsStorageImpl< + is_method, typename std::decay<BoundArgs>::type...>::Type; // BindState<> // @@ -376,40 +398,31 @@ struct BindState<Runnable, R(Args...), BoundArgs...> final using StorageType = BindState<Runnable, R(Args...), BoundArgs...>; using RunnableType = Runnable; + enum { is_method = HasIsMethodTag<Runnable>::value }; + // true_type if Runnable is a method invocation and the first bound argument // is a WeakPtr. using IsWeakCall = - IsWeakMethod<HasIsMethodTag<Runnable>::value, BoundArgs...>; + IsWeakMethod<is_method, typename std::decay<BoundArgs>::type...>; using BoundIndices = MakeIndexSequence<sizeof...(BoundArgs)>; - using Unwrappers = TypeList<UnwrapTraits<BoundArgs>...>; - using UnboundForwardArgs = DropTypeListItem< - sizeof...(BoundArgs), - TypeList<typename CallbackParamTraits<Args>::ForwardType...>>; - using UnboundForwardRunType = MakeFunctionType<R, UnboundForwardArgs>; - - using InvokeHelperArgs = ConcatTypeLists< - TypeList<typename UnwrapTraits<BoundArgs>::ForwardType...>, - UnboundForwardArgs>; - using InvokeHelperType = - InvokeHelper<IsWeakCall::value, R, Runnable, InvokeHelperArgs>; + using InvokeHelperType = InvokeHelper<IsWeakCall::value, R, Runnable>; using UnboundArgs = DropTypeListItem<sizeof...(BoundArgs), TypeList<Args...>>; public: - using InvokerType = Invoker<BoundIndices, StorageType, Unwrappers, - InvokeHelperType, UnboundForwardRunType>; using UnboundRunType = MakeFunctionType<R, UnboundArgs>; + using InvokerType = + Invoker<BoundIndices, StorageType, InvokeHelperType, UnboundRunType>; - BindState(const Runnable& runnable, const BoundArgs&... bound_args) + template <typename... ForwardArgs> + BindState(const Runnable& runnable, ForwardArgs&&... bound_args) : BindStateBase(&Destroy), runnable_(runnable), - ref_(bound_args...), - bound_args_(bound_args...) {} + bound_args_(std::forward<ForwardArgs>(bound_args)...) {} RunnableType runnable_; - MaybeScopedRefPtr<HasIsMethodTag<Runnable>::value, BoundArgs...> ref_; - Tuple<BoundArgs...> bound_args_; + MakeArgsStorage<is_method, BoundArgs...> bound_args_; private: ~BindState() {} diff --git a/third_party/chromium/base/bind_unittest.cc b/third_party/chromium/base/bind_unittest.cc index f3a1211..76d158b 100644 --- a/third_party/chromium/base/bind_unittest.cc +++ b/third_party/chromium/base/bind_unittest.cc @@ -6,6 +6,7 @@ #include <memory> #include <utility> +#include <vector> #include <gmock/gmock.h> #include <gtest/gtest.h> @@ -93,43 +94,87 @@ class NoRefChild : public NoRefParent { void NonVirtualSet() { value = kChildValue; } }; -// Used for probing the number of copies that occur if a type must be coerced -// during argument forwarding in the Run() methods. -struct DerivedCopyCounter { - DerivedCopyCounter(int* copies, int* assigns) - : copies_(copies), assigns_(assigns) { - } +// Used for probing the number of copies and moves that occur if a type must be +// coerced during argument forwarding in the Run() methods. +struct DerivedCopyMoveCounter { + DerivedCopyMoveCounter(int* copies, + int* assigns, + int* move_constructs, + int* move_assigns) + : copies_(copies), + assigns_(assigns), + move_constructs_(move_constructs), + move_assigns_(move_assigns) {} int* copies_; int* assigns_; + int* move_constructs_; + int* move_assigns_; }; -// Used for probing the number of copies in an argument. -class CopyCounter { +// Used for probing the number of copies and moves in an argument. +class CopyMoveCounter { public: - CopyCounter(int* copies, int* assigns) - : copies_(copies), assigns_(assigns) { + CopyMoveCounter(int* copies, + int* assigns, + int* move_constructs, + int* move_assigns) + : copies_(copies), + assigns_(assigns), + move_constructs_(move_constructs), + move_assigns_(move_assigns) {} + + CopyMoveCounter(const CopyMoveCounter& other) + : copies_(other.copies_), + assigns_(other.assigns_), + move_constructs_(other.move_constructs_), + move_assigns_(other.move_assigns_) { + (*copies_)++; } - CopyCounter(const CopyCounter& other) + CopyMoveCounter(CopyMoveCounter&& other) : copies_(other.copies_), - assigns_(other.assigns_) { - (*copies_)++; + assigns_(other.assigns_), + move_constructs_(other.move_constructs_), + move_assigns_(other.move_assigns_) { + (*move_constructs_)++; } // Probing for copies from coercion. - explicit CopyCounter(const DerivedCopyCounter& other) + explicit CopyMoveCounter(const DerivedCopyMoveCounter& other) : copies_(other.copies_), - assigns_(other.assigns_) { + assigns_(other.assigns_), + move_constructs_(other.move_constructs_), + move_assigns_(other.move_assigns_) { (*copies_)++; } - const CopyCounter& operator=(const CopyCounter& rhs) { + // Probing for moves from coercion. + explicit CopyMoveCounter(DerivedCopyMoveCounter&& other) + : copies_(other.copies_), + assigns_(other.assigns_), + move_constructs_(other.move_constructs_), + move_assigns_(other.move_assigns_) { + (*move_constructs_)++; + } + + const CopyMoveCounter& operator=(const CopyMoveCounter& rhs) { + copies_ = rhs.copies_; + assigns_ = rhs.assigns_; + move_constructs_ = rhs.move_constructs_; + move_assigns_ = rhs.move_assigns_; + + (*assigns_)++; + + return *this; + } + + const CopyMoveCounter& operator=(CopyMoveCounter&& rhs) { copies_ = rhs.copies_; assigns_ = rhs.assigns_; + move_constructs_ = rhs.move_constructs_; + move_assigns_ = rhs.move_assigns_; - if (assigns_) { - (*assigns_)++; - } + (*move_assigns_)++; return *this; } @@ -141,6 +186,47 @@ class CopyCounter { private: int* copies_; int* assigns_; + int* move_constructs_; + int* move_assigns_; +}; + +// Used for probing the number of copies in an argument. The instance is a +// copyable and non-movable type. +class CopyCounter { + public: + CopyCounter(int* copies, int* assigns) + : counter_(copies, assigns, nullptr, nullptr) {} + CopyCounter(const CopyCounter& other) : counter_(other.counter_) {} + CopyCounter& operator=(const CopyCounter& other) { + counter_ = other.counter_; + return *this; + } + + explicit CopyCounter(const DerivedCopyMoveCounter& other) : counter_(other) {} + + int copies() const { return counter_.copies(); } + + private: + CopyMoveCounter counter_; +}; + +// Used for probing the number of moves in an argument. The instance is a +// non-copyable and movable type. +class MoveCounter { + public: + MoveCounter(int* move_constructs, int* move_assigns) + : counter_(nullptr, nullptr, move_constructs, move_assigns) {} + MoveCounter(MoveCounter&& other) : counter_(std::move(other.counter_)) {} + MoveCounter& operator=(MoveCounter&& other) { + counter_ = std::move(other.counter_); + return *this; + } + + explicit MoveCounter(DerivedCopyMoveCounter&& other) + : counter_(std::move(other)) {} + + private: + CopyMoveCounter counter_; }; class DeleteCounter { @@ -191,7 +277,7 @@ const char* CStringIdentity(const char* s) { return s; } -int GetCopies(const CopyCounter& counter) { +int GetCopies(const CopyMoveCounter& counter) { return counter.copies(); } @@ -339,8 +425,8 @@ TEST_F(BindTest, CurryingRvalueResultOfBind) { // preserve virtual dispatch). TEST_F(BindTest, FunctionTypeSupport) { EXPECT_CALL(static_func_mock_, VoidMethod0()); - EXPECT_CALL(has_ref_, AddRef()).Times(5); - EXPECT_CALL(has_ref_, Release()).Times(5); + EXPECT_CALL(has_ref_, AddRef()).Times(4); + EXPECT_CALL(has_ref_, Release()).Times(4); EXPECT_CALL(has_ref_, VoidMethod0()).Times(2); EXPECT_CALL(has_ref_, VoidConstMethod0()).Times(2); @@ -670,12 +756,16 @@ TEST_F(BindTest, ConstRef) { int copies = 0; int assigns = 0; - CopyCounter counter(&copies, &assigns); + int move_constructs = 0; + int move_assigns = 0; + CopyMoveCounter counter(&copies, &assigns, &move_constructs, &move_assigns); Callback<int()> all_const_ref_cb = Bind(&GetCopies, ConstRef(counter)); EXPECT_EQ(0, all_const_ref_cb.Run()); EXPECT_EQ(0, copies); EXPECT_EQ(0, assigns); + EXPECT_EQ(0, move_constructs); + EXPECT_EQ(0, move_assigns); } TEST_F(BindTest, ScopedRefptr) { @@ -718,80 +808,57 @@ TEST_F(BindTest, Owned) { EXPECT_EQ(1, deletes); } -// Passed() wrapper support. +// Tests for Passed() wrapper support: // - Passed() can be constructed from a pointer to scoper. // - Passed() can be constructed from a scoper rvalue. // - Using Passed() gives Callback Ownership. // - Ownership is transferred from Callback to callee on the first Run(). // - Callback supports unbound arguments. -TEST_F(BindTest, ScopedPtr) { - int deletes = 0; +template <typename T> +class BindMoveOnlyTypeTest : public ::testing::Test { +}; - // Tests the Passed() function's support for pointers. - scoped_ptr<DeleteCounter> ptr(new DeleteCounter(&deletes)); - Callback<scoped_ptr<DeleteCounter>()> unused_callback = - Bind(&PassThru<scoped_ptr<DeleteCounter> >, Passed(&ptr)); - EXPECT_FALSE(ptr.get()); - EXPECT_EQ(0, deletes); +struct CustomDeleter { + void operator()(DeleteCounter* c) { delete c; } +}; - // If we never invoke the Callback, it retains ownership and deletes. - unused_callback.Reset(); - EXPECT_EQ(1, deletes); +using MoveOnlyTypesToTest = + ::testing::Types<scoped_ptr<DeleteCounter>, + std::unique_ptr<DeleteCounter>, + std::unique_ptr<DeleteCounter, CustomDeleter>>; +TYPED_TEST_CASE(BindMoveOnlyTypeTest, MoveOnlyTypesToTest); - // Tests the Passed() function's support for rvalues. - deletes = 0; - DeleteCounter* counter = new DeleteCounter(&deletes); - Callback<scoped_ptr<DeleteCounter>()> callback = - Bind(&PassThru<scoped_ptr<DeleteCounter> >, - Passed(scoped_ptr<DeleteCounter>(counter))); - EXPECT_FALSE(ptr.get()); - EXPECT_EQ(0, deletes); +TYPED_TEST(BindMoveOnlyTypeTest, PassedToBoundCallback) { + int deletes = 0; - // Check that ownership can be transferred back out. - scoped_ptr<DeleteCounter> result = callback.Run(); - ASSERT_EQ(counter, result.get()); + TypeParam ptr(new DeleteCounter(&deletes)); + Callback<TypeParam()> callback = Bind(&PassThru<TypeParam>, Passed(&ptr)); + EXPECT_FALSE(ptr.get()); EXPECT_EQ(0, deletes); - // Resetting does not delete since ownership was transferred. + // If we never invoke the Callback, it retains ownership and deletes. callback.Reset(); - EXPECT_EQ(0, deletes); - - // Ensure that we actually did get ownership. - result.reset(); EXPECT_EQ(1, deletes); - - // Test unbound argument forwarding. - Callback<scoped_ptr<DeleteCounter>(scoped_ptr<DeleteCounter>)> cb_unbound = - Bind(&PassThru<scoped_ptr<DeleteCounter> >); - ptr.reset(new DeleteCounter(&deletes)); - cb_unbound.Run(std::move(ptr)); } -TEST_F(BindTest, UniquePtr) { +TYPED_TEST(BindMoveOnlyTypeTest, PassedWithRvalue) { int deletes = 0; - - // Tests the Passed() function's support for pointers. - std::unique_ptr<DeleteCounter> ptr(new DeleteCounter(&deletes)); - Callback<std::unique_ptr<DeleteCounter>()> unused_callback = - Bind(&PassThru<std::unique_ptr<DeleteCounter>>, Passed(&ptr)); - EXPECT_FALSE(ptr.get()); + Callback<TypeParam()> callback = Bind( + &PassThru<TypeParam>, Passed(TypeParam(new DeleteCounter(&deletes)))); EXPECT_EQ(0, deletes); // If we never invoke the Callback, it retains ownership and deletes. - unused_callback.Reset(); + callback.Reset(); EXPECT_EQ(1, deletes); +} - // Tests the Passed() function's support for rvalues. - deletes = 0; +// Check that ownership can be transferred back out. +TYPED_TEST(BindMoveOnlyTypeTest, ReturnMoveOnlyType) { + int deletes = 0; DeleteCounter* counter = new DeleteCounter(&deletes); - Callback<std::unique_ptr<DeleteCounter>()> callback = - Bind(&PassThru<std::unique_ptr<DeleteCounter>>, - Passed(std::unique_ptr<DeleteCounter>(counter))); - EXPECT_FALSE(ptr.get()); - EXPECT_EQ(0, deletes); - - // Check that ownership can be transferred back out. - std::unique_ptr<DeleteCounter> result = callback.Run(); + Callback<TypeParam()> callback = + Bind(&PassThru<TypeParam>, Passed(TypeParam(counter))); + TypeParam result = callback.Run(); ASSERT_EQ(counter, result.get()); EXPECT_EQ(0, deletes); @@ -802,15 +869,49 @@ TEST_F(BindTest, UniquePtr) { // Ensure that we actually did get ownership. result.reset(); EXPECT_EQ(1, deletes); +} +TYPED_TEST(BindMoveOnlyTypeTest, UnboundForwarding) { + int deletes = 0; + TypeParam ptr(new DeleteCounter(&deletes)); // Test unbound argument forwarding. - Callback<std::unique_ptr<DeleteCounter>(std::unique_ptr<DeleteCounter>)> - cb_unbound = Bind(&PassThru<std::unique_ptr<DeleteCounter>>); - ptr.reset(new DeleteCounter(&deletes)); + Callback<TypeParam(TypeParam)> cb_unbound = Bind(&PassThru<TypeParam>); cb_unbound.Run(std::move(ptr)); + EXPECT_EQ(1, deletes); +} + +void VerifyVector(const std::vector<scoped_ptr<int>>& v) { + ASSERT_EQ(1u, v.size()); + EXPECT_EQ(12345, *v[0]); } -// Argument Copy-constructor usage for non-reference parameters. +std::vector<scoped_ptr<int>> AcceptAndReturnMoveOnlyVector( + std::vector<scoped_ptr<int>> v) { + VerifyVector(v); + return v; +} + +// Test that a vector containing move-only types can be used with Callback. +TEST_F(BindTest, BindMoveOnlyVector) { + using MoveOnlyVector = std::vector<scoped_ptr<int>>; + + MoveOnlyVector v; + v.push_back(make_scoped_ptr(new int(12345))); + + // Early binding should work: + base::Callback<MoveOnlyVector()> bound_cb = + base::Bind(&AcceptAndReturnMoveOnlyVector, Passed(&v)); + MoveOnlyVector intermediate_result = bound_cb.Run(); + VerifyVector(intermediate_result); + + // As should passing it as an argument to Run(): + base::Callback<MoveOnlyVector(MoveOnlyVector)> unbound_cb = + base::Bind(&AcceptAndReturnMoveOnlyVector); + MoveOnlyVector final_result = unbound_cb.Run(std::move(intermediate_result)); + VerifyVector(final_result); +} + +// Argument copy-constructor usage for non-reference copy-only parameters. // - Bound arguments are only copied once. // - Forwarded arguments are only copied once. // - Forwarded arguments with coercions are only copied twice (once for the @@ -820,28 +921,148 @@ TEST_F(BindTest, ArgumentCopies) { int assigns = 0; CopyCounter counter(&copies, &assigns); + Bind(&VoidPolymorphic<CopyCounter>::Run, counter); + EXPECT_EQ(1, copies); + EXPECT_EQ(0, assigns); + + copies = 0; + assigns = 0; + Bind(&VoidPolymorphic<CopyCounter>::Run, CopyCounter(&copies, &assigns)); + EXPECT_EQ(1, copies); + EXPECT_EQ(0, assigns); + + copies = 0; + assigns = 0; + Bind(&VoidPolymorphic<CopyCounter>::Run).Run(counter); + EXPECT_EQ(2, copies); + EXPECT_EQ(0, assigns); + + copies = 0; + assigns = 0; + Bind(&VoidPolymorphic<CopyCounter>::Run).Run(CopyCounter(&copies, &assigns)); + EXPECT_EQ(1, copies); + EXPECT_EQ(0, assigns); - Callback<void()> copy_cb = - Bind(&VoidPolymorphic<CopyCounter>::Run, counter); - EXPECT_GE(1, copies); + copies = 0; + assigns = 0; + DerivedCopyMoveCounter derived(&copies, &assigns, nullptr, nullptr); + Bind(&VoidPolymorphic<CopyCounter>::Run).Run(CopyCounter(derived)); + EXPECT_EQ(2, copies); EXPECT_EQ(0, assigns); copies = 0; assigns = 0; - Callback<void(CopyCounter)> forward_cb = - Bind(&VoidPolymorphic<CopyCounter>::Run); - forward_cb.Run(counter); - EXPECT_GE(1, copies); + Bind(&VoidPolymorphic<CopyCounter>::Run) + .Run(CopyCounter( + DerivedCopyMoveCounter(&copies, &assigns, nullptr, nullptr))); + EXPECT_EQ(2, copies); + EXPECT_EQ(0, assigns); +} + +// Argument move-constructor usage for move-only parameters. +// - Bound arguments passed by move are not copied. +TEST_F(BindTest, ArgumentMoves) { + int move_constructs = 0; + int move_assigns = 0; + + Bind(&VoidPolymorphic<const MoveCounter&>::Run, + MoveCounter(&move_constructs, &move_assigns)); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + // TODO(tzik): Support binding move-only type into a non-reference parameter + // of a variant of Callback. + + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<MoveCounter>::Run) + .Run(MoveCounter(&move_constructs, &move_assigns)); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<MoveCounter>::Run) + .Run(MoveCounter(DerivedCopyMoveCounter( + nullptr, nullptr, &move_constructs, &move_assigns))); + EXPECT_EQ(2, move_constructs); + EXPECT_EQ(0, move_assigns); +} + +// Argument constructor usage for non-reference movable-copyable +// parameters. +// - Bound arguments passed by move are not copied. +// - Forwarded arguments are only copied once. +// - Forwarded arguments with coercions are only copied once and moved once. +TEST_F(BindTest, ArgumentCopiesAndMoves) { + int copies = 0; + int assigns = 0; + int move_constructs = 0; + int move_assigns = 0; + + CopyMoveCounter counter(&copies, &assigns, &move_constructs, &move_assigns); + Bind(&VoidPolymorphic<CopyMoveCounter>::Run, counter); + EXPECT_EQ(1, copies); EXPECT_EQ(0, assigns); + EXPECT_EQ(0, move_constructs); + EXPECT_EQ(0, move_assigns); copies = 0; assigns = 0; - DerivedCopyCounter derived(&copies, &assigns); - Callback<void(CopyCounter)> coerce_cb = - Bind(&VoidPolymorphic<CopyCounter>::Run); - coerce_cb.Run(CopyCounter(derived)); - EXPECT_GE(2, copies); + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<CopyMoveCounter>::Run, + CopyMoveCounter(&copies, &assigns, &move_constructs, &move_assigns)); + EXPECT_EQ(0, copies); + EXPECT_EQ(0, assigns); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + copies = 0; + assigns = 0; + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<CopyMoveCounter>::Run).Run(counter); + EXPECT_EQ(1, copies); + EXPECT_EQ(0, assigns); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + copies = 0; + assigns = 0; + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<CopyMoveCounter>::Run) + .Run(CopyMoveCounter(&copies, &assigns, &move_constructs, &move_assigns)); + EXPECT_EQ(0, copies); + EXPECT_EQ(0, assigns); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + DerivedCopyMoveCounter derived_counter(&copies, &assigns, &move_constructs, + &move_assigns); + copies = 0; + assigns = 0; + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<CopyMoveCounter>::Run) + .Run(CopyMoveCounter(derived_counter)); + EXPECT_EQ(1, copies); + EXPECT_EQ(0, assigns); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + copies = 0; + assigns = 0; + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<CopyMoveCounter>::Run) + .Run(CopyMoveCounter(DerivedCopyMoveCounter( + &copies, &assigns, &move_constructs, &move_assigns))); + EXPECT_EQ(0, copies); EXPECT_EQ(0, assigns); + EXPECT_EQ(2, move_constructs); + EXPECT_EQ(0, move_assigns); } // Callback construction and assignment tests. diff --git a/third_party/chromium/base/callback.h b/third_party/chromium/base/callback.h index 3bf0008..c04e90d 100644 --- a/third_party/chromium/base/callback.h +++ b/third_party/chromium/base/callback.h @@ -7,7 +7,6 @@ #include "base/callback_forward.h" #include "base/callback_internal.h" -#include "base/template_util.h" // NOTE: Header files that do not require the full definition of Callback or // Closure should #include "base/callback_forward.h" instead of this file. @@ -341,63 +340,65 @@ // void Bar(char* ptr); // Bind(&Foo, "test"); // Bind(&Bar, "test"); // This fails because ptr is not const. - -namespace base { - -// First, we forward declare the Callback class template. This informs the -// compiler that the template only has 1 type parameter which is the function -// signature that the Callback is representing. -// -// After this, create template specializations for 0-7 parameters. Note that -// even though the template typelist grows, the specialization still -// only has one type: the function signature. // // If you are thinking of forward declaring Callback in your own header file, // please include "base/callback_forward.h" instead. +namespace base { namespace internal { template <typename Runnable, typename RunType, typename... BoundArgsType> struct BindState; } // namespace internal -template <typename R, typename... Args> -class Callback<R(Args...)> : public internal::CallbackBase { +template <typename R, typename... Args, internal::CopyMode copy_mode> +class Callback<R(Args...), copy_mode> + : public internal::CallbackBase<copy_mode> { public: // MSVC 2013 doesn't support Type Alias of function types. // Revisit this after we update it to newer version. typedef R RunType(Args...); - Callback() : CallbackBase(nullptr) { } + Callback() : internal::CallbackBase<copy_mode>(nullptr) {} - template <typename Runnable, typename BindRunType, typename... BoundArgsType> + template <typename Runnable, typename BindRunType, typename... BoundArgs> explicit Callback( - internal::BindState<Runnable, BindRunType, BoundArgsType...>* bind_state) - : CallbackBase(bind_state) { + internal::BindState<Runnable, BindRunType, BoundArgs...>* bind_state) + : internal::CallbackBase<copy_mode>(bind_state) { // Force the assignment to a local variable of PolymorphicInvoke // so the compiler will typecheck that the passed in Run() method has // the correct type. PolymorphicInvoke invoke_func = - &internal::BindState<Runnable, BindRunType, BoundArgsType...> + &internal::BindState<Runnable, BindRunType, BoundArgs...> ::InvokerType::Run; - polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func); + using InvokeFuncStorage = + typename internal::CallbackBase<copy_mode>::InvokeFuncStorage; + this->polymorphic_invoke_ = + reinterpret_cast<InvokeFuncStorage>(invoke_func); } bool Equals(const Callback& other) const { - return CallbackBase::Equals(other); + return this->EqualsInternal(other); } - R Run(typename internal::CallbackParamTraits<Args>::ForwardType... args) - const { + // Run() makes an extra copy compared to directly calling the bound function + // if an argument is passed-by-value and is copyable-but-not-movable: + // i.e. below copies CopyableNonMovableType twice. + // void F(CopyableNonMovableType) {} + // Bind(&F).Run(CopyableNonMovableType()); + // + // We can not fully apply Perfect Forwarding idiom to the callchain from + // Callback::Run() to the target function. Perfect Forwarding requires + // knowing how the caller will pass the arguments. However, the signature of + // InvokerType::Run() needs to be fixed in the callback constructor, so Run() + // cannot template its arguments based on how it's called. + R Run(Args... args) const { PolymorphicInvoke f = - reinterpret_cast<PolymorphicInvoke>(polymorphic_invoke_); - - return f(bind_state_.get(), internal::CallbackForward(args)...); + reinterpret_cast<PolymorphicInvoke>(this->polymorphic_invoke_); + return f(this->bind_state_.get(), std::forward<Args>(args)...); } private: - using PolymorphicInvoke = - R(*)(internal::BindStateBase*, - typename internal::CallbackParamTraits<Args>::ForwardType...); + using PolymorphicInvoke = R (*)(internal::BindStateBase*, Args&&...); }; } // namespace base diff --git a/third_party/chromium/base/callback_forward.h b/third_party/chromium/base/callback_forward.h index a9a263a..8b9b89c 100644 --- a/third_party/chromium/base/callback_forward.h +++ b/third_party/chromium/base/callback_forward.h @@ -6,8 +6,19 @@ #define BASE_CALLBACK_FORWARD_H_ namespace base { +namespace internal { -template <typename Sig> +// CopyMode is used to control the copyablity of a Callback. +// MoveOnly indicates the Callback is not copyable but movable, and Copyable +// indicates it is copyable and movable. +enum class CopyMode { + MoveOnly, Copyable, +}; + +} // namespace internal + +template <typename Signature, + internal::CopyMode copy_mode = internal::CopyMode::Copyable> class Callback; // Syntactic sugar to make Callback<void()> easier to declare since it diff --git a/third_party/chromium/base/callback_internal.cc b/third_party/chromium/base/callback_internal.cc index f0d16bb..932d36c 100644 --- a/third_party/chromium/base/callback_internal.cc +++ b/third_party/chromium/base/callback_internal.cc @@ -18,29 +18,66 @@ void BindStateBase::Release() { destructor_(this); } -CallbackBase::CallbackBase(const CallbackBase& c) = default; -CallbackBase& CallbackBase::operator=(const CallbackBase& c) = default; +CallbackBase<CopyMode::MoveOnly>::CallbackBase(CallbackBase&& c) + : bind_state_(std::move(c.bind_state_)), + polymorphic_invoke_(c.polymorphic_invoke_) { + c.polymorphic_invoke_ = nullptr; +} + +CallbackBase<CopyMode::MoveOnly>& +CallbackBase<CopyMode::MoveOnly>::operator=(CallbackBase&& c) { + bind_state_ = std::move(c.bind_state_); + polymorphic_invoke_ = c.polymorphic_invoke_; + c.polymorphic_invoke_ = nullptr; + return *this; +} -void CallbackBase::Reset() { - polymorphic_invoke_ = NULL; +void CallbackBase<CopyMode::MoveOnly>::Reset() { + polymorphic_invoke_ = nullptr; // NULL the bind_state_ last, since it may be holding the last ref to whatever // object owns us, and we may be deleted after that. - bind_state_ = NULL; + bind_state_ = nullptr; } -bool CallbackBase::Equals(const CallbackBase& other) const { +bool CallbackBase<CopyMode::MoveOnly>::EqualsInternal( + const CallbackBase& other) const { return bind_state_.get() == other.bind_state_.get() && polymorphic_invoke_ == other.polymorphic_invoke_; } -CallbackBase::CallbackBase(BindStateBase* bind_state) - : bind_state_(bind_state), - polymorphic_invoke_(NULL) { +CallbackBase<CopyMode::MoveOnly>::CallbackBase( + BindStateBase* bind_state) + : bind_state_(bind_state) { DCHECK(!bind_state_.get() || bind_state_->ref_count_ == 1); } -CallbackBase::~CallbackBase() { +CallbackBase<CopyMode::MoveOnly>::~CallbackBase() {} + +CallbackBase<CopyMode::Copyable>::CallbackBase( + const CallbackBase& c) + : CallbackBase<CopyMode::MoveOnly>(nullptr) { + bind_state_ = c.bind_state_; + polymorphic_invoke_ = c.polymorphic_invoke_; +} + +CallbackBase<CopyMode::Copyable>::CallbackBase(CallbackBase&& c) + : CallbackBase<CopyMode::MoveOnly>(std::move(c)) {} + +CallbackBase<CopyMode::Copyable>& +CallbackBase<CopyMode::Copyable>::operator=(const CallbackBase& c) { + bind_state_ = c.bind_state_; + polymorphic_invoke_ = c.polymorphic_invoke_; + return *this; } +CallbackBase<CopyMode::Copyable>& +CallbackBase<CopyMode::Copyable>::operator=(CallbackBase&& c) { + *static_cast<CallbackBase<CopyMode::MoveOnly>*>(this) = std::move(c); + return *this; +} + +template class CallbackBase<CopyMode::MoveOnly>; +template class CallbackBase<CopyMode::Copyable>; + } // namespace internal } // namespace base diff --git a/third_party/chromium/base/callback_internal.h b/third_party/chromium/base/callback_internal.h index 6339437..3682bf9 100644 --- a/third_party/chromium/base/callback_internal.h +++ b/third_party/chromium/base/callback_internal.h @@ -15,13 +15,14 @@ #include <vector> #include "base/base_export.h" +#include "base/callback_forward.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/template_util.h" namespace base { namespace internal { +template <CopyMode copy_mode> class CallbackBase; // BindStateBase is used to provide an opaque handle that the Callback @@ -43,6 +44,7 @@ class BindStateBase { private: friend class scoped_refptr<BindStateBase>; + template <CopyMode copy_mode> friend class CallbackBase; void AddRef(); @@ -58,10 +60,13 @@ class BindStateBase { // Holds the Callback methods that don't require specialization to reduce // template bloat. -class BASE_EXPORT CallbackBase { +// CallbackBase<MoveOnly> is a direct base class of MoveOnly callbacks, and +// CallbackBase<Copyable> uses CallbackBase<MoveOnly> for its implementation. +template <> +class BASE_EXPORT CallbackBase<CopyMode::MoveOnly> { public: - CallbackBase(const CallbackBase& c); - CallbackBase& operator=(const CallbackBase& c); + CallbackBase(CallbackBase&& c); + CallbackBase& operator=(CallbackBase&& c); // Returns true if Callback is null (doesn't refer to anything). bool is_null() const { return bind_state_.get() == NULL; } @@ -77,7 +82,7 @@ class BASE_EXPORT CallbackBase { using InvokeFuncStorage = void(*)(); // Returns true if this callback equals |other|. |other| may be null. - bool Equals(const CallbackBase& other) const; + bool EqualsInternal(const CallbackBase& other) const; // Allow initializing of |bind_state_| via the constructor to avoid default // initialization of the scoped_refptr. We do not also initialize @@ -91,9 +96,27 @@ class BASE_EXPORT CallbackBase { ~CallbackBase(); scoped_refptr<BindStateBase> bind_state_; - InvokeFuncStorage polymorphic_invoke_; + InvokeFuncStorage polymorphic_invoke_ = nullptr; +}; + +// CallbackBase<Copyable> is a direct base class of Copyable Callbacks. +template <> +class BASE_EXPORT CallbackBase<CopyMode::Copyable> + : public CallbackBase<CopyMode::MoveOnly> { + public: + CallbackBase(const CallbackBase& c); + CallbackBase(CallbackBase&& c); + CallbackBase& operator=(const CallbackBase& c); + CallbackBase& operator=(CallbackBase&& c); + protected: + explicit CallbackBase(BindStateBase* bind_state) + : CallbackBase<CopyMode::MoveOnly>(bind_state) {} + ~CallbackBase() {} }; +extern template class CallbackBase<CopyMode::MoveOnly>; +extern template class CallbackBase<CopyMode::Copyable>; + // A helper template to determine if given type is non-const move-only-type, // i.e. if a value of the given type should be passed via std::move() in a // destructive way. Types are considered to be move-only if they have a @@ -103,7 +126,13 @@ class BASE_EXPORT CallbackBase { // confuses template deduction in VS2013 with certain types such as // std::unique_ptr. // TODO(dcheng): Revisit this when Windows switches to VS2015 by default. + template <typename T> struct IsMoveOnlyType { + // Types YesType and NoType are guaranteed such that sizeof(YesType) < + // sizeof(NoType). + using YesType = char; + struct NoType { YesType dummy[2]; }; + template <typename U> static YesType Test(const typename U::MoveOnlyTypeForCPP03*); @@ -116,8 +145,14 @@ template <typename T> struct IsMoveOnlyType { // Specialization of IsMoveOnlyType so that std::unique_ptr is still considered // move-only, even without the sentinel member. -template <typename T> -struct IsMoveOnlyType<std::unique_ptr<T>> : std::true_type {}; +template <typename T, typename D> +struct IsMoveOnlyType<std::unique_ptr<T, D>> : std::true_type {}; + +// Specialization of std::vector, so that it's considered move-only if the +// element type is move-only. Allocator is explicitly ignored when determining +// move-only status of the std::vector. +template <typename T, typename Allocator> +struct IsMoveOnlyType<std::vector<T, Allocator>> : IsMoveOnlyType<T> {}; template <typename> struct CallbackParamTraitsForMoveOnlyType; @@ -130,16 +165,7 @@ struct CallbackParamTraitsForNonMoveOnlyType; // http://connect.microsoft.com/VisualStudio/feedbackdetail/view/957801/compilation-error-with-variadic-templates // // This is a typetraits object that's used to take an argument type, and -// extract a suitable type for storing and forwarding arguments. -// -// In particular, it strips off references, and converts arrays to -// pointers for storage; and it avoids accidentally trying to create a -// "reference of a reference" if the argument is a reference type. -// -// This array type becomes an issue for storage because we are passing bound -// parameters by const reference. In this case, we end up passing an actual -// array type in the initializer list which C++ does not allow. This will -// break passing of C-string literals. +// extract a suitable type for forwarding arguments. template <typename T> struct CallbackParamTraits : std::conditional<IsMoveOnlyType<T>::value, @@ -150,18 +176,6 @@ struct CallbackParamTraits template <typename T> struct CallbackParamTraitsForNonMoveOnlyType { using ForwardType = const T&; - using StorageType = T; -}; - -// The Storage should almost be impossible to trigger unless someone manually -// specifies type of the bind parameters. However, in case they do, -// this will guard against us accidentally storing a reference parameter. -// -// The ForwardType should only be used for unbound arguments. -template <typename T> -struct CallbackParamTraitsForNonMoveOnlyType<T&> { - using ForwardType = T&; - using StorageType = T; }; // Note that for array types, we implicitly add a const in the conversion. This @@ -172,14 +186,12 @@ struct CallbackParamTraitsForNonMoveOnlyType<T&> { template <typename T, size_t n> struct CallbackParamTraitsForNonMoveOnlyType<T[n]> { using ForwardType = const T*; - using StorageType = const T*; }; // See comment for CallbackParamTraits<T[n]>. template <typename T> struct CallbackParamTraitsForNonMoveOnlyType<T[]> { using ForwardType = const T*; - using StorageType = const T*; }; // Parameter traits for movable-but-not-copyable scopers. @@ -198,7 +210,6 @@ struct CallbackParamTraitsForNonMoveOnlyType<T[]> { template <typename T> struct CallbackParamTraitsForMoveOnlyType { using ForwardType = T; - using StorageType = T; }; // CallbackForward() is a very limited simulation of C++11's std::forward() diff --git a/third_party/chromium/base/logging.cc b/third_party/chromium/base/logging.cc index 6306f47..ab97073 100644 --- a/third_party/chromium/base/logging.cc +++ b/third_party/chromium/base/logging.cc @@ -233,7 +233,7 @@ void LogMessage::Init(const char* file, int line) { } void RawLog(int level, const char* message) { - if (level >= g_min_log_level) { + if (level >= g_min_log_level && message) { size_t bytes_written = 0; const size_t message_len = strlen(message); int rv; diff --git a/third_party/chromium/base/logging.h b/third_party/chromium/base/logging.h index ee10ae2..1290bfe 100644 --- a/third_party/chromium/base/logging.h +++ b/third_party/chromium/base/logging.h @@ -342,9 +342,6 @@ const LogSeverity LOG_DFATAL = LOG_FATAL; #define LOG_IF(severity, condition) \ LAZY_STREAM(LOG_STREAM(severity), LOG_IS_ON(severity) && (condition)) -#define SYSLOG(severity) LOG(severity) -#define SYSLOG_IF(severity, condition) LOG_IF(severity, condition) - // The VLOG macros log with negative verbosities. #define VLOG_STREAM(verbose_level) \ logging::LogMessage(__FILE__, __LINE__, -verbose_level).stream() @@ -358,8 +355,6 @@ const LogSeverity LOG_DFATAL = LOG_FATAL; #define LOG_ASSERT(condition) \ LOG_IF(FATAL, !(condition)) << "Assert failed: " #condition ". " -#define SYSLOG_ASSERT(condition) \ - SYSLOG_IF(FATAL, !(condition)) << "Assert failed: " #condition ". " // The actual stream used isn't important. #define EAT_STREAM_PARAMETERS \ @@ -656,12 +651,6 @@ class BASE_EXPORT LogMessage { DISALLOW_COPY_AND_ASSIGN(LogMessage); }; -// A non-macro interface to the log facility; (useful -// when the logging level is not a compile-time constant). -inline void LogAtLevel(int log_level, const std::string& msg) { - LogMessage(__FILE__, __LINE__, log_level).stream() << msg; -} - // This class is used to explicitly ignore values in the conditional // logging macros. This avoids compiler warnings like "value computed // is not used" and "statement has no effect". diff --git a/third_party/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h b/third_party/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h index 33bacba..5dbc183 100644 --- a/third_party/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h +++ b/third_party/chromium/base/memory/raw_scoped_refptr_mismatch_checker.h @@ -5,10 +5,10 @@ #ifndef BASE_MEMORY_RAW_SCOPED_REFPTR_MISMATCH_CHECKER_H_ #define BASE_MEMORY_RAW_SCOPED_REFPTR_MISMATCH_CHECKER_H_ +#include <tuple> +#include <type_traits> + #include "base/memory/ref_counted.h" -#include "base/template_util.h" -#include "base/tuple.h" -#include "build/build_config.h" // It is dangerous to post a task with a T* argument where T is a subtype of // RefCounted(Base|ThreadSafeBase), since by the time the parameter is used, the @@ -25,11 +25,6 @@ namespace internal { template <typename T> struct NeedsScopedRefptrButGetsRawPtr { -#if defined(OS_WIN) - enum { - value = base::false_type::value - }; -#else enum { // Human readable translation: you needed to be a scoped_refptr if you are a // raw pointer type and are convertible to a RefCounted(Base|ThreadSafeBase) @@ -38,7 +33,6 @@ struct NeedsScopedRefptrButGetsRawPtr { (std::is_convertible<T, subtle::RefCountedBase*>::value || std::is_convertible<T, subtle::RefCountedThreadSafeBase*>::value)) }; -#endif }; template <typename Params> @@ -47,14 +41,14 @@ struct ParamsUseScopedRefptrCorrectly { }; template <> -struct ParamsUseScopedRefptrCorrectly<Tuple<>> { +struct ParamsUseScopedRefptrCorrectly<std::tuple<>> { enum { value = 1 }; }; template <typename Head, typename... Tail> -struct ParamsUseScopedRefptrCorrectly<Tuple<Head, Tail...>> { +struct ParamsUseScopedRefptrCorrectly<std::tuple<Head, Tail...>> { enum { value = !NeedsScopedRefptrButGetsRawPtr<Head>::value && - ParamsUseScopedRefptrCorrectly<Tuple<Tail...>>::value }; + ParamsUseScopedRefptrCorrectly<std::tuple<Tail...>>::value }; }; } // namespace internal diff --git a/third_party/chromium/base/memory/ref_counted.h b/third_party/chromium/base/memory/ref_counted.h index 95fa565..a480eb0 100644 --- a/third_party/chromium/base/memory/ref_counted.h +++ b/third_party/chromium/base/memory/ref_counted.h @@ -347,15 +347,25 @@ class scoped_refptr { private: template <typename U> friend class scoped_refptr; - // Allow scoped_refptr<T> to be used in boolean expressions, but not - // implicitly convertible to a real bool (which is dangerous). + // Implement "Safe Bool Idiom" + // https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool // - // Note that this trick is only safe when the == and != operators - // are declared explicitly, as otherwise "refptr1 == refptr2" - // will compile but do the wrong thing (i.e., convert to Testable - // and then do the comparison). + // Allow scoped_refptr<T> to be used in boolean expressions such as + // if (ref_ptr_instance) + // But do not become convertible to a real bool (which is dangerous). + // Implementation requires: + // typedef Testable + // operator Testable() const + // operator== + // operator!= + // + // == and != operators must be declared explicitly or dissallowed, as + // otherwise "ptr1 == ptr2" will compile but do the wrong thing (i.e., convert + // to Testable and then do the comparison). + // + // C++11 provides for "explicit operator bool()", however it is currently + // banned due to MSVS2013. https://chromium-cpp.appspot.com/#core-blacklist typedef T* scoped_refptr::*Testable; - public: operator Testable() const { return ptr_ ? &scoped_refptr::ptr_ : nullptr; } @@ -403,8 +413,6 @@ scoped_refptr<T> make_scoped_refptr(T* t) { return scoped_refptr<T>(t); } -// Temporary operator overloads to facilitate the transition. See -// https://crbug.com/110610. template <typename T, typename U> bool operator==(const scoped_refptr<T>& lhs, const U* rhs) { return lhs.get() == rhs; diff --git a/third_party/chromium/base/memory/ref_counted_unittest.cc b/third_party/chromium/base/memory/ref_counted_unittest.cc index 88ee981..739b45f 100644 --- a/third_party/chromium/base/memory/ref_counted_unittest.cc +++ b/third_party/chromium/base/memory/ref_counted_unittest.cc @@ -139,10 +139,31 @@ TEST(RefCountedUnitTest, ScopedRefPtrToSelfMoveAssignment) { } TEST(RefCountedUnitTest, BooleanTesting) { - scoped_refptr<SelfAssign> p; - EXPECT_FALSE(p); - p = new SelfAssign; - EXPECT_TRUE(p); + scoped_refptr<SelfAssign> ptr_to_an_instance = new SelfAssign; + EXPECT_TRUE(ptr_to_an_instance); + EXPECT_FALSE(!ptr_to_an_instance); + + if (ptr_to_an_instance) { + } else { + ADD_FAILURE() << "Pointer to an instance should result in true."; + } + + if (!ptr_to_an_instance) { // check for operator!(). + ADD_FAILURE() << "Pointer to an instance should result in !x being false."; + } + + scoped_refptr<SelfAssign> null_ptr; + EXPECT_FALSE(null_ptr); + EXPECT_TRUE(!null_ptr); + + if (null_ptr) { + ADD_FAILURE() << "Null pointer should result in false."; + } + + if (!null_ptr) { // check for operator!(). + } else { + ADD_FAILURE() << "Null pointer should result in !x being true."; + } } TEST(RefCountedUnitTest, Equality) { diff --git a/third_party/chromium/base/memory/scoped_ptr.h b/third_party/chromium/base/memory/scoped_ptr.h index d68604e..2d2c0ec 100644 --- a/third_party/chromium/base/memory/scoped_ptr.h +++ b/third_party/chromium/base/memory/scoped_ptr.h @@ -33,6 +33,15 @@ // foo[10].Method(); // Foo::Method on the 10th element. // } // +// Scopers are testable as booleans: +// { +// scoped_ptr<Foo> foo; +// if (!foo) +// foo.reset(new Foo()); +// if (foo) +// LOG(INFO) << "This code is reached." +// } +// // These scopers also implement part of the functionality of C++11 unique_ptr // in that they are "movable but not copyable." You can use the scopers in // the parameter and return types of functions to signify ownership transfer @@ -81,7 +90,8 @@ // This is an implementation designed to match the anticipated future TR2 // implementation of the scoped_ptr class. -#include <assert.h> +// TODO(dcheng): Clean up these headers, but there are likely lots of existing +// IWYU violations. #include <stddef.h> #include <stdlib.h> @@ -91,17 +101,13 @@ #include <utility> #include "base/compiler_specific.h" +#include "base/logging.h" #include "base/macros.h" #include "base/move.h" -#include "base/template_util.h" +#include "build/build_config.h" namespace base { -namespace subtle { -class RefCountedBase; -class RefCountedThreadSafeBase; -} // namespace subtle - // Function object which invokes 'free' on its parameter, which must be // a pointer. Can be used to store malloc-allocated pointers in scoped_ptr: // @@ -113,485 +119,10 @@ struct FreeDeleter { } }; -namespace internal { - -template <typename T> struct IsNotRefCounted { - enum { - value = !std::is_convertible<T*, base::subtle::RefCountedBase*>::value && - !std::is_convertible<T*, base::subtle::RefCountedThreadSafeBase*>:: - value - }; -}; - -// Minimal implementation of the core logic of scoped_ptr, suitable for -// reuse in both scoped_ptr and its specializations. -template <class T, class D> -class scoped_ptr_impl { - public: - explicit scoped_ptr_impl(T* p) : data_(p) {} - - // Initializer for deleters that have data parameters. - scoped_ptr_impl(T* p, const D& d) : data_(p, d) {} - - // Templated constructor that destructively takes the value from another - // scoped_ptr_impl. - template <typename U, typename V> - scoped_ptr_impl(scoped_ptr_impl<U, V>* other) - : data_(other->release(), other->get_deleter()) { - // We do not support move-only deleters. We could modify our move - // emulation to have base::subtle::move() and base::subtle::forward() - // functions that are imperfect emulations of their C++11 equivalents, - // but until there's a requirement, just assume deleters are copyable. - } - - template <typename U, typename V> - void TakeState(scoped_ptr_impl<U, V>* other) { - // See comment in templated constructor above regarding lack of support - // for move-only deleters. - reset(other->release()); - get_deleter() = other->get_deleter(); - } - - ~scoped_ptr_impl() { - // Match libc++, which calls reset() in its destructor. - // Use nullptr as the new value for three reasons: - // 1. libc++ does it. - // 2. Avoids infinitely recursing into destructors if two classes are owned - // in a reference cycle (see ScopedPtrTest.ReferenceCycle). - // 3. If |this| is accessed in the future, in a use-after-free bug, attempts - // to dereference |this|'s pointer should cause either a failure or a - // segfault closer to the problem. If |this| wasn't reset to nullptr, - // the access would cause the deleted memory to be read or written - // leading to other more subtle issues. - reset(nullptr); - } - - void reset(T* p) { - // Match C++11's definition of unique_ptr::reset(), which requires changing - // the pointer before invoking the deleter on the old pointer. This prevents - // |this| from being accessed after the deleter is run, which may destroy - // |this|. - T* old = data_.ptr; - data_.ptr = p; - if (old != nullptr) - static_cast<D&>(data_)(old); - } - - T* get() const { return data_.ptr; } - - D& get_deleter() { return data_; } - const D& get_deleter() const { return data_; } - - void swap(scoped_ptr_impl& p2) { - // Standard swap idiom: 'using std::swap' ensures that std::swap is - // present in the overload set, but we call swap unqualified so that - // any more-specific overloads can be used, if available. - using std::swap; - swap(static_cast<D&>(data_), static_cast<D&>(p2.data_)); - swap(data_.ptr, p2.data_.ptr); - } - - T* release() { - T* old_ptr = data_.ptr; - data_.ptr = nullptr; - return old_ptr; - } - - private: - // Needed to allow type-converting constructor. - template <typename U, typename V> friend class scoped_ptr_impl; - - // Use the empty base class optimization to allow us to have a D - // member, while avoiding any space overhead for it when D is an - // empty class. See e.g. http://www.cantrip.org/emptyopt.html for a good - // discussion of this technique. - struct Data : public D { - explicit Data(T* ptr_in) : ptr(ptr_in) {} - Data(T* ptr_in, const D& other) : D(other), ptr(ptr_in) {} - T* ptr; - }; - - Data data_; - - DISALLOW_COPY_AND_ASSIGN(scoped_ptr_impl); -}; - -} // namespace internal - } // namespace base -// A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T> -// automatically deletes the pointer it holds (if any). -// That is, scoped_ptr<T> owns the T object that it points to. -// Like a T*, a scoped_ptr<T> may hold either nullptr or a pointer to a T -// object. Also like T*, scoped_ptr<T> is thread-compatible, and once you -// dereference it, you get the thread safety guarantees of T. -// -// The size of scoped_ptr is small. On most compilers, when using the -// std::default_delete, sizeof(scoped_ptr<T>) == sizeof(T*). Custom deleters -// will increase the size proportional to whatever state they need to have. See -// comments inside scoped_ptr_impl<> for details. -// -// Current implementation targets having a strict subset of C++11's -// unique_ptr<> features. Known deficiencies include not supporting move-only -// deleteres, function pointers as deleters, and deleters with reference -// types. -template <class T, class D = std::default_delete<T>> -class scoped_ptr { - DISALLOW_COPY_AND_ASSIGN_WITH_MOVE_FOR_BIND(scoped_ptr) - - static_assert(!std::is_array<T>::value, - "scoped_ptr doesn't support array with size"); - static_assert(base::internal::IsNotRefCounted<T>::value, - "T is a refcounted type and needs a scoped_refptr"); - - public: - // The element and deleter types. - using element_type = T; - using deleter_type = D; - - // Constructor. Defaults to initializing with nullptr. - scoped_ptr() : impl_(nullptr) {} - - // Constructor. Takes ownership of p. - explicit scoped_ptr(element_type* p) : impl_(p) {} - - // Constructor. Allows initialization of a stateful deleter. - scoped_ptr(element_type* p, const D& d) : impl_(p, d) {} - - // Constructor. Allows construction from a nullptr. - scoped_ptr(std::nullptr_t) : impl_(nullptr) {} - - // Move constructor. - // - // IMPLEMENTATION NOTE: Clang requires a move constructor to be defined (and - // not just the conversion constructor) in order to warn on pessimizing moves. - // The requirements for the move constructor are specified in C++11 - // 20.7.1.2.1.15-17, which has some subtleties around reference deleters. As - // we don't support reference (or move-only) deleters, the post conditions are - // trivially true: we always copy construct the deleter from other's deleter. - scoped_ptr(scoped_ptr&& other) : impl_(&other.impl_) {} - - // Conversion constructor. Allows construction from a scoped_ptr rvalue for a - // convertible type and deleter. - // - // IMPLEMENTATION NOTE: C++ 20.7.1.2.1.19 requires this constructor to only - // participate in overload resolution if all the following are true: - // - U is implicitly convertible to T: this is important for 2 reasons: - // 1. So type traits don't incorrectly return true, e.g. - // std::is_convertible<scoped_ptr<Base>, scoped_ptr<Derived>>::value - // should be false. - // 2. To make sure code like this compiles: - // void F(scoped_ptr<int>); - // void F(scoped_ptr<Base>); - // // Ambiguous since both conversion constructors match. - // F(scoped_ptr<Derived>()); - // - U is not an array type: to prevent conversions from scoped_ptr<T[]> to - // scoped_ptr<T>. - // - D is a reference type and E is the same type, or D is not a reference - // type and E is implicitly convertible to D: again, we don't support - // reference deleters, so we only worry about the latter requirement. - template <typename U, - typename E, - typename std::enable_if<!std::is_array<U>::value && - std::is_convertible<U*, T*>::value && - std::is_convertible<E, D>::value>::type* = - nullptr> - scoped_ptr(scoped_ptr<U, E>&& other) - : impl_(&other.impl_) {} - - // operator=. - // - // IMPLEMENTATION NOTE: Unlike the move constructor, Clang does not appear to - // require a move assignment operator to trigger the pessimizing move warning: - // in this case, the warning triggers when moving a temporary. For consistency - // with the move constructor, we define it anyway. C++11 20.7.1.2.3.1-3 - // defines several requirements around this: like the move constructor, the - // requirements are simplified by the fact that we don't support move-only or - // reference deleters. - scoped_ptr& operator=(scoped_ptr&& rhs) { - impl_.TakeState(&rhs.impl_); - return *this; - } - - // operator=. Allows assignment from a scoped_ptr rvalue for a convertible - // type and deleter. - // - // IMPLEMENTATION NOTE: C++11 unique_ptr<> keeps this operator= distinct from - // the normal move assignment operator. C++11 20.7.1.2.3.4-7 contains the - // requirement for this operator, but like the conversion constructor, the - // requirements are greatly simplified by not supporting move-only or - // reference deleters. - template <typename U, - typename E, - typename std::enable_if<!std::is_array<U>::value && - std::is_convertible<U*, T*>::value && - // Note that this really should be - // std::is_assignable, but <type_traits> - // appears to be missing this on some - // platforms. This is close enough (though - // it's not the same). - std::is_convertible<D, E>::value>::type* = - nullptr> - scoped_ptr& operator=(scoped_ptr<U, E>&& rhs) { - impl_.TakeState(&rhs.impl_); - return *this; - } - - // operator=. Allows assignment from a nullptr. Deletes the currently owned - // object, if any. - scoped_ptr& operator=(std::nullptr_t) { - reset(); - return *this; - } - - // Reset. Deletes the currently owned object, if any. - // Then takes ownership of a new object, if given. - void reset(element_type* p = nullptr) { impl_.reset(p); } - - // Accessors to get the owned object. - // operator* and operator-> will assert() if there is no current object. - element_type& operator*() const { - assert(impl_.get() != nullptr); - return *impl_.get(); - } - element_type* operator->() const { - assert(impl_.get() != nullptr); - return impl_.get(); - } - element_type* get() const { return impl_.get(); } - - // Access to the deleter. - deleter_type& get_deleter() { return impl_.get_deleter(); } - const deleter_type& get_deleter() const { return impl_.get_deleter(); } - - // Allow scoped_ptr<element_type> to be used in boolean expressions, but not - // implicitly convertible to a real bool (which is dangerous). - // - // Note that this trick is only safe when the == and != operators - // are declared explicitly, as otherwise "scoped_ptr1 == - // scoped_ptr2" will compile but do the wrong thing (i.e., convert - // to Testable and then do the comparison). - private: - typedef base::internal::scoped_ptr_impl<element_type, deleter_type> - scoped_ptr::*Testable; - - public: - operator Testable() const { - return impl_.get() ? &scoped_ptr::impl_ : nullptr; - } - - // Swap two scoped pointers. - void swap(scoped_ptr& p2) { - impl_.swap(p2.impl_); - } - - // Release a pointer. - // The return value is the current pointer held by this object. If this object - // holds a nullptr, the return value is nullptr. After this operation, this - // object will hold a nullptr, and will not own the object any more. - element_type* release() WARN_UNUSED_RESULT { - return impl_.release(); - } - - private: - // Needed to reach into |impl_| in the constructor. - template <typename U, typename V> friend class scoped_ptr; - base::internal::scoped_ptr_impl<element_type, deleter_type> impl_; - - // Forbidden for API compatibility with std::unique_ptr. - explicit scoped_ptr(int disallow_construction_from_null); -}; - -template <class T, class D> -class scoped_ptr<T[], D> { - DISALLOW_COPY_AND_ASSIGN_WITH_MOVE_FOR_BIND(scoped_ptr) - - public: - // The element and deleter types. - using element_type = T; - using deleter_type = D; - - // Constructor. Defaults to initializing with nullptr. - scoped_ptr() : impl_(nullptr) {} - - // Constructor. Stores the given array. Note that the argument's type - // must exactly match T*. In particular: - // - it cannot be a pointer to a type derived from T, because it is - // inherently unsafe in the general case to access an array through a - // pointer whose dynamic type does not match its static type (eg., if - // T and the derived types had different sizes access would be - // incorrectly calculated). Deletion is also always undefined - // (C++98 [expr.delete]p3). If you're doing this, fix your code. - // - it cannot be const-qualified differently from T per unique_ptr spec - // (http://cplusplus.github.com/LWG/lwg-active.html#2118). Users wanting - // to work around this may use const_cast<const T*>(). - explicit scoped_ptr(element_type* array) : impl_(array) {} - - // Constructor. Allows construction from a nullptr. - scoped_ptr(std::nullptr_t) : impl_(nullptr) {} - - // Constructor. Allows construction from a scoped_ptr rvalue. - scoped_ptr(scoped_ptr&& other) : impl_(&other.impl_) {} - - // operator=. Allows assignment from a scoped_ptr rvalue. - scoped_ptr& operator=(scoped_ptr&& rhs) { - impl_.TakeState(&rhs.impl_); - return *this; - } - - // operator=. Allows assignment from a nullptr. Deletes the currently owned - // array, if any. - scoped_ptr& operator=(std::nullptr_t) { - reset(); - return *this; - } - - // Reset. Deletes the currently owned array, if any. - // Then takes ownership of a new object, if given. - void reset(element_type* array = nullptr) { impl_.reset(array); } - - // Accessors to get the owned array. - element_type& operator[](size_t i) const { - assert(impl_.get() != nullptr); - return impl_.get()[i]; - } - element_type* get() const { return impl_.get(); } - - // Access to the deleter. - deleter_type& get_deleter() { return impl_.get_deleter(); } - const deleter_type& get_deleter() const { return impl_.get_deleter(); } - - // Allow scoped_ptr<element_type> to be used in boolean expressions, but not - // implicitly convertible to a real bool (which is dangerous). - private: - typedef base::internal::scoped_ptr_impl<element_type, deleter_type> - scoped_ptr::*Testable; - - public: - operator Testable() const { - return impl_.get() ? &scoped_ptr::impl_ : nullptr; - } - - // Swap two scoped pointers. - void swap(scoped_ptr& p2) { - impl_.swap(p2.impl_); - } - - // Release a pointer. - // The return value is the current pointer held by this object. If this object - // holds a nullptr, the return value is nullptr. After this operation, this - // object will hold a nullptr, and will not own the object any more. - element_type* release() WARN_UNUSED_RESULT { - return impl_.release(); - } - - private: - // Force element_type to be a complete type. - enum { type_must_be_complete = sizeof(element_type) }; - - // Actually hold the data. - base::internal::scoped_ptr_impl<element_type, deleter_type> impl_; - - // Disable initialization from any type other than element_type*, by - // providing a constructor that matches such an initialization, but is - // private and has no definition. This is disabled because it is not safe to - // call delete[] on an array whose static type does not match its dynamic - // type. - template <typename U> explicit scoped_ptr(U* array); - explicit scoped_ptr(int disallow_construction_from_null); - - // Disable reset() from any type other than element_type*, for the same - // reasons as the constructor above. - template <typename U> void reset(U* array); - void reset(int disallow_reset_from_null); -}; - -// Free functions -template <class T, class D> -void swap(scoped_ptr<T, D>& p1, scoped_ptr<T, D>& p2) { - p1.swap(p2); -} - -template <class T1, class D1, class T2, class D2> -bool operator==(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) { - return p1.get() == p2.get(); -} -template <class T, class D> -bool operator==(const scoped_ptr<T, D>& p, std::nullptr_t) { - return p.get() == nullptr; -} -template <class T, class D> -bool operator==(std::nullptr_t, const scoped_ptr<T, D>& p) { - return p.get() == nullptr; -} - -template <class T1, class D1, class T2, class D2> -bool operator!=(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) { - return !(p1 == p2); -} -template <class T, class D> -bool operator!=(const scoped_ptr<T, D>& p, std::nullptr_t) { - return !(p == nullptr); -} -template <class T, class D> -bool operator!=(std::nullptr_t, const scoped_ptr<T, D>& p) { - return !(p == nullptr); -} - -template <class T1, class D1, class T2, class D2> -bool operator<(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) { - return p1.get() < p2.get(); -} -template <class T, class D> -bool operator<(const scoped_ptr<T, D>& p, std::nullptr_t) { - auto* ptr = p.get(); - return ptr < static_cast<decltype(ptr)>(nullptr); -} -template <class T, class D> -bool operator<(std::nullptr_t, const scoped_ptr<T, D>& p) { - auto* ptr = p.get(); - return static_cast<decltype(ptr)>(nullptr) < ptr; -} - -template <class T1, class D1, class T2, class D2> -bool operator>(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) { - return p2 < p1; -} -template <class T, class D> -bool operator>(const scoped_ptr<T, D>& p, std::nullptr_t) { - return nullptr < p; -} -template <class T, class D> -bool operator>(std::nullptr_t, const scoped_ptr<T, D>& p) { - return p < nullptr; -} - -template <class T1, class D1, class T2, class D2> -bool operator<=(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) { - return !(p1 > p2); -} -template <class T, class D> -bool operator<=(const scoped_ptr<T, D>& p, std::nullptr_t) { - return !(p > nullptr); -} -template <class T, class D> -bool operator<=(std::nullptr_t, const scoped_ptr<T, D>& p) { - return !(nullptr > p); -} - -template <class T1, class D1, class T2, class D2> -bool operator>=(const scoped_ptr<T1, D1>& p1, const scoped_ptr<T2, D2>& p2) { - return !(p1 < p2); -} -template <class T, class D> -bool operator>=(const scoped_ptr<T, D>& p, std::nullptr_t) { - return !(p < nullptr); -} -template <class T, class D> -bool operator>=(std::nullptr_t, const scoped_ptr<T, D>& p) { - return !(nullptr < p); -} +template <typename T, typename D = std::default_delete<T>> +using scoped_ptr = std::unique_ptr<T, D>; // A function to convert T* into scoped_ptr<T> // Doing e.g. make_scoped_ptr(new FooBarBaz<type>(arg)) is a shorter notation @@ -601,9 +132,4 @@ scoped_ptr<T> make_scoped_ptr(T* ptr) { return scoped_ptr<T>(ptr); } -template <typename T> -std::ostream& operator<<(std::ostream& out, const scoped_ptr<T>& p) { - return out << p.get(); -} - #endif // BASE_MEMORY_SCOPED_PTR_H_ diff --git a/third_party/chromium/base/memory/scoped_ptr_unittest.cc b/third_party/chromium/base/memory/scoped_ptr_unittest.cc deleted file mode 100644 index c1eb469..0000000 --- a/third_party/chromium/base/memory/scoped_ptr_unittest.cc +++ /dev/null @@ -1,843 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/memory/scoped_ptr.h" - -#include <stddef.h> - -#include <sstream> - -#include <gtest/gtest.h> - -#include "base/bind.h" -#include "base/callback.h" -#include "base/macros.h" -#include "build/build_config.h" - -namespace { - -// Used to test depth subtyping. -class ConDecLoggerParent { - public: - virtual ~ConDecLoggerParent() {} - - virtual void SetPtr(int* ptr) = 0; - - virtual int SomeMeth(int x) const = 0; -}; - -class ConDecLogger : public ConDecLoggerParent { - public: - ConDecLogger() : ptr_(NULL) { } - explicit ConDecLogger(int* ptr) { SetPtr(ptr); } - ~ConDecLogger() override { --*ptr_; } - - void SetPtr(int* ptr) override { - ptr_ = ptr; - ++*ptr_; - } - - int SomeMeth(int x) const override { return x; } - - private: - int* ptr_; - - DISALLOW_COPY_AND_ASSIGN(ConDecLogger); -}; - -struct CountingDeleter { - explicit CountingDeleter(int* count) : count_(count) {} - inline void operator()(double* ptr) const { - (*count_)++; - } - int* count_; -}; - -// Used to test assignment of convertible deleters. -struct CountingDeleterChild : public CountingDeleter { - explicit CountingDeleterChild(int* count) : CountingDeleter(count) {} -}; - -class OverloadedNewAndDelete { - public: - void* operator new(size_t size) { - g_new_count++; - return malloc(size); - } - - void operator delete(void* ptr) { - g_delete_count++; - free(ptr); - } - - static void ResetCounters() { - g_new_count = 0; - g_delete_count = 0; - } - - static int new_count() { return g_new_count; } - static int delete_count() { return g_delete_count; } - - private: - static int g_new_count; - static int g_delete_count; -}; - -int OverloadedNewAndDelete::g_new_count = 0; -int OverloadedNewAndDelete::g_delete_count = 0; - -scoped_ptr<ConDecLogger> PassThru(scoped_ptr<ConDecLogger> logger) { - return logger; -} - -void GrabAndDrop(scoped_ptr<ConDecLogger> logger) { -} - -// Do not delete this function! It's existence is to test that you can -// return a temporarily constructed version of the scoper. -scoped_ptr<ConDecLogger> TestReturnOfType(int* constructed) { - return scoped_ptr<ConDecLogger>(new ConDecLogger(constructed)); -} - -} // namespace - -TEST(ScopedPtrTest, ScopedPtr) { - int constructed = 0; - - // Ensure size of scoped_ptr<> doesn't increase unexpectedly. - static_assert(sizeof(int*) >= sizeof(scoped_ptr<int>), - "scoped_ptr shouldn't be larger than the raw pointer"); - - { - scoped_ptr<ConDecLogger> scoper(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - EXPECT_EQ(10, scoper->SomeMeth(10)); - EXPECT_EQ(10, scoper.get()->SomeMeth(10)); - EXPECT_EQ(10, (*scoper).SomeMeth(10)); - } - EXPECT_EQ(0, constructed); - - // Test reset() and release() - { - scoped_ptr<ConDecLogger> scoper(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - scoper.reset(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - scoper.reset(); - EXPECT_EQ(0, constructed); - EXPECT_FALSE(scoper.get()); - - scoper.reset(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - ConDecLogger* take = scoper.release(); - EXPECT_EQ(1, constructed); - EXPECT_FALSE(scoper.get()); - delete take; - EXPECT_EQ(0, constructed); - - scoper.reset(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - } - EXPECT_EQ(0, constructed); - - // Test swap(). - { - scoped_ptr<ConDecLogger> scoper1; - scoped_ptr<ConDecLogger> scoper2; - EXPECT_TRUE(scoper1.get() == scoper2.get()); - EXPECT_FALSE(scoper1.get() != scoper2.get()); - - ConDecLogger* logger = new ConDecLogger(&constructed); - scoper1.reset(logger); - EXPECT_EQ(logger, scoper1.get()); - EXPECT_FALSE(scoper2.get()); - EXPECT_FALSE(scoper1.get() == scoper2.get()); - EXPECT_TRUE(scoper1.get() != scoper2.get()); - - scoper2.swap(scoper1); - EXPECT_EQ(logger, scoper2.get()); - EXPECT_FALSE(scoper1.get()); - EXPECT_FALSE(scoper1.get() == scoper2.get()); - EXPECT_TRUE(scoper1.get() != scoper2.get()); - } - EXPECT_EQ(0, constructed); -} - -TEST(ScopedPtrTest, ScopedPtrDepthSubtyping) { - int constructed = 0; - - // Test construction from a scoped_ptr to a derived class. - { - scoped_ptr<ConDecLogger> scoper(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - scoped_ptr<ConDecLoggerParent> scoper_parent(std::move(scoper)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper_parent.get()); - EXPECT_FALSE(scoper.get()); - - EXPECT_EQ(10, scoper_parent->SomeMeth(10)); - EXPECT_EQ(10, scoper_parent.get()->SomeMeth(10)); - EXPECT_EQ(10, (*scoper_parent).SomeMeth(10)); - } - EXPECT_EQ(0, constructed); - - // Test assignment from a scoped_ptr to a derived class. - { - scoped_ptr<ConDecLogger> scoper(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - scoped_ptr<ConDecLoggerParent> scoper_parent; - scoper_parent = std::move(scoper); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper_parent.get()); - EXPECT_FALSE(scoper.get()); - } - EXPECT_EQ(0, constructed); - - // Test construction of a scoped_ptr with an additional const annotation. - { - scoped_ptr<ConDecLogger> scoper(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - scoped_ptr<const ConDecLogger> scoper_const(std::move(scoper)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper_const.get()); - EXPECT_FALSE(scoper.get()); - - EXPECT_EQ(10, scoper_const->SomeMeth(10)); - EXPECT_EQ(10, scoper_const.get()->SomeMeth(10)); - EXPECT_EQ(10, (*scoper_const).SomeMeth(10)); - } - EXPECT_EQ(0, constructed); - - // Test assignment to a scoped_ptr with an additional const annotation. - { - scoped_ptr<ConDecLogger> scoper(new ConDecLogger(&constructed)); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper.get()); - - scoped_ptr<const ConDecLogger> scoper_const; - scoper_const = std::move(scoper); - EXPECT_EQ(1, constructed); - EXPECT_TRUE(scoper_const.get()); - EXPECT_FALSE(scoper.get()); - } - EXPECT_EQ(0, constructed); - - // Test assignment to a scoped_ptr deleter of parent type. - { - // Custom deleters never touch these value. - double dummy_value, dummy_value2; - int deletes = 0; - int alternate_deletes = 0; - scoped_ptr<double, CountingDeleter> scoper(&dummy_value, - CountingDeleter(&deletes)); - scoped_ptr<double, CountingDeleterChild> scoper_child( - &dummy_value2, CountingDeleterChild(&alternate_deletes)); - - EXPECT_TRUE(scoper); - EXPECT_TRUE(scoper_child); - EXPECT_EQ(0, deletes); - EXPECT_EQ(0, alternate_deletes); - - // Test this compiles and correctly overwrites the deleter state. - scoper = std::move(scoper_child); - EXPECT_TRUE(scoper); - EXPECT_FALSE(scoper_child); - EXPECT_EQ(1, deletes); - EXPECT_EQ(0, alternate_deletes); - - scoper.reset(); - EXPECT_FALSE(scoper); - EXPECT_FALSE(scoper_child); - EXPECT_EQ(1, deletes); - EXPECT_EQ(1, alternate_deletes); - - scoper_child.reset(&dummy_value); - EXPECT_TRUE(scoper_child); - EXPECT_EQ(1, deletes); - EXPECT_EQ(1, alternate_deletes); - scoped_ptr<double, CountingDeleter> scoper_construct( - std::move(scoper_child)); - EXPECT_TRUE(scoper_construct); - EXPECT_FALSE(scoper_child); - EXPECT_EQ(1, deletes); - EXPECT_EQ(1, alternate_deletes); - - scoper_construct.reset(); - EXPECT_EQ(1, deletes); - EXPECT_EQ(2, alternate_deletes); - } -} - -TEST(ScopedPtrTest, ScopedPtrWithArray) { - static const int kNumLoggers = 12; - - int constructed = 0; - - { - scoped_ptr<ConDecLogger[]> scoper(new ConDecLogger[kNumLoggers]); - EXPECT_TRUE(scoper); - EXPECT_EQ(&scoper[0], scoper.get()); - for (int i = 0; i < kNumLoggers; ++i) { - scoper[i].SetPtr(&constructed); - } - EXPECT_EQ(12, constructed); - - EXPECT_EQ(10, scoper.get()->SomeMeth(10)); - EXPECT_EQ(10, scoper[2].SomeMeth(10)); - } - EXPECT_EQ(0, constructed); - - // Test reset() and release() - { - scoped_ptr<ConDecLogger[]> scoper; - EXPECT_FALSE(scoper.get()); - EXPECT_FALSE(scoper.release()); - EXPECT_FALSE(scoper.get()); - scoper.reset(); - EXPECT_FALSE(scoper.get()); - - scoper.reset(new ConDecLogger[kNumLoggers]); - for (int i = 0; i < kNumLoggers; ++i) { - scoper[i].SetPtr(&constructed); - } - EXPECT_EQ(12, constructed); - scoper.reset(); - EXPECT_EQ(0, constructed); - - scoper.reset(new ConDecLogger[kNumLoggers]); - for (int i = 0; i < kNumLoggers; ++i) { - scoper[i].SetPtr(&constructed); - } - EXPECT_EQ(12, constructed); - ConDecLogger* ptr = scoper.release(); - EXPECT_EQ(12, constructed); - delete[] ptr; - EXPECT_EQ(0, constructed); - } - EXPECT_EQ(0, constructed); - - // Test swap() and type-safe Boolean. - { - scoped_ptr<ConDecLogger[]> scoper1; - scoped_ptr<ConDecLogger[]> scoper2; - EXPECT_TRUE(scoper1.get() == scoper2.get()); - EXPECT_FALSE(scoper1.get() != scoper2.get()); - - ConDecLogger* loggers = new ConDecLogger[kNumLoggers]; - for (int i = 0; i < kNumLoggers; ++i) { - loggers[i].SetPtr(&constructed); - } - scoper1.reset(loggers); - EXPECT_TRUE(scoper1); - EXPECT_EQ(loggers, scoper1.get()); - EXPECT_FALSE(scoper2); - EXPECT_FALSE(scoper2.get()); - EXPECT_FALSE(scoper1.get() == scoper2.get()); - EXPECT_TRUE(scoper1.get() != scoper2.get()); - - scoper2.swap(scoper1); - EXPECT_EQ(loggers, scoper2.get()); - EXPECT_FALSE(scoper1.get()); - EXPECT_FALSE(scoper1.get() == scoper2.get()); - EXPECT_TRUE(scoper1.get() != scoper2.get()); - } - EXPECT_EQ(0, constructed); - - { - ConDecLogger* loggers = new ConDecLogger[kNumLoggers]; - scoped_ptr<ConDecLogger[]> scoper(loggers); - EXPECT_TRUE(scoper); - for (int i = 0; i < kNumLoggers; ++i) { - scoper[i].SetPtr(&constructed); - } - EXPECT_EQ(kNumLoggers, constructed); - - // Test moving with constructor; - scoped_ptr<ConDecLogger[]> scoper2(std::move(scoper)); - EXPECT_EQ(kNumLoggers, constructed); - - // Test moving with assignment; - scoped_ptr<ConDecLogger[]> scoper3; - scoper3 = std::move(scoper2); - EXPECT_EQ(kNumLoggers, constructed); - EXPECT_FALSE(scoper); - EXPECT_FALSE(scoper2); - EXPECT_TRUE(scoper3); - } - EXPECT_EQ(0, constructed); -} - -TEST(ScopedPtrTest, MoveBehavior) { - int constructed = 0; - { - ConDecLogger* logger = new ConDecLogger(&constructed); - scoped_ptr<ConDecLogger> scoper(logger); - EXPECT_EQ(1, constructed); - - // Test moving with constructor; - scoped_ptr<ConDecLogger> scoper2(std::move(scoper)); - EXPECT_EQ(1, constructed); - - // Test moving with assignment; - scoped_ptr<ConDecLogger> scoper3; - scoper3 = std::move(scoper2); - EXPECT_EQ(1, constructed); - EXPECT_FALSE(scoper.get()); - EXPECT_FALSE(scoper2.get()); - EXPECT_TRUE(scoper3.get()); - } - -#if !defined(OS_ANDROID) && !defined(OS_LINUX) - // Test uncaught Pass() does not have side effects, because Pass() - // is implemented by std::move(). - // TODO(danakj): Remove this test case when we remove Pass(). - { - ConDecLogger* logger = new ConDecLogger(&constructed); - scoped_ptr<ConDecLogger> scoper(logger); - EXPECT_EQ(1, constructed); - - // Should auto-destruct logger by end of scope. - scoped_ptr<ConDecLogger>&& rvalue = scoper.Pass(); - // The Pass() function mimics std::move(), which does not have side-effects. - EXPECT_TRUE(scoper.get()); - EXPECT_TRUE(rvalue); - } - EXPECT_EQ(0, constructed); -#endif - - // Test that passing to function which does nothing does not leak. - { - ConDecLogger* logger = new ConDecLogger(&constructed); - scoped_ptr<ConDecLogger> scoper(logger); - EXPECT_EQ(1, constructed); - - // Should auto-destruct logger by end of scope. - GrabAndDrop(std::move(scoper)); - EXPECT_FALSE(scoper.get()); - } - EXPECT_EQ(0, constructed); -} - -TEST(ScopedPtrTest, ReturnTypeBehavior) { - int constructed = 0; - - // Test that we can return a scoped_ptr. - { - ConDecLogger* logger = new ConDecLogger(&constructed); - scoped_ptr<ConDecLogger> scoper(logger); - EXPECT_EQ(1, constructed); - - PassThru(std::move(scoper)); - EXPECT_FALSE(scoper.get()); - } - EXPECT_EQ(0, constructed); - - // Test uncaught return type not leak. - { - ConDecLogger* logger = new ConDecLogger(&constructed); - scoped_ptr<ConDecLogger> scoper(logger); - EXPECT_EQ(1, constructed); - - // Should auto-destruct logger by end of scope. - PassThru(std::move(scoper)); - EXPECT_FALSE(scoper.get()); - } - EXPECT_EQ(0, constructed); - - // Call TestReturnOfType() so the compiler doesn't warn for an unused - // function. - { - TestReturnOfType(&constructed); - } - EXPECT_EQ(0, constructed); -} - -TEST(ScopedPtrTest, CustomDeleter) { - double dummy_value; // Custom deleter never touches this value. - int deletes = 0; - int alternate_deletes = 0; - - // Normal delete support. - { - deletes = 0; - scoped_ptr<double, CountingDeleter> scoper(&dummy_value, - CountingDeleter(&deletes)); - EXPECT_EQ(0, deletes); - EXPECT_TRUE(scoper.get()); - } - EXPECT_EQ(1, deletes); - - // Test reset() and release(). - deletes = 0; - { - scoped_ptr<double, CountingDeleter> scoper(NULL, - CountingDeleter(&deletes)); - EXPECT_FALSE(scoper.get()); - EXPECT_FALSE(scoper.release()); - EXPECT_FALSE(scoper.get()); - scoper.reset(); - EXPECT_FALSE(scoper.get()); - EXPECT_EQ(0, deletes); - - scoper.reset(&dummy_value); - scoper.reset(); - EXPECT_EQ(1, deletes); - - scoper.reset(&dummy_value); - EXPECT_EQ(&dummy_value, scoper.release()); - } - EXPECT_EQ(1, deletes); - - // Test get_deleter(). - deletes = 0; - alternate_deletes = 0; - { - scoped_ptr<double, CountingDeleter> scoper(&dummy_value, - CountingDeleter(&deletes)); - // Call deleter manually. - EXPECT_EQ(0, deletes); - scoper.get_deleter()(&dummy_value); - EXPECT_EQ(1, deletes); - - // Deleter is still there after reset. - scoper.reset(); - EXPECT_EQ(2, deletes); - scoper.get_deleter()(&dummy_value); - EXPECT_EQ(3, deletes); - - // Deleter can be assigned into (matches C++11 unique_ptr<> spec). - scoper.get_deleter() = CountingDeleter(&alternate_deletes); - scoper.reset(&dummy_value); - EXPECT_EQ(0, alternate_deletes); - - } - EXPECT_EQ(3, deletes); - EXPECT_EQ(1, alternate_deletes); - - // Test operator= deleter support. - deletes = 0; - alternate_deletes = 0; - { - double dummy_value2; - scoped_ptr<double, CountingDeleter> scoper(&dummy_value, - CountingDeleter(&deletes)); - scoped_ptr<double, CountingDeleter> scoper2( - &dummy_value2, - CountingDeleter(&alternate_deletes)); - EXPECT_EQ(0, deletes); - EXPECT_EQ(0, alternate_deletes); - - // Pass the second deleter through a constructor and an operator=. Then - // reinitialize the empty scopers to ensure that each one is deleting - // properly. - scoped_ptr<double, CountingDeleter> scoper3(std::move(scoper2)); - scoper = std::move(scoper3); - EXPECT_EQ(1, deletes); - - scoper2.reset(&dummy_value2); - scoper3.reset(&dummy_value2); - EXPECT_EQ(0, alternate_deletes); - - } - EXPECT_EQ(1, deletes); - EXPECT_EQ(3, alternate_deletes); - - // Test swap(), and type-safe Boolean. - { - scoped_ptr<double, CountingDeleter> scoper1(NULL, - CountingDeleter(&deletes)); - scoped_ptr<double, CountingDeleter> scoper2(NULL, - CountingDeleter(&deletes)); - EXPECT_TRUE(scoper1.get() == scoper2.get()); - EXPECT_FALSE(scoper1.get() != scoper2.get()); - - scoper1.reset(&dummy_value); - EXPECT_TRUE(scoper1); - EXPECT_EQ(&dummy_value, scoper1.get()); - EXPECT_FALSE(scoper2); - EXPECT_FALSE(scoper2.get()); - EXPECT_FALSE(scoper1.get() == scoper2.get()); - EXPECT_TRUE(scoper1.get() != scoper2.get()); - - scoper2.swap(scoper1); - EXPECT_EQ(&dummy_value, scoper2.get()); - EXPECT_FALSE(scoper1.get()); - EXPECT_FALSE(scoper1.get() == scoper2.get()); - EXPECT_TRUE(scoper1.get() != scoper2.get()); - } -} - -// Sanity check test for overloaded new and delete operators. Does not do full -// coverage of reset/release/move operations as that is redundant with the -// above. -TEST(ScopedPtrTest, OverloadedNewAndDelete) { - { - OverloadedNewAndDelete::ResetCounters(); - scoped_ptr<OverloadedNewAndDelete> scoper(new OverloadedNewAndDelete()); - EXPECT_TRUE(scoper.get()); - - scoped_ptr<OverloadedNewAndDelete> scoper2(std::move(scoper)); - } - EXPECT_EQ(1, OverloadedNewAndDelete::delete_count()); - EXPECT_EQ(1, OverloadedNewAndDelete::new_count()); -} - -scoped_ptr<int> NullIntReturn() { - return nullptr; -} - -TEST(ScopedPtrTest, Nullptr) { - scoped_ptr<int> scoper1(nullptr); - scoped_ptr<int> scoper2(new int); - scoper2 = nullptr; - scoped_ptr<int> scoper3(NullIntReturn()); - scoped_ptr<int> scoper4 = NullIntReturn(); - EXPECT_EQ(nullptr, scoper1.get()); - EXPECT_EQ(nullptr, scoper2.get()); - EXPECT_EQ(nullptr, scoper3.get()); - EXPECT_EQ(nullptr, scoper4.get()); -} - -scoped_ptr<int[]> NullIntArrayReturn() { - return nullptr; -} - -TEST(ScopedPtrTest, NullptrArray) { - scoped_ptr<int[]> scoper1(nullptr); - scoped_ptr<int[]> scoper2(new int[3]); - scoper2 = nullptr; - scoped_ptr<int[]> scoper3(NullIntArrayReturn()); - scoped_ptr<int[]> scoper4 = NullIntArrayReturn(); - EXPECT_EQ(nullptr, scoper1.get()); - EXPECT_EQ(nullptr, scoper2.get()); - EXPECT_EQ(nullptr, scoper3.get()); - EXPECT_EQ(nullptr, scoper4.get()); -} - -class Super {}; -class Sub : public Super {}; - -scoped_ptr<Sub> SubClassReturn() { - return make_scoped_ptr(new Sub); -} - -TEST(ScopedPtrTest, Conversion) { - scoped_ptr<Sub> sub1(new Sub); - scoped_ptr<Sub> sub2(new Sub); - - // Upcast with move works. - scoped_ptr<Super> super1 = std::move(sub1); - super1 = std::move(sub2); - - // Upcast with an rvalue works. - scoped_ptr<Super> super2 = SubClassReturn(); - super2 = SubClassReturn(); -} - -// Logging a scoped_ptr<T> to an ostream shouldn't convert it to a boolean -// value first. -TEST(ScopedPtrTest, LoggingDoesntConvertToBoolean) { - scoped_ptr<int> x(new int); - std::stringstream s1; - s1 << x; - - std::stringstream s2; - s2 << x.get(); - - EXPECT_EQ(s2.str(), s1.str()); -} - -TEST(ScopedPtrTest, ReferenceCycle) { - struct StructB; - struct StructA { - scoped_ptr<StructB> b; - }; - - struct StructB { - scoped_ptr<StructA> a; - }; - - // Create a reference cycle. - StructA* a = new StructA; - a->b.reset(new StructB); - a->b->a.reset(a); - - // Break the cycle by calling reset(). This will cause |a| (and hence, |a->b|) - // to be deleted before the call to reset() returns. This tests that the - // implementation of scoped_ptr::reset() doesn't access |this| after it - // deletes the underlying pointer. This behaviour is consistent with the - // definition of unique_ptr::reset in C++11. - a->b.reset(); - - // Go again, but this time, break the cycle by invoking |a|'s destructor. This - // tests that the implementation of ~scoped_ptr doesn't infinitely recurse - // into the destructors of |a| and |a->b|. Note, deleting |a| instead will - // cause |a| to be double-free'd because |a->b| owns |a| and deletes it via - // its destructor. - a = new StructA; - a->b.reset(new StructB); - a->b->a.reset(a); - a->~StructA(); -} - -TEST(ScopedPtrTest, Operators) { - struct Parent {}; - struct Child : public Parent {}; - - scoped_ptr<Parent> p(new Parent); - scoped_ptr<Parent> p2(new Parent); - scoped_ptr<Child> c(new Child); - scoped_ptr<Parent> pnull; - - // Operator==. - EXPECT_TRUE(p == p); - EXPECT_FALSE(p == c); - EXPECT_FALSE(p == p2); - EXPECT_FALSE(p == pnull); - - EXPECT_FALSE(p == nullptr); - EXPECT_FALSE(nullptr == p); - EXPECT_TRUE(pnull == nullptr); - EXPECT_TRUE(nullptr == pnull); - - // Operator!=. - EXPECT_FALSE(p != p); - EXPECT_TRUE(p != c); - EXPECT_TRUE(p != p2); - EXPECT_TRUE(p != pnull); - - EXPECT_TRUE(p != nullptr); - EXPECT_TRUE(nullptr != p); - EXPECT_FALSE(pnull != nullptr); - EXPECT_FALSE(nullptr != pnull); - - // Compare two scoped_ptr<T>. - EXPECT_EQ(p.get() < p2.get(), p < p2); - EXPECT_EQ(p.get() <= p2.get(), p <= p2); - EXPECT_EQ(p.get() > p2.get(), p > p2); - EXPECT_EQ(p.get() >= p2.get(), p >= p2); - EXPECT_EQ(p2.get() < p.get(), p2 < p); - EXPECT_EQ(p2.get() <= p.get(), p2 <= p); - EXPECT_EQ(p2.get() > p.get(), p2 > p); - EXPECT_EQ(p2.get() >= p.get(), p2 >= p); - - // And convertible scoped_ptr<T> and scoped_ptr<U>. - EXPECT_EQ(p.get() < c.get(), p < c); - EXPECT_EQ(p.get() <= c.get(), p <= c); - EXPECT_EQ(p.get() > c.get(), p > c); - EXPECT_EQ(p.get() >= c.get(), p >= c); - EXPECT_EQ(c.get() < p.get(), c < p); - EXPECT_EQ(c.get() <= p.get(), c <= p); - EXPECT_EQ(c.get() > p.get(), c > p); - EXPECT_EQ(c.get() >= p.get(), c >= p); - - // Compare to nullptr. - EXPECT_TRUE(p > nullptr); - EXPECT_FALSE(nullptr > p); - EXPECT_FALSE(pnull > nullptr); - EXPECT_FALSE(nullptr > pnull); - - EXPECT_TRUE(p >= nullptr); - EXPECT_FALSE(nullptr >= p); - EXPECT_TRUE(pnull >= nullptr); - EXPECT_TRUE(nullptr >= pnull); - - EXPECT_FALSE(p < nullptr); - EXPECT_TRUE(nullptr < p); - EXPECT_FALSE(pnull < nullptr); - EXPECT_FALSE(nullptr < pnull); - - EXPECT_FALSE(p <= nullptr); - EXPECT_TRUE(nullptr <= p); - EXPECT_TRUE(pnull <= nullptr); - EXPECT_TRUE(nullptr <= pnull); -}; - -TEST(ScopedPtrTest, ArrayOperators) { - struct Parent {}; - struct Child : public Parent {}; - - scoped_ptr<Parent[]> p(new Parent[1]); - scoped_ptr<Parent[]> p2(new Parent[1]); - scoped_ptr<Child[]> c(new Child[1]); - scoped_ptr<Parent[]> pnull; - - // Operator==. - EXPECT_TRUE(p == p); - EXPECT_FALSE(p == c); - EXPECT_FALSE(p == p2); - EXPECT_FALSE(p == pnull); - - EXPECT_FALSE(p == nullptr); - EXPECT_FALSE(nullptr == p); - EXPECT_TRUE(pnull == nullptr); - EXPECT_TRUE(nullptr == pnull); - - // Operator!=. - EXPECT_FALSE(p != p); - EXPECT_TRUE(p != c); - EXPECT_TRUE(p != p2); - EXPECT_TRUE(p != pnull); - - EXPECT_TRUE(p != nullptr); - EXPECT_TRUE(nullptr != p); - EXPECT_FALSE(pnull != nullptr); - EXPECT_FALSE(nullptr != pnull); - - // Compare two scoped_ptr<T>. - EXPECT_EQ(p.get() < p2.get(), p < p2); - EXPECT_EQ(p.get() <= p2.get(), p <= p2); - EXPECT_EQ(p.get() > p2.get(), p > p2); - EXPECT_EQ(p.get() >= p2.get(), p >= p2); - EXPECT_EQ(p2.get() < p.get(), p2 < p); - EXPECT_EQ(p2.get() <= p.get(), p2 <= p); - EXPECT_EQ(p2.get() > p.get(), p2 > p); - EXPECT_EQ(p2.get() >= p.get(), p2 >= p); - - // And convertible scoped_ptr<T> and scoped_ptr<U>. - EXPECT_EQ(p.get() < c.get(), p < c); - EXPECT_EQ(p.get() <= c.get(), p <= c); - EXPECT_EQ(p.get() > c.get(), p > c); - EXPECT_EQ(p.get() >= c.get(), p >= c); - EXPECT_EQ(c.get() < p.get(), c < p); - EXPECT_EQ(c.get() <= p.get(), c <= p); - EXPECT_EQ(c.get() > p.get(), c > p); - EXPECT_EQ(c.get() >= p.get(), c >= p); - - // Compare to nullptr. - EXPECT_TRUE(p > nullptr); - EXPECT_FALSE(nullptr > p); - EXPECT_FALSE(pnull > nullptr); - EXPECT_FALSE(nullptr > pnull); - - EXPECT_TRUE(p >= nullptr); - EXPECT_FALSE(nullptr >= p); - EXPECT_TRUE(pnull >= nullptr); - EXPECT_TRUE(nullptr >= pnull); - - EXPECT_FALSE(p < nullptr); - EXPECT_TRUE(nullptr < p); - EXPECT_FALSE(pnull < nullptr); - EXPECT_FALSE(nullptr < pnull); - - EXPECT_FALSE(p <= nullptr); - EXPECT_TRUE(nullptr <= p); - EXPECT_TRUE(pnull <= nullptr); - EXPECT_TRUE(nullptr <= pnull); -} diff --git a/third_party/chromium/base/memory/weak_ptr.cc b/third_party/chromium/base/memory/weak_ptr.cc index 0f91ef3..7c9ced0 100644 --- a/third_party/chromium/base/memory/weak_ptr.cc +++ b/third_party/chromium/base/memory/weak_ptr.cc @@ -24,6 +24,8 @@ WeakReference::Flag::~Flag() { WeakReference::WeakReference() { } +WeakReference::WeakReference(const WeakReference& other) = default; + WeakReference::WeakReference(const Flag* flag) : flag_(flag) { } diff --git a/third_party/chromium/base/memory/weak_ptr.h b/third_party/chromium/base/memory/weak_ptr.h index c1c52ee..601c379 100644 --- a/third_party/chromium/base/memory/weak_ptr.h +++ b/third_party/chromium/base/memory/weak_ptr.h @@ -70,6 +70,8 @@ #ifndef BASE_MEMORY_WEAK_PTR_H_ #define BASE_MEMORY_WEAK_PTR_H_ +#include <type_traits> + #include "base/base_export.h" #include "base/logging.h" #include "base/macros.h" @@ -104,6 +106,7 @@ class BASE_EXPORT WeakReference { }; WeakReference(); + WeakReference(const WeakReference& other); explicit WeakReference(const Flag* flag); ~WeakReference(); @@ -156,10 +159,9 @@ class SupportsWeakPtrBase { // function that makes calling this easier. template<typename Derived> static WeakPtr<Derived> StaticAsWeakPtr(Derived* t) { - typedef std::is_convertible<Derived*, internal::SupportsWeakPtrBase*> - convertible; - static_assert(convertible::value, - "AsWeakPtr argument must inherit from SupportsWeakPtr"); + static_assert( + std::is_base_of<internal::SupportsWeakPtrBase, Derived>::value, + "AsWeakPtr argument must inherit from SupportsWeakPtr"); return AsWeakPtrImpl<Derived>(t, *t); } @@ -215,27 +217,38 @@ class WeakPtr : public internal::WeakPtrBase { return get(); } - // Allow WeakPtr<element_type> to be used in boolean expressions, but not - // implicitly convertible to a real bool (which is dangerous). + void reset() { + ref_ = internal::WeakReference(); + ptr_ = NULL; + } + + // Implement "Safe Bool Idiom" + // https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool + // + // Allow WeakPtr<element_type> to be used in boolean expressions such as + // if (weak_ptr_instance) + // But do not become convertible to a real bool (which is dangerous). + // Implementation requires: + // typedef Testable + // operator Testable() const + // operator== + // operator!= // - // Note that this trick is only safe when the == and != operators - // are declared explicitly, as otherwise "weak_ptr1 == weak_ptr2" - // will compile but do the wrong thing (i.e., convert to Testable - // and then do the comparison). + // == and != operators must be declared explicitly or dissallowed, as + // otherwise "ptr1 == ptr2" will compile but do the wrong thing (i.e., convert + // to Testable and then do the comparison). + // + // C++11 provides for "explicit operator bool()", however it is currently + // banned due to MSVS2013. https://chromium-cpp.appspot.com/#core-blacklist private: typedef T* WeakPtr::*Testable; public: operator Testable() const { return get() ? &WeakPtr::ptr_ : NULL; } - void reset() { - ref_ = internal::WeakReference(); - ptr_ = NULL; - } - private: - // Explicitly declare comparison operators as required by the bool - // trick, but keep them private. + // Explicitly declare comparison operators as required by the "Safe Bool + // Idiom", but keep them private. template <class U> bool operator==(WeakPtr<U> const&) const; template <class U> bool operator!=(WeakPtr<U> const&) const; diff --git a/third_party/chromium/base/memory/weak_ptr_unittest.cc b/third_party/chromium/base/memory/weak_ptr_unittest.cc index 8d4057c..fdbb280 100644 --- a/third_party/chromium/base/memory/weak_ptr_unittest.cc +++ b/third_party/chromium/base/memory/weak_ptr_unittest.cc @@ -119,6 +119,37 @@ TEST(WeakPtrTest, DerivedTarget) { EXPECT_EQ(&target, ptr.get()); } +TEST(WeakPtrFactoryTest, BooleanTesting) { + int data; + WeakPtrFactory<int> factory(&data); + + WeakPtr<int> ptr_to_an_instance = factory.GetWeakPtr(); + EXPECT_TRUE(ptr_to_an_instance); + EXPECT_FALSE(!ptr_to_an_instance); + + if (ptr_to_an_instance) { + } else { + ADD_FAILURE() << "Pointer to an instance should result in true."; + } + + if (!ptr_to_an_instance) { // check for operator!(). + ADD_FAILURE() << "Pointer to an instance should result in !x being false."; + } + + WeakPtr<int> null_ptr; + EXPECT_FALSE(null_ptr); + EXPECT_TRUE(!null_ptr); + + if (null_ptr) { + ADD_FAILURE() << "Null pointer should result in false."; + } + + if (!null_ptr) { // check for operator!(). + } else { + ADD_FAILURE() << "Null pointer should result in !x being true."; + } +} + TEST(WeakPtrTest, InvalidateWeakPtrs) { int data; WeakPtrFactory<int> factory(&data); diff --git a/third_party/chromium/base/move.h b/third_party/chromium/base/move.h index 24bf9d7..42242b4 100644 --- a/third_party/chromium/base/move.h +++ b/third_party/chromium/base/move.h @@ -5,6 +5,7 @@ #ifndef BASE_MOVE_H_ #define BASE_MOVE_H_ +// TODO(dcheng): Remove this header. #include <utility> #include "base/compiler_specific.h" @@ -25,13 +26,11 @@ // into a scoped_ptr. The class must define a move constructor and move // assignment operator to make this work. // -// This version of the macro adds a Pass() function and a cryptic -// MoveOnlyTypeForCPP03 typedef for the base::Callback implementation to use. -// See IsMoveOnlyType template and its usage in base/callback_internal.h -// for more details. +// This version of the macro adds a cryptic MoveOnlyTypeForCPP03 typedef for the +// base::Callback implementation to use. See IsMoveOnlyType template and its +// usage in base/callback_internal.h for more details. // TODO(crbug.com/566182): Remove this macro and use DISALLOW_COPY_AND_ASSIGN // everywhere instead. -#if defined(OS_ANDROID) || defined(OS_LINUX) #define DISALLOW_COPY_AND_ASSIGN_WITH_MOVE_FOR_BIND(type) \ private: \ type(const type&) = delete; \ @@ -41,17 +40,5 @@ typedef void MoveOnlyTypeForCPP03; \ \ private: -#else -#define DISALLOW_COPY_AND_ASSIGN_WITH_MOVE_FOR_BIND(type) \ - private: \ - type(const type&) = delete; \ - void operator=(const type&) = delete; \ - \ - public: \ - type&& Pass() WARN_UNUSED_RESULT { return std::move(*this); } \ - typedef void MoveOnlyTypeForCPP03; \ - \ - private: -#endif #endif // BASE_MOVE_H_ diff --git a/third_party/chromium/base/numerics/safe_conversions_impl.h b/third_party/chromium/base/numerics/safe_conversions_impl.h index e1d376a..03e7ee6 100644 --- a/third_party/chromium/base/numerics/safe_conversions_impl.h +++ b/third_party/chromium/base/numerics/safe_conversions_impl.h @@ -8,8 +8,6 @@ #include <limits.h> #include <stdint.h> -#include <limits> - namespace base { namespace internal { diff --git a/third_party/chromium/base/numerics/safe_math.h b/third_party/chromium/base/numerics/safe_math.h index ddffb6e..9757f1c 100644 --- a/third_party/chromium/base/numerics/safe_math.h +++ b/third_party/chromium/base/numerics/safe_math.h @@ -6,6 +6,7 @@ #define BASE_NUMERICS_SAFE_MATH_H_ #include <stddef.h> +#include <type_traits> #include "base/numerics/safe_math_impl.h" diff --git a/third_party/chromium/base/numerics/safe_numerics_unittest.cc b/third_party/chromium/base/numerics/safe_numerics_unittest.cc index 8ac7b0c..de93e49 100644 --- a/third_party/chromium/base/numerics/safe_numerics_unittest.cc +++ b/third_party/chromium/base/numerics/safe_numerics_unittest.cc @@ -13,7 +13,6 @@ #include "base/compiler_specific.h" #include "base/numerics/safe_conversions.h" #include "base/numerics/safe_math.h" -#include "base/template_util.h" using std::numeric_limits; using base::CheckedNumeric; diff --git a/third_party/chromium/base/rand_util.cc b/third_party/chromium/base/rand_util.cc index c02c875..fab6c66 100644 --- a/third_party/chromium/base/rand_util.cc +++ b/third_party/chromium/base/rand_util.cc @@ -68,8 +68,8 @@ uint64_t RandGenerator(uint64_t range) { std::string RandBytesAsString(size_t length) { DCHECK_GT(length, 0u); - std::string result(length, ' '); - RandBytes(&result[0], length); + std::string result; + RandBytes(WriteInto(&result, length + 1), length); return result; } diff --git a/third_party/chromium/base/strings/string_number_conversions.cc b/third_party/chromium/base/strings/string_number_conversions.cc index ac0fd46..529e913 100644 --- a/third_party/chromium/base/strings/string_number_conversions.cc +++ b/third_party/chromium/base/strings/string_number_conversions.cc @@ -136,6 +136,7 @@ class IteratorRangeToNumber { if (begin != end && *begin == '-') { if (!std::numeric_limits<value_type>::is_signed) { + *output = 0; valid = false; } else if (!Negative::Invoke(begin + 1, end, output)) { valid = false; diff --git a/third_party/chromium/base/strings/string_number_conversions_unittest.cc b/third_party/chromium/base/strings/string_number_conversions_unittest.cc index 1cfb2c8..d570c08 100644 --- a/third_party/chromium/base/strings/string_number_conversions_unittest.cc +++ b/third_party/chromium/base/strings/string_number_conversions_unittest.cc @@ -133,7 +133,7 @@ TEST(StringNumberConversionsTest, StringToInt) { }; for (size_t i = 0; i < arraysize(cases); ++i) { - int output = 0; + int output = cases[i].output ^ 1; // Ensure StringToInt wrote something. EXPECT_EQ(cases[i].success, StringToInt(cases[i].input, &output)); EXPECT_EQ(cases[i].output, output); } @@ -182,7 +182,8 @@ TEST(StringNumberConversionsTest, StringToUint) { }; for (size_t i = 0; i < arraysize(cases); ++i) { - unsigned output = 0; + unsigned output = + cases[i].output ^ 1; // Ensure StringToUint wrote something. EXPECT_EQ(cases[i].success, StringToUint(cases[i].input, &output)); EXPECT_EQ(cases[i].output, output); } diff --git a/third_party/chromium/base/strings/string_piece.h b/third_party/chromium/base/strings/string_piece.h index c79d9fa..b68b1f2 100644 --- a/third_party/chromium/base/strings/string_piece.h +++ b/third_party/chromium/base/strings/string_piece.h @@ -143,6 +143,8 @@ template <typename STRING_TYPE> class BasicStringPiece { } value_type operator[](size_type i) const { return ptr_[i]; } + value_type front() const { return ptr_[0]; } + value_type back() const { return ptr_[length_ - 1]; } void remove_prefix(size_type n) { ptr_ += n; diff --git a/third_party/chromium/base/strings/string_util.cc b/third_party/chromium/base/strings/string_util.cc index eb0c546..50f7a23 100644 --- a/third_party/chromium/base/strings/string_util.cc +++ b/third_party/chromium/base/strings/string_util.cc @@ -242,4 +242,17 @@ bool IsStringUTF8(const StringPiece& str) { return true; } + +template <class string_type> +inline typename string_type::value_type* WriteIntoT(string_type* str, + size_t length_with_null) { + DCHECK_GT(length_with_null, 1u); + str->reserve(length_with_null); + str->resize(length_with_null - 1); + return &((*str)[0]); +} + +char* WriteInto(std::string* str, size_t length_with_null) { + return WriteIntoT(str, length_with_null); +} } // namespace base diff --git a/third_party/chromium/base/strings/string_util.h b/third_party/chromium/base/strings/string_util.h index d6b6e10..f505bb6 100644 --- a/third_party/chromium/base/strings/string_util.h +++ b/third_party/chromium/base/strings/string_util.h @@ -173,6 +173,28 @@ TrimPositions TrimWhitespaceASCII(const std::string& input, bool IsStringUTF8(const StringPiece& str); bool IsStringASCII(const StringPiece& str); +// Reserves enough memory in |str| to accommodate |length_with_null| characters, +// sets the size of |str| to |length_with_null - 1| characters, and returns a +// pointer to the underlying contiguous array of characters. This is typically +// used when calling a function that writes results into a character array, but +// the caller wants the data to be managed by a string-like object. It is +// convenient in that is can be used inline in the call, and fast in that it +// avoids copying the results of the call from a char* into a string. +// +// |length_with_null| must be at least 2, since otherwise the underlying string +// would have size 0, and trying to access &((*str)[0]) in that case can result +// in a number of problems. +// +// Internally, this takes linear time because the resize() call 0-fills the +// underlying array for potentially all +// (|length_with_null - 1| * sizeof(string_type::value_type)) bytes. Ideally we +// could avoid this aspect of the resize() call, as we expect the caller to +// immediately write over this memory, but there is no other way to set the size +// of the string, and not doing that will mean people who access |str| rather +// than str.c_str() will get back a string of whatever size |str| had on entry +// to this function (probably 0). +char* WriteInto(std::string* str, size_t length_with_null); + } // namespace base #if defined(OS_WIN) diff --git a/third_party/chromium/base/strings/string_util_unittest.cc b/third_party/chromium/base/strings/string_util_unittest.cc index 451fbd8..8d77d36 100644 --- a/third_party/chromium/base/strings/string_util_unittest.cc +++ b/third_party/chromium/base/strings/string_util_unittest.cc @@ -147,4 +147,39 @@ TEST(StringUtilTest, ReplaceChars) { } } +class WriteIntoTest : public testing::Test { + protected: + static void WritesCorrectly(size_t num_chars) { + std::string buffer; + char kOriginal[] = "supercali"; + strncpy(WriteInto(&buffer, num_chars + 1), kOriginal, num_chars); + // Using std::string(buffer.c_str()) instead of |buffer| truncates the + // string at the first \0. + EXPECT_EQ(std::string(kOriginal, + std::min(num_chars, arraysize(kOriginal) - 1)), + std::string(buffer.c_str())); + EXPECT_EQ(num_chars, buffer.size()); + } +}; + +TEST_F(WriteIntoTest, WriteInto) { + // Validate that WriteInto reserves enough space and + // sizes a string correctly. + WritesCorrectly(1); + WritesCorrectly(2); + WritesCorrectly(5000); + + // Validate that WriteInto doesn't modify other strings + // when using a Copy-on-Write implementation. + const char kLive[] = "live"; + const char kDead[] = "dead"; + const std::string live = kLive; + std::string dead = live; + strncpy(WriteInto(&dead, 5), kDead, 4); + EXPECT_EQ(kDead, dead); + EXPECT_EQ(4u, dead.size()); + EXPECT_EQ(kLive, live); + EXPECT_EQ(4u, live.size()); +} + } // namespace base diff --git a/third_party/chromium/base/template_util.h b/third_party/chromium/base/template_util.h index 7b61d24..0c3cac2 100644 --- a/third_party/chromium/base/template_util.h +++ b/third_party/chromium/base/template_util.h @@ -5,7 +5,6 @@ #ifndef BASE_TEMPLATE_UTIL_H_ #define BASE_TEMPLATE_UTIL_H_ -#include <stddef.h> #include <type_traits> #include "build/build_config.h" @@ -16,18 +15,6 @@ template <class T> struct is_non_const_reference : std::false_type {}; template <class T> struct is_non_const_reference<T&> : std::true_type {}; template <class T> struct is_non_const_reference<const T&> : std::false_type {}; -namespace internal { - -// Types YesType and NoType are guaranteed such that sizeof(YesType) < -// sizeof(NoType). -typedef char YesType; - -struct NoType { - YesType dummy[2]; -}; - -} // namespace internal - } // namespace base #endif // BASE_TEMPLATE_UTIL_H_ diff --git a/third_party/chromium/base/template_util_unittest.cc b/third_party/chromium/base/template_util_unittest.cc index ce029af..25f4ba3 100644 --- a/third_party/chromium/base/template_util_unittest.cc +++ b/third_party/chromium/base/template_util_unittest.cc @@ -10,106 +10,11 @@ namespace base { namespace { -struct AStruct {}; -class AClass {}; -enum AnEnum {}; - -class Parent {}; -class Child : public Parent {}; - -using std::is_pointer; -using std::is_array; -using std::is_convertible; -using std::is_same; -using std::is_class; -using std::is_member_function_pointer; - -// is_pointer<Type> -static_assert(!is_pointer<int>::value, "IsPointer"); -static_assert(!is_pointer<int&>::value, "IsPointer"); -static_assert(is_pointer<int*>::value, "IsPointer"); -static_assert(is_pointer<const int*>::value, "IsPointer"); - -// is_array<Type> -static_assert(!is_array<int>::value, "IsArray"); -static_assert(!is_array<int*>::value, "IsArray"); -static_assert(!is_array<int (*)[3]>::value, "IsArray"); -static_assert(is_array<int[]>::value, "IsArray"); -static_assert(is_array<const int[]>::value, "IsArray"); -static_assert(is_array<int[3]>::value, "IsArray"); - // is_non_const_reference<Type> static_assert(!is_non_const_reference<int>::value, "IsNonConstReference"); static_assert(!is_non_const_reference<const int&>::value, "IsNonConstReference"); static_assert(is_non_const_reference<int&>::value, "IsNonConstReference"); -// is_convertible<From, To> - -// Extra parens needed to make preprocessor macro parsing happy. Otherwise, -// it sees the equivalent of: -// -// (is_convertible < Child), (Parent > ::value) -// -// Silly C++. -static_assert((is_convertible<Child, Parent>::value), "IsConvertible"); -static_assert(!(is_convertible<Parent, Child>::value), "IsConvertible"); -static_assert(!(is_convertible<Parent, AStruct>::value), "IsConvertible"); -static_assert((is_convertible<int, double>::value), "IsConvertible"); -static_assert((is_convertible<int*, void*>::value), "IsConvertible"); -static_assert(!(is_convertible<void*, int*>::value), "IsConvertible"); - -// Array types are an easy corner case. Make sure to test that -// it does indeed compile. -static_assert(!(is_convertible<int[10], double>::value), "IsConvertible"); -static_assert(!(is_convertible<double, int[10]>::value), "IsConvertible"); -static_assert((is_convertible<int[10], int*>::value), "IsConvertible"); - -// is_same<Type1, Type2> -static_assert(!(is_same<Child, Parent>::value), "IsSame"); -static_assert(!(is_same<Parent, Child>::value), "IsSame"); -static_assert((is_same<Parent, Parent>::value), "IsSame"); -static_assert((is_same<int*, int*>::value), "IsSame"); -static_assert((is_same<int, int>::value), "IsSame"); -static_assert((is_same<void, void>::value), "IsSame"); -static_assert(!(is_same<int, double>::value), "IsSame"); - -// is_class<Type> -static_assert(is_class<AStruct>::value, "IsClass"); -static_assert(is_class<AClass>::value, "IsClass"); -static_assert(!is_class<AnEnum>::value, "IsClass"); -static_assert(!is_class<int>::value, "IsClass"); -static_assert(!is_class<char*>::value, "IsClass"); -static_assert(!is_class<int&>::value, "IsClass"); -static_assert(!is_class<char[3]>::value, "IsClass"); - -static_assert(!is_member_function_pointer<int>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<int*>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<void*>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<AStruct>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<AStruct*>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<void (*)()>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<int (*)(int)>::value, - "IsMemberFunctionPointer"); -static_assert(!is_member_function_pointer<int (*)(int, int)>::value, - "IsMemberFunctionPointer"); - -static_assert(is_member_function_pointer<void (AStruct::*)()>::value, - "IsMemberFunctionPointer"); -static_assert(is_member_function_pointer<void (AStruct::*)(int)>::value, - "IsMemberFunctionPointer"); -static_assert(is_member_function_pointer<int (AStruct::*)(int)>::value, - "IsMemberFunctionPointer"); -static_assert(is_member_function_pointer<int (AStruct::*)(int) const>::value, - "IsMemberFunctionPointer"); -static_assert(is_member_function_pointer<int (AStruct::*)(int, int)>::value, - "IsMemberFunctionPointer"); - } // namespace } // namespace base diff --git a/third_party/chromium/base/third_party/dmg_fp/README.chromium b/third_party/chromium/base/third_party/dmg_fp/README.chromium index 4538b7e..d16ce9d 100644 --- a/third_party/chromium/base/third_party/dmg_fp/README.chromium +++ b/third_party/chromium/base/third_party/dmg_fp/README.chromium @@ -17,4 +17,6 @@ List of changes made to original code: - made minor changes for -Wextra for Mac build, see mac_wextra.patch - crash fix for running with reduced CPU float precision, see float_precision_crash.patch and crbug.com/123157 - - fixed warnings under msvc, see msvc_warnings.patch
\ No newline at end of file + - fixed warnings under msvc, see msvc_warnings.patch + - fixed -Wchar-subscripts warning in "if (!hexdig['0'])" in dtoa.cc + - removed the use of 'register' keyword from g_fmt.cc diff --git a/third_party/chromium/base/third_party/dmg_fp/dtoa.cc b/third_party/chromium/base/third_party/dmg_fp/dtoa.cc index c06219c..8eabfe2 100644 --- a/third_party/chromium/base/third_party/dmg_fp/dtoa.cc +++ b/third_party/chromium/base/third_party/dmg_fp/dtoa.cc @@ -59,7 +59,6 @@ * 4. Because of 3., we don't need a large table of powers of 10 * for ten-to-e (just some small tables, e.g. of 10^k * for 0 <= k <= 22). - * 5. Fixed -Wchar-subscripts warning in "if (!hexdig['0'])" */ /* diff --git a/third_party/chromium/base/tuple.h b/third_party/chromium/base/tuple.h index e5872cc..78dfd75 100644 --- a/third_party/chromium/base/tuple.h +++ b/third_party/chromium/base/tuple.h @@ -29,6 +29,7 @@ #define BASE_TUPLE_H_ #include <stddef.h> +#include <tuple> #include "base/bind_helpers.h" #include "build/build_config.h" @@ -109,28 +110,6 @@ struct MakeIndexSequenceImpl<N, Ns...> template <size_t N> using MakeIndexSequence = typename MakeIndexSequenceImpl<N>::Type; -// Traits ---------------------------------------------------------------------- -// -// A simple traits class for tuple arguments. -// -// ValueType: the bare, nonref version of a type (same as the type for nonrefs). -// RefType: the ref version of a type (same as the type for refs). -// ParamType: what type to pass to functions (refs should not be constified). - -template <class P> -struct TupleTraits { - typedef P ValueType; - typedef P& RefType; - typedef const P& ParamType; -}; - -template <class P> -struct TupleTraits<P&> { - typedef P ValueType; - typedef P& RefType; - typedef P& ParamType; -}; - // Tuple ----------------------------------------------------------------------- // // This set of classes is useful for bundling 0 or more heterogeneous data types @@ -145,75 +124,10 @@ struct TupleTraits<P&> { // want filled by the dispatchee, and the tuple is merely a container for that // output (a "tier"). See MakeRefTuple and its usages. -template <typename IxSeq, typename... Ts> -struct TupleBaseImpl; template <typename... Ts> -using TupleBase = TupleBaseImpl<MakeIndexSequence<sizeof...(Ts)>, Ts...>; -template <size_t N, typename T> -struct TupleLeaf; +using Tuple = std::tuple<Ts...>; -template <typename... Ts> -struct Tuple final : TupleBase<Ts...> { - Tuple() : TupleBase<Ts...>() {} - explicit Tuple(typename TupleTraits<Ts>::ParamType... args) - : TupleBase<Ts...>(args...) {} -}; - -// Avoids ambiguity between Tuple's two constructors. -template <> -struct Tuple<> final {}; - -template <size_t... Ns, typename... Ts> -struct TupleBaseImpl<IndexSequence<Ns...>, Ts...> : TupleLeaf<Ns, Ts>... { - TupleBaseImpl() : TupleLeaf<Ns, Ts>()... {} - explicit TupleBaseImpl(typename TupleTraits<Ts>::ParamType... args) - : TupleLeaf<Ns, Ts>(args)... {} -}; - -template <size_t N, typename T> -struct TupleLeaf { - TupleLeaf() {} - explicit TupleLeaf(typename TupleTraits<T>::ParamType x) : x(x) {} - - T& get() { return x; } - const T& get() const { return x; } - - T x; -}; - -// Tuple getters -------------------------------------------------------------- -// -// Allows accessing an arbitrary tuple element by index. -// -// Example usage: -// base::Tuple<int, double> t2; -// base::get<0>(t2) = 42; -// base::get<1>(t2) = 3.14; - -template <size_t I, typename T> -T& get(TupleLeaf<I, T>& leaf) { - return leaf.get(); -} - -template <size_t I, typename T> -const T& get(const TupleLeaf<I, T>& leaf) { - return leaf.get(); -} - -// Tuple types ---------------------------------------------------------------- -// -// Allows for selection of ValueTuple/RefTuple/ParamTuple without needing the -// definitions of class types the tuple takes as parameters. - -template <typename T> -struct TupleTypes; - -template <typename... Ts> -struct TupleTypes<Tuple<Ts...>> { - using ValueTuple = Tuple<typename TupleTraits<Ts>::ValueType...>; - using RefTuple = Tuple<typename TupleTraits<Ts>::RefType...>; - using ParamTuple = Tuple<typename TupleTraits<Ts>::ParamType...>; -}; +using std::get; // Tuple creators ------------------------------------------------------------- // @@ -245,15 +159,15 @@ inline Tuple<Ts&...> MakeRefTuple(Ts&... arg) { // Non-Static Dispatchers with no out params. template <typename ObjT, typename Method, typename... Ts, size_t... Ns> -inline void DispatchToMethodImpl(ObjT* obj, +inline void DispatchToMethodImpl(const ObjT& obj, Method method, const Tuple<Ts...>& arg, IndexSequence<Ns...>) { - (obj->*method)(base::internal::UnwrapTraits<Ts>::Unwrap(get<Ns>(arg))...); + (obj->*method)(internal::Unwrap(get<Ns>(arg))...); } template <typename ObjT, typename Method, typename... Ts> -inline void DispatchToMethod(ObjT* obj, +inline void DispatchToMethod(const ObjT& obj, Method method, const Tuple<Ts...>& arg) { DispatchToMethodImpl(obj, method, arg, MakeIndexSequence<sizeof...(Ts)>()); @@ -265,7 +179,7 @@ template <typename Function, typename... Ts, size_t... Ns> inline void DispatchToFunctionImpl(Function function, const Tuple<Ts...>& arg, IndexSequence<Ns...>) { - (*function)(base::internal::UnwrapTraits<Ts>::Unwrap(get<Ns>(arg))...); + (*function)(internal::Unwrap(get<Ns>(arg))...); } template <typename Function, typename... Ts> @@ -281,18 +195,17 @@ template <typename ObjT, typename... OutTs, size_t... InNs, size_t... OutNs> -inline void DispatchToMethodImpl(ObjT* obj, +inline void DispatchToMethodImpl(const ObjT& obj, Method method, const Tuple<InTs...>& in, Tuple<OutTs...>* out, IndexSequence<InNs...>, IndexSequence<OutNs...>) { - (obj->*method)(base::internal::UnwrapTraits<InTs>::Unwrap(get<InNs>(in))..., - &get<OutNs>(*out)...); + (obj->*method)(internal::Unwrap(get<InNs>(in))..., &get<OutNs>(*out)...); } template <typename ObjT, typename Method, typename... InTs, typename... OutTs> -inline void DispatchToMethod(ObjT* obj, +inline void DispatchToMethod(const ObjT& obj, Method method, const Tuple<InTs...>& in, Tuple<OutTs...>* out) { diff --git a/third_party/chromium/base/values.cc b/third_party/chromium/base/values.cc index 55719da..29f0301 100644 --- a/third_party/chromium/base/values.cc +++ b/third_party/chromium/base/values.cc @@ -794,6 +794,8 @@ DictionaryValue::Iterator::Iterator(const DictionaryValue& target) : target_(target), it_(target.dictionary_.begin()) {} +DictionaryValue::Iterator::Iterator(const Iterator& other) = default; + DictionaryValue::Iterator::~Iterator() {} DictionaryValue* DictionaryValue::DeepCopy() const { diff --git a/third_party/chromium/base/values.h b/third_party/chromium/base/values.h index 25d2a79..36e24cc 100644 --- a/third_party/chromium/base/values.h +++ b/third_party/chromium/base/values.h @@ -348,6 +348,7 @@ class BASE_EXPORT DictionaryValue : public Value { class BASE_EXPORT Iterator { public: explicit Iterator(const DictionaryValue& target); + Iterator(const Iterator& other); ~Iterator(); bool IsAtEnd() const { return it_ == target_.dictionary_.end(); } diff --git a/third_party/get_cross.sh b/third_party/get_cross.sh new file mode 100755 index 0000000..f69291c --- /dev/null +++ b/third_party/get_cross.sh @@ -0,0 +1,186 @@ +#!/bin/bash +# Copyright 2016 The Weave Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +set -e + +SCRIPT=$(readlink -f "$0") +THIRD_PARTY=$(dirname "${SCRIPT}") +cd "${THIRD_PARTY}" + +OUT="cross" +DISTDIR="${OUT}/distfiles" + +CROS_OVERLAY_URL="https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/chromeos" +CONF_SDK_LATEST="${CROS_OVERLAY_URL}/binhost/host/sdk_version.conf" + +SDK_BUCKET="https://commondatastorage.googleapis.com/chromiumos-sdk" +BINPKG_BUCKET="https://commondatastorage.googleapis.com/chromeos-prebuilt" +CROS_BUCKET="https://commondatastorage.googleapis.com/chromeos-image-archive" + +PKGS=( + app-emulation/qemu +) +TARGETS=( +# aarch64-cros-linux-gnu + armv7a-cros-linux-gnueabi + i686-pc-linux-gnu +# mips-cros-linux-gnu + mipsel-cros-linux-gnu + x86_64-cros-linux-gnu +) +BOARDS=( +# aarch64-generic-full + amd64-generic-full + arm-generic-full +# mips-o32-generic-full + mipsel-o32-generic-full + x86-generic-full +) + +usage() { + cat <<EOF +Usage: get_cross.sh + +Download cross-compilers for building & testing against other arches. +EOF + exit 0 +} + +get_gitiles() { + local url="$1" data + data=$(curl -s "${url}?format=TEXT") + echo "${data}" | base64 -d +} + +json() { + local file="$1" arg="$2" + python <<EOF +import json +print(json.load(open("${file}"))${arg}) +EOF +} + +fetch() { + local url=$1 + file="${2:-${DISTDIR}/${url##*/}}" + if [[ ! -e ${file} ]]; then + printf '[downloading] ' + mkdir -p "${DISTDIR}" + wget "${url}" -O "${file}" + fi +} + +unpack() { + local out="$1" file="$2" + printf '[unpacking] ' + rm -rf "${out}" + mkdir -p "${out}" + tar xf "${file}" -C "${out}" +} + +fetch_pkgs() { + local pkg + local sub_url url file manifest + local out ver_file old_ver ver + + # Grab a few helper packages. + printf 'Getting SDK manifest ... ' + sub_url="cros-sdk-${SDK_LATEST_VERSION}.tar.xz.Manifest" + url="${SDK_BUCKET}/${sub_url}" + fetch "${url}" + manifest=${file} + printf '%s\n' "${manifest}" + + for pkg in "${PKGS[@]}"; do + printf 'Getting binpkg %s ... ' "${pkg}" + ver=$(json "${manifest}" '["packages"]["app-emulation/qemu"][0][0]') + sub_url="host/amd64/amd64-host/chroot-${SDK_LATEST_VERSION}/packages/${pkg}-${ver}.tbz2" + url="${BINPKG_BUCKET}/${sub_url}" + fetch "${url}" + + out="${OUT}/${pkg}" + ver_file="${out}/.ver" + old_ver=$(cat "${ver_file}" 2>/dev/null || :) + if [[ "${old_ver}" != "${ver}" ]]; then + unpack "${out}" "${file}" + echo "${ver}" > "${ver_file}" + fi + + printf '%s\n' "${ver}" + done +} + +fetch_toolchains() { + local target + local sub_url url file + local out ver_file ver + + # Download the base toolchains. + for target in "${TARGETS[@]}"; do + printf 'Getting toolchain for %s ... ' "${target}" + + sub_url="${TC_PATH/\%(target)s/${target}}" + url="${SDK_BUCKET}/${sub_url}" + file="${DISTDIR}/${url##*/}" + fetch "${url}" + + out="${OUT}/${target}" + ver_file="${out}/.ver" + ver=$(cat "${ver_file}" 2>/dev/null || :) + if [[ "${ver}" != "${SDK_LATEST_VERSION}" ]]; then + unpack "${out}" "${file}" + echo "${SDK_LATEST_VERSION}" > "${ver_file}" + fi + + printf '%s\n' "${sub_url}" + done +} + +fetch_sysroots() { + local board + local board_latest_url sub_url url file + local out ver_file ver + + # Get the full sysroot. + for board in "${BOARDS[@]}"; do + printf 'Getting sysroot for %s ... ' "${board}" + board_latest_url="${CROS_BUCKET}/${board}/LATEST-master" + if ! board_ver=$(curl --fail -s "${board_latest_url}"); then + echo 'error: not found' + continue + fi + + url="${CROS_BUCKET}/${board}/${board_ver}/sysroot_chromeos-base_chromeos-chrome.tar.xz" + file="${DISTDIR}/${board}-${board_ver}-${url##*/}" + fetch "${url}" "${file}" + + out="${OUT}/${board}" + ver_file="${out}/.ver" + ver=$(cat "${ver_file}" 2>/dev/null || :) + if [[ "${ver}" != "${board_ver}" ]]; then + unpack "${out}" "${file}" + echo "${board_ver}" > "${ver_file}" + fi + + printf '%s\n' "${board_ver}" + done +} + +main() { + if [[ $# -ne 0 ]]; then + usage + fi + + # Get the current SDK versions. + printf 'Getting CrOS SDK version ... ' + data=$(get_gitiles "${CONF_SDK_LATEST}") + eval "${data}" + echo "${SDK_LATEST_VERSION}" + + fetch_pkgs + fetch_toolchains + fetch_sysroots +} +main "$@" diff --git a/third_party/get_gtest.sh b/third_party/get_gtest.sh deleted file mode 100755 index 9b546ab..0000000 --- a/third_party/get_gtest.sh +++ /dev/null @@ -1,31 +0,0 @@ -#!/bin/bash -# Copyright 2015 The Weave Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -# Make gtest and gmock. -cd $(dirname "$0") -THIRD_PARTY=$(pwd) - -mkdir -p include lib - -rm -rf $THIRD_PARTY/googletest -git clone https://github.com/google/googletest.git || exit 1 -cd googletest - -# gtest is in process of changing of dir structure and it has broken build -# files. So this is temporarily workaround to fix that. -git reset --hard 82b11b8cfcca464c2ac74b623d04e74452e74f32 -mv googletest googlemock/gtest - -cd $THIRD_PARTY/googletest/googlemock/gtest/make || exit 1 -make gtest.a || exit 1 -cp -rf ../include/* $THIRD_PARTY/include/ || exit 1 -cp -rf gtest.a $THIRD_PARTY/lib/ || exit 1 - -cd $THIRD_PARTY/googletest/googlemock/make || exit 1 -make gmock.a || exit 1 -cp -rf ../include/* $THIRD_PARTY/include/ || exit 1 -cp -rf gmock.a $THIRD_PARTY/lib/ || exit 1 - -rm -rf $THIRD_PARTY/googletest diff --git a/third_party/get_libevhtp.sh b/third_party/get_libevhtp.sh deleted file mode 100755 index cfcada9..0000000 --- a/third_party/get_libevhtp.sh +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/bash -# Copyright 2016 The Weave Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -# Make libevhtp. -# Example uses libevhtp to implement HTTPS server. This step could be -# replaced with apt-get in future (Debian jessie, Ubuntu vivid). -cd $(dirname "$0") -THIRD_PARTY=$(pwd) - -LIBEVHTP_VERSION=1.2.11n - -mkdir -p include lib - -rm -rf $THIRD_PARTY/libevhtp -curl -L https://github.com/ellzey/libevhtp/archive/$LIBEVHTP_VERSION.tar.gz | tar xz || exit 1 -mv libevhtp-$LIBEVHTP_VERSION $THIRD_PARTY/libevhtp || exit 1 -cd $THIRD_PARTY/libevhtp || exit 1 - -cmake -D EVHTP_DISABLE_REGEX:BOOL=ON . || exit 1 -make evhtp || exit 1 - -cp -rf *.h $THIRD_PARTY/include/ || exit 1 -cp -f libevhtp.a $THIRD_PARTY/lib/ || exit 1 - -rm -rf $THIRD_PARTY/libevhtp diff --git a/third_party/libuweave/src/macaroon.c b/third_party/libuweave/src/macaroon.c index c823804..007aa15 100644 --- a/third_party/libuweave/src/macaroon.c +++ b/third_party/libuweave/src/macaroon.c @@ -31,15 +31,15 @@ static bool create_mac_tag_(const uint8_t* key, // Compute the first tag by using the key if (!uw_macaroon_caveat_sign_(key, key_len, context, caveats[0], mac_tag_buff, - UW_MACAROON_MAC_LEN)) { + sizeof(mac_tag_buff))) { return false; } // Compute the rest of the tags by using the tag as the key for (size_t i = 1; i < num_caveats; i++) { - if (!uw_macaroon_caveat_sign_(mac_tag_buff, UW_MACAROON_MAC_LEN, context, + if (!uw_macaroon_caveat_sign_(mac_tag_buff, sizeof(mac_tag_buff), context, caveats[i], mac_tag_buff, - UW_MACAROON_MAC_LEN)) { + sizeof(mac_tag_buff))) { return false; } } @@ -100,25 +100,32 @@ bool uw_macaroon_extend_(const UwMacaroon* old_macaroon, additional_caveat == NULL || buffer == NULL || buffer_size == 0) { return false; } + // If we update the same macaroon in-place, do not zero it. + if (new_macaroon != old_macaroon) { + *new_macaroon = (UwMacaroon){}; + } + + const size_t old_count = old_macaroon->num_caveats; + const size_t new_count = old_count + 1; - new_macaroon->num_caveats = old_macaroon->num_caveats + 1; + new_macaroon->num_caveats = new_count; // Extend the caveat pointer list - if ((new_macaroon->num_caveats) * sizeof(UwMacaroonCaveat*) > buffer_size) { + if (new_count * sizeof(new_macaroon->caveats[0]) > buffer_size) { // Not enough memory to store the extended caveat pointer list return false; } const UwMacaroonCaveat** extended_list = (const UwMacaroonCaveat**)buffer; - if (new_macaroon->caveats != old_macaroon->caveats) { + if (extended_list != old_macaroon->caveats) { memcpy(extended_list, old_macaroon->caveats, - old_macaroon->num_caveats * sizeof(old_macaroon->caveats[0])); + old_count * sizeof(old_macaroon->caveats[0])); } - extended_list[old_macaroon->num_caveats] = additional_caveat; + extended_list[old_count] = additional_caveat; new_macaroon->caveats = (const UwMacaroonCaveat* const*)extended_list; // Compute the new MAC tag return create_mac_tag_(old_macaroon->mac_tag, UW_MACAROON_MAC_LEN, context, - new_macaroon->caveats + old_macaroon->num_caveats, 1, + new_macaroon->caveats + old_count, 1, new_macaroon->mac_tag); } @@ -133,7 +140,7 @@ static void init_validation_result(UwMacaroonValidationResult* result) { /** Reset the result object to the lowest scope when encountering errors */ static void reset_validation_result(UwMacaroonValidationResult* result) { *result = (UwMacaroonValidationResult){ - .weave_app_restricted = true, + .app_commands_only = true, .granted_scope = UW_MACAROON_CAVEAT_SCOPE_LOWEST_POSSIBLE}; } @@ -318,3 +325,12 @@ bool uw_macaroon_deserialize_(const uint8_t* in, return true; } + +time_t uw_macaroon_get_expiration_unix_epoch_time_( + UwMacaroonValidationResult* result) { + // Expiration times of 0 and UINT32_MAX both mean no expiration. + if (result->expiration_time == 0 || result->expiration_time == UINT32_MAX) { + return 0; + } + return uw_macaroon_j2000_to_unix_epoch(result->expiration_time); +} diff --git a/third_party/libuweave/src/macaroon.h b/third_party/libuweave/src/macaroon.h index 5e73b28..310f988 100644 --- a/third_party/libuweave/src/macaroon.h +++ b/third_party/libuweave/src/macaroon.h @@ -8,12 +8,16 @@ #include <stdbool.h> #include <stddef.h> #include <stdint.h> +#include <time.h> #include "src/macaroon_caveat.h" #include "src/macaroon_context.h" #define UW_MACAROON_MAC_LEN 16 +// Jan 1st 2000 00:00:00 in unix epoch seconds. +#define J2000_EPOCH_OFFSET 946684800 + // Note: If we are looking to make memory savings on MCUs, // at the cost of a little extra processing, we can make // the macaroon encoding the actual in-memory representation. @@ -44,8 +48,8 @@ typedef struct { typedef struct { UwMacaroonCaveatScopeType granted_scope; - uint32_t expiration_time; - bool weave_app_restricted; + uint32_t expiration_time; // In number of seconds since Jan 1st 2000 00:00:00 + bool app_commands_only; const uint8_t* lan_session_id; size_t lan_session_id_len; UwMacaroonDelegateeInfo delegatees[MAX_NUM_DELEGATEES]; @@ -59,7 +63,10 @@ bool uw_macaroon_create_from_root_key_(UwMacaroon* new_macaroon, const UwMacaroonCaveat* const caveats[], size_t num_caveats); -/** Creates a new macaroon with a new caveat. */ +/** + * Creates a new macaroon with a new caveat. The buffer must be large enough to + * hold the count of caveats in the old_macaroon plus one. + */ bool uw_macaroon_extend_(const UwMacaroon* old_macaroon, UwMacaroon* new_macaroon, const UwMacaroonContext* context, @@ -97,4 +104,21 @@ bool uw_macaroon_deserialize_(const uint8_t* in, size_t buffer_size, UwMacaroon* new_macaroon); +/** Converts a j2000 timestamp to a unix timestamp. */ +static inline time_t uw_macaroon_j2000_to_unix_epoch(time_t j2000) { + return j2000 + J2000_EPOCH_OFFSET; +} + +/** Converts a unix timestamp to a j2000 timestamp. */ +static inline time_t uw_macaroon_unix_epoch_to_j2000(time_t unix) { + return unix - J2000_EPOCH_OFFSET; +} + +/** + * Gets the expiration time of the macaroon as the number of seconds since the + * unix epoch. A value of 0 means no expiration. + */ +time_t uw_macaroon_get_expiration_unix_epoch_time_( + UwMacaroonValidationResult* result); + #endif // LIBUWEAVE_SRC_MACAROON_H_ diff --git a/third_party/libuweave/src/macaroon_caveat.c b/third_party/libuweave/src/macaroon_caveat.c index 0abf7ca..bd1011b 100644 --- a/third_party/libuweave/src/macaroon_caveat.c +++ b/third_party/libuweave/src/macaroon_caveat.c @@ -349,8 +349,7 @@ bool uw_macaroon_caveat_sign_(const uint8_t* key, } UwMacaroonCaveatType caveat_type; - if (!uw_macaroon_caveat_get_type_(caveat, &caveat_type) || - !is_valid_caveat_type_(caveat_type)) { + if (!uw_macaroon_caveat_get_type_(caveat, &caveat_type)) { return false; } @@ -407,7 +406,7 @@ bool uw_macaroon_caveat_sign_(const uint8_t* key, } // The length here includes: 1. the header for the whole byte string; 2. the - // header for the addtional value part; 3. the additional value part. + // header for the additional value part; 3. the additional value part. size_t total_length = caveat->num_bytes + value_cbor_prefix_len + additional_value_str_len; if (!uw_macaroon_encoding_encode_byte_str_len_( @@ -579,7 +578,7 @@ bool uw_macaroon_caveat_validate_(const UwMacaroonCaveat* caveat, return true; case kUwMacaroonCaveatTypeAppCommandsOnly: - result->weave_app_restricted = true; + result->app_commands_only = true; return true; case kUwMacaroonCaveatTypeLanSessionID: diff --git a/third_party/third_party.mk b/third_party/third_party.mk index 7f651a2..18f9b98 100644 --- a/third_party/third_party.mk +++ b/third_party/third_party.mk @@ -13,7 +13,7 @@ $(third_party_chromium_base_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party_chromium_base_unittest_obj_files := $(THIRD_PARTY_CHROMIUM_BASE_UNITTEST_SRC_FILES:%.cc=out/$(BUILD_MODE)/%.o) -$(third_party_chromium_base_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party/include/gtest/gtest.h +$(third_party_chromium_base_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_TEST) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< @@ -25,7 +25,7 @@ $(third_party_chromium_crypto_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party_chromium_crypto_unittest_obj_files := $(THIRD_PARTY_CHROMIUM_CRYPTO_UNITTEST_SRC_FILES:%.cc=out/$(BUILD_MODE)/%.o) -$(third_party_chromium_crypto_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc third_party/include/gtest/gtest.h +$(third_party_chromium_crypto_unittest_obj_files) : out/$(BUILD_MODE)/%.o : %.cc mkdir -p $(dir $@) $(CXX) $(DEFS_TEST) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< @@ -48,31 +48,49 @@ $(third_party_libuweave_obj_files) : out/$(BUILD_MODE)/%.o : %.c $(CC) $(DEFS_$(BUILD_MODE)) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_C) -c -o $@ $< ### -# libgtest and libgmock (third_party, downloaded on build) +# libgtest and libgmock (third_party) -third_party/lib/gtest.a: third_party/include/gtest/gtest.h -third_party/lib/gmock.a: third_party/include/gtest/gtest.h +third_party_gtest_lib = out/$(BUILD_MODE)/third_party/googletest/libgtest.a +third_party_gmock_lib = out/$(BUILD_MODE)/third_party/googletest/libgmock.a -third_party/include/gtest/gtest.h: - @echo Downloading and building libgtest and libgmock... - third_party/get_gtest.sh - @echo Finished downloading and building libgtest and libgmock. +third_party_gtest_all = out/$(BUILD_MODE)/third_party/googletest/gtest-all.o +third_party_gmock_all = out/$(BUILD_MODE)/third_party/googletest/gmock-all.o + +$(third_party_gtest_all) : third_party/googletest/googletest/src/gtest-all.cc + mkdir -p $(dir $@) + $(CXX) $(DEFS_$(BUILD_MODE)) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< \ + -I third_party/googletest/googletest/ + +$(third_party_gmock_all) : third_party/googletest/googlemock/src/gmock-all.cc + mkdir -p $(dir $@) + $(CXX) $(DEFS_$(BUILD_MODE)) $(INCLUDES) $(CFLAGS) $(CFLAGS_$(BUILD_MODE)) $(CFLAGS_CC) -c -o $@ $< \ + -I third_party/googletest/googlemock/ + +$(third_party_gtest_lib) : $(third_party_gtest_all) + mkdir -p $(dir $@) + $(AR) crs $@ $^ + +$(third_party_gmock_lib) : $(third_party_gmock_all) + mkdir -p $(dir $@) + $(AR) crs $@ $^ clean-gtest : - rm -rf third_party/include/gtest third_party/include/gmock - rm -rf third_party/lib/libgmock* third_party/lib/libgtest* - rm -rf third_party/googletest + rm -rf $(third_party_gtest_lib) $(third_party_gmock_lib) \ + $(third_party_gtest_all) $(third_party_gmock_all) ### -# libevhtp (third_party, downloaded on build) +# libevhtp (third_party) + +third_party_libevhtp = out/$(BUILD_MODE)/third_party/libevhtp +third_party_libevhtp_lib = $(third_party_libevhtp)/libevhtp.a +third_party_libevhtp_header = $(third_party_libevhtp)/evhtp-config.h + +$(third_party_libevhtp_header) : + mkdir -p $(dir $@) + cd $(dir $@) && cmake -D EVHTP_DISABLE_REGEX:BOOL=ON $(PWD)/third_party/libevhtp -third_party/lib/libevhtp.a : third_party/include/evhtp.h -third_party/include/evhtp.h : - @echo Downloading and building libevhtp... - third_party/get_libevhtp.sh - @echo Finished downloading and building libevhtp. +$(third_party_libevhtp_lib) : $(third_party_libevhtp_header) + $(MAKE) -C $(third_party_libevhtp) clean-libevhtp : - rm -rf third_party/include/evhtp.h third_party/include/evhtp-config.h third_party/include/evthr.h third_party/include/htparse.h - rm -rf third_party/lib/libevhtp.a - rm -rf third_party/libevhtp + rm -rf $(third_party_libevhtp) |