diff options
author | rsleevi@chromium.org <rsleevi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-21 04:11:38 +0000 |
---|---|---|
committer | rsleevi@chromium.org <rsleevi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-21 04:11:38 +0000 |
commit | 0b5364316686f5c99e708ae2801eef13d0cddbcb (patch) | |
tree | a05e56435a375f068d42d3bda4eb0cfdad4afde0 | |
parent | fd6b2ddeb13cc927f3168b194697e6867f30bbda (diff) | |
download | chromium_src-0b5364316686f5c99e708ae2801eef13d0cddbcb.zip chromium_src-0b5364316686f5c99e708ae2801eef13d0cddbcb.tar.gz chromium_src-0b5364316686f5c99e708ae2801eef13d0cddbcb.tar.bz2 |
Revert 106660 - Speculative revert. Tests are now sporadically timing out/crashing with ExtensionSettingsApiTest on the stack.
Add onChanged events to the extension settings API, both from sync and between split mode background pages.
BUG=97545
TEST=Updated ExtensionSettingsStorageUnittest, ExtensionSettingsSyncUnittest, ExtensionSettingsApitest
Review URL: http://codereview.chromium.org/8177022
TBR=kalman@chromium.org
Review URL: http://codereview.chromium.org/8365027
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@106671 0039d316-1c4b-4281-b951-d872f2087c98
30 files changed, 471 insertions, 1672 deletions
diff --git a/chrome/browser/extensions/extension_event_names.cc b/chrome/browser/extensions/extension_event_names.cc index bc63d34..8c67a4d 100644 --- a/chrome/browser/extensions/extension_event_names.cc +++ b/chrome/browser/extensions/extension_event_names.cc @@ -35,6 +35,4 @@ const char kOnInputMethodChanged[] = "inputMethodPrivate.onChanged"; const char kOnDownloadCreated[] = "experimental.downloads.onCreated"; const char kOnDownloadChanged[] = "experimental.downloads.onChanged"; const char kOnDownloadErased[] = "experimental.downloads.onErased"; - -const char kOnSettingsChanged[] = "experimental.settings.onChanged"; } // namespace extension_event_names diff --git a/chrome/browser/extensions/extension_event_names.h b/chrome/browser/extensions/extension_event_names.h index 3955276..67b3dd2 100644 --- a/chrome/browser/extensions/extension_event_names.h +++ b/chrome/browser/extensions/extension_event_names.h @@ -45,9 +45,6 @@ extern const char kOnDownloadCreated[]; extern const char kOnDownloadChanged[]; extern const char kOnDownloadErased[]; -// Settings. -extern const char kOnSettingsChanged[]; - }; // namespace extension_event_names #endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_EVENT_NAMES_H_ diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc index 193540f2..e23bba9 100644 --- a/chrome/browser/extensions/extension_service.cc +++ b/chrome/browser/extensions/extension_service.cc @@ -591,7 +591,8 @@ ExtensionService::ExtensionService(Profile* profile, : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), profile_(profile), extension_prefs_(extension_prefs), - extension_settings_frontend_(profile), + extension_settings_frontend_( + profile->GetPath().AppendASCII(kSettingsDirectoryName)), pending_extension_manager_(*ALLOW_THIS_IN_INITIALIZER_LIST(this)), install_directory_(install_directory), extensions_enabled_(extensions_enabled), diff --git a/chrome/browser/extensions/extension_setting_changes.cc b/chrome/browser/extensions/extension_setting_changes.cc deleted file mode 100644 index ce1a95c..0000000 --- a/chrome/browser/extensions/extension_setting_changes.cc +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2011 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 "chrome/browser/extensions/extension_setting_changes.h" - -#include "base/json/json_writer.h" - -void ExtensionSettingChanges::Builder::AppendChange( - const std::string& key, Value* old_value, Value* new_value) { - DictionaryValue* change = new DictionaryValue(); - changes_.Append(change); - - change->SetString("key", key); - if (old_value) { - change->Set("oldValue", old_value); - } - if (new_value) { - change->Set("newValue", new_value); - } -} - -ExtensionSettingChanges ExtensionSettingChanges::Builder::Build() { - return ExtensionSettingChanges(&changes_); -} - -ExtensionSettingChanges::ExtensionSettingChanges(ListValue* changes) - : inner_(new Inner()) { - inner_->changes_.Swap(changes); -} - -ExtensionSettingChanges::~ExtensionSettingChanges() {} - -std::string ExtensionSettingChanges::ToJson() const { - std::string changes_json; - base::JSONWriter::Write(&inner_->changes_, false, &changes_json); - return changes_json; -} diff --git a/chrome/browser/extensions/extension_setting_changes.h b/chrome/browser/extensions/extension_setting_changes.h deleted file mode 100644 index af9f363..0000000 --- a/chrome/browser/extensions/extension_setting_changes.h +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) 2011 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. - -#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTING_CHANGES_H_ -#define CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTING_CHANGES_H_ -#pragma once - -#include "base/memory/ref_counted.h" -#include "base/values.h" - -// A list of changes to extension settings. Create using the Builder class. -// The Changes object is thread-safe and efficient to copy, while the Builder -// object isn't. -class ExtensionSettingChanges { - public: - // Non-thread-safe builder for ExtensionSettingChanges. - class Builder { - public: - Builder() {} - - // Appends a change for a setting. - void AppendChange( - const std::string& key, - // Ownership taken. May be NULL to imply there is no old value. - Value* old_value, - // Ownership taken. May be NULL to imply there is no new value. - Value* new_value); - - // Creates an ExtensionSettingChanges object. The Builder should not be - // used after calling this. - ExtensionSettingChanges Build(); - - private: - ListValue changes_; - - DISALLOW_COPY_AND_ASSIGN(Builder); - }; - - ~ExtensionSettingChanges(); - - // Gets the JSON serialized form of the changes, for example: - // [ { "key": "foo", "oldValue": "bar", "newValue": "baz" } ] - std::string ToJson() const; - - private: - // Create using the Builder class. |changes| will be cleared. - explicit ExtensionSettingChanges(ListValue* changes); - - // Ref-counted inner class, a wrapper over a non-thread-safe string. - class Inner : public base::RefCountedThreadSafe<Inner> { - public: - Inner() {} - - // swap() this with the changes from the Builder after construction. - ListValue changes_; - - private: - virtual ~Inner() {} - friend class base::RefCountedThreadSafe<Inner>; - }; - - scoped_refptr<Inner> inner_; -}; - -#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTING_CHANGES_H_ diff --git a/chrome/browser/extensions/extension_settings_api.cc b/chrome/browser/extensions/extension_settings_api.cc index 64526d7..5da832f 100644 --- a/chrome/browser/extensions/extension_settings_api.cc +++ b/chrome/browser/extensions/extension_settings_api.cc @@ -25,7 +25,7 @@ bool SettingsFunction::RunImpl() { void SettingsFunction::RunWithBackendOnFileThread( ExtensionSettingsBackend* backend) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - bool success = RunWithStorage(backend, backend->GetStorage(extension_id())); + bool success = RunWithStorage(backend->GetStorage(extension_id())); BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, @@ -33,35 +33,13 @@ void SettingsFunction::RunWithBackendOnFileThread( } bool SettingsFunction::UseResult( - ExtensionSettingsBackend* backend, const ExtensionSettingsStorage::Result& storage_result) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); if (storage_result.HasError()) { error_ = storage_result.GetError(); return false; } - - const DictionaryValue* settings = storage_result.GetSettings(); - if (settings) { - result_.reset(settings->DeepCopy()); - } - - const std::set<std::string>* changed_keys = storage_result.GetChangedKeys(); - if (changed_keys && !changed_keys->empty()) { - ExtensionSettingChanges::Builder changes; - for (std::set<std::string>::const_iterator it = changed_keys->begin(); - it != changed_keys->end(); ++it) { - const Value* old_value = storage_result.GetOldValue(*it); - const Value* new_value = storage_result.GetNewValue(*it); - changes.AppendChange( - *it, - old_value ? old_value->DeepCopy() : NULL, - new_value ? new_value->DeepCopy() : NULL); - } - backend->TriggerOnSettingsChanged( - profile(), extension_id(), changes.Build()); - } - + DictionaryValue* settings = storage_result.GetSettings(); + result_.reset(settings == NULL ? NULL : settings->DeepCopy()); return true; } @@ -80,57 +58,47 @@ static void AddAllStringValues( } bool GetSettingsFunction::RunWithStorage( - ExtensionSettingsBackend* backend, ExtensionSettingsStorage* storage) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); Value *input; EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &input)); std::string as_string; ListValue* as_list; if (input->IsType(Value::TYPE_NULL)) { - return UseResult(backend, storage->Get()); + return UseResult(storage->Get()); } else if (input->GetAsString(&as_string)) { - return UseResult(backend, storage->Get(as_string)); + return UseResult(storage->Get(as_string)); } else if (input->GetAsList(&as_list)) { std::vector<std::string> string_list; AddAllStringValues(*as_list, &string_list); - return UseResult(backend, storage->Get(string_list)); + return UseResult(storage->Get(string_list)); } - return UseResult( - backend, ExtensionSettingsStorage::Result(kUnsupportedArgumentType)); + return UseResult(ExtensionSettingsStorage::Result(kUnsupportedArgumentType)); } bool SetSettingsFunction::RunWithStorage( - ExtensionSettingsBackend* backend, ExtensionSettingsStorage* storage) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); DictionaryValue *input; EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &input)); - return UseResult(backend, storage->Set(*input)); + return UseResult(storage->Set(*input)); } bool RemoveSettingsFunction::RunWithStorage( - ExtensionSettingsBackend* backend, ExtensionSettingsStorage* storage) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); Value *input; EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &input)); std::string as_string; ListValue* as_list; if (input->GetAsString(&as_string)) { - return UseResult(backend, storage->Remove(as_string)); + return UseResult(storage->Remove(as_string)); } else if (input->GetAsList(&as_list)) { std::vector<std::string> string_list; AddAllStringValues(*as_list, &string_list); - return UseResult(backend, storage->Remove(string_list)); + return UseResult(storage->Remove(string_list)); } - return UseResult( - backend, ExtensionSettingsStorage::Result(kUnsupportedArgumentType)); + return UseResult(ExtensionSettingsStorage::Result(kUnsupportedArgumentType)); } bool ClearSettingsFunction::RunWithStorage( - ExtensionSettingsBackend* backend, ExtensionSettingsStorage* storage) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - return UseResult(backend, storage->Clear()); + return UseResult(storage->Clear()); } diff --git a/chrome/browser/extensions/extension_settings_api.h b/chrome/browser/extensions/extension_settings_api.h index 15f3fcb..886319b 100644 --- a/chrome/browser/extensions/extension_settings_api.h +++ b/chrome/browser/extensions/extension_settings_api.h @@ -21,15 +21,11 @@ class SettingsFunction : public AsyncExtensionFunction { // // Implementations should fill in args themselves, though (like RunImpl) // may return false to imply failure. - virtual bool RunWithStorage( - ExtensionSettingsBackend* backend, - ExtensionSettingsStorage* storage) = 0; + virtual bool RunWithStorage(ExtensionSettingsStorage* storage) = 0; // Sets error_ or result_ depending on the value of a storage Result, and // returns whether the Result implies success (i.e. !error). - bool UseResult( - ExtensionSettingsBackend* backend, - const ExtensionSettingsStorage::Result& storage_result); + bool UseResult(const ExtensionSettingsStorage::Result& storage_result); private: // Called via PostTask from RunImpl. Calls RunWithStorage and then @@ -42,9 +38,7 @@ class GetSettingsFunction : public SettingsFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.settings.get"); protected: - virtual bool RunWithStorage( - ExtensionSettingsBackend* backend, - ExtensionSettingsStorage* storage) OVERRIDE; + virtual bool RunWithStorage(ExtensionSettingsStorage* storage) OVERRIDE; }; class SetSettingsFunction : public SettingsFunction { @@ -52,9 +46,7 @@ class SetSettingsFunction : public SettingsFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.settings.set"); protected: - virtual bool RunWithStorage( - ExtensionSettingsBackend* backend, - ExtensionSettingsStorage* storage) OVERRIDE; + virtual bool RunWithStorage(ExtensionSettingsStorage* storage) OVERRIDE; }; class RemoveSettingsFunction : public SettingsFunction { @@ -62,9 +54,7 @@ class RemoveSettingsFunction : public SettingsFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.settings.remove"); protected: - virtual bool RunWithStorage( - ExtensionSettingsBackend* backend, - ExtensionSettingsStorage* storage) OVERRIDE; + virtual bool RunWithStorage(ExtensionSettingsStorage* storage) OVERRIDE; }; class ClearSettingsFunction : public SettingsFunction { @@ -72,9 +62,7 @@ class ClearSettingsFunction : public SettingsFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.settings.clear"); protected: - virtual bool RunWithStorage( - ExtensionSettingsBackend* backend, - ExtensionSettingsStorage* storage) OVERRIDE; + virtual bool RunWithStorage(ExtensionSettingsStorage* storage) OVERRIDE; }; #endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_API_H_ diff --git a/chrome/browser/extensions/extension_settings_apitest.cc b/chrome/browser/extensions/extension_settings_apitest.cc index ad1a1fb..213d921 100644 --- a/chrome/browser/extensions/extension_settings_apitest.cc +++ b/chrome/browser/extensions/extension_settings_apitest.cc @@ -2,36 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/bind.h" #include "base/command_line.h" #include "base/json/json_writer.h" #include "chrome/browser/extensions/extension_apitest.h" -#include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/extensions/extension_settings_backend.h" -#include "chrome/browser/extensions/extension_settings_sync_util.h" #include "chrome/browser/extensions/extension_test_message_listener.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/sync/api/sync_change.h" -#include "chrome/browser/sync/api/sync_change_processor.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/chrome_switches.h" #include "chrome/test/base/ui_test_utils.h" -namespace { - -class NoopSyncChangeProcessor : public SyncChangeProcessor { - public: - virtual SyncError ProcessSyncChanges( - const tracked_objects::Location& from_here, - const SyncChangeList& change_list) OVERRIDE { - return SyncError(); - } - - virtual ~NoopSyncChangeProcessor() {}; -}; - -} // namespace - class ExtensionSettingsApiTest : public ExtensionApiTest { protected: void ReplyWhenSatisfied( @@ -41,11 +20,11 @@ class ExtensionSettingsApiTest : public ExtensionApiTest { normal_action, incognito_action, NULL, false); } - const Extension* LoadAndReplyWhenSatisfied( + void LoadAndReplyWhenSatisfied( const std::string& normal_action, const std::string& incognito_action, const std::string& extension_dir) { - return MaybeLoadAndReplyWhenSatisfied( + MaybeLoadAndReplyWhenSatisfied( normal_action, incognito_action, &extension_dir, false); } @@ -55,26 +34,8 @@ class ExtensionSettingsApiTest : public ExtensionApiTest { MaybeLoadAndReplyWhenSatisfied(normal_action, incognito_action, NULL, true); } - void InitSync(SyncChangeProcessor* sync_processor) { - browser()->profile()->GetExtensionService()-> - extension_settings_frontend()->RunWithBackend(base::Bind( - &ExtensionSettingsApiTest::InitSyncWithBackend, - this, - sync_processor)); - MessageLoop::current()->RunAllPending(); - } - - void SendChanges(const SyncChangeList& change_list) { - browser()->profile()->GetExtensionService()-> - extension_settings_frontend()->RunWithBackend(base::Bind( - &ExtensionSettingsApiTest::SendChangesToBackend, - this, - change_list)); - MessageLoop::current()->RunAllPending(); - } - private: - const Extension* MaybeLoadAndReplyWhenSatisfied( + void MaybeLoadAndReplyWhenSatisfied( const std::string& normal_action, const std::string& incognito_action, // May be NULL to imply not loading the extension. @@ -85,11 +46,9 @@ class ExtensionSettingsApiTest : public ExtensionApiTest { // Only load the extension after the listeners have been set up, to avoid // initialisation race conditions. - const Extension* extension = NULL; if (extension_dir) { - extension = LoadExtensionIncognito( - test_data_dir_.AppendASCII("settings").AppendASCII(*extension_dir)); - EXPECT_TRUE(extension); + ASSERT_TRUE(LoadExtensionIncognito(test_data_dir_ + .AppendASCII("settings").AppendASCII(*extension_dir))); } EXPECT_TRUE(listener.WaitUntilSatisfied()); @@ -97,7 +56,6 @@ class ExtensionSettingsApiTest : public ExtensionApiTest { listener.Reply(CreateMessage(normal_action, is_final_action)); listener_incognito.Reply(CreateMessage(incognito_action, is_final_action)); - return extension; } std::string CreateMessage(const std::string& action, bool is_final_action) { @@ -108,19 +66,6 @@ class ExtensionSettingsApiTest : public ExtensionApiTest { base::JSONWriter::Write(message.get(), false, &message_json); return message_json; } - - void InitSyncWithBackend( - SyncChangeProcessor* sync_processor, ExtensionSettingsBackend* backend) { - EXPECT_FALSE(backend->MergeDataAndStartSyncing( - syncable::EXTENSION_SETTINGS, - SyncDataList(), - sync_processor).IsSet()); - } - - void SendChangesToBackend( - const SyncChangeList& change_list, ExtensionSettingsBackend* backend) { - EXPECT_FALSE(backend->ProcessSyncChanges(FROM_HERE, change_list).IsSet()); - } }; IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, SimpleTest) { @@ -145,93 +90,15 @@ IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, SplitModeIncognito) { browser()->profile()->GetOffTheRecordProfile()); LoadAndReplyWhenSatisfied("assertEmpty", "assertEmpty", "split_incognito"); - ReplyWhenSatisfied("noop", "setFoo"); - ReplyWhenSatisfied("assertFoo", "assertFoo"); + ReplyWhenSatisfied("noop", "setFoobar"); + ReplyWhenSatisfied("assertFoobar", "assertFoobar"); ReplyWhenSatisfied("clear", "noop"); ReplyWhenSatisfied("assertEmpty", "assertEmpty"); - ReplyWhenSatisfied("setFoo", "noop"); - ReplyWhenSatisfied("assertFoo", "assertFoo"); + ReplyWhenSatisfied("setFoobar", "noop"); + ReplyWhenSatisfied("assertFoobar", "assertFoobar"); ReplyWhenSatisfied("noop", "removeFoo"); FinalReplyWhenSatisfied("assertEmpty", "assertEmpty"); EXPECT_TRUE(catcher.GetNextResult()) << catcher.message(); EXPECT_TRUE(catcher_incognito.GetNextResult()) << catcher.message(); } - -IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, - OnChangedNotificationsBetweenBackgroundPages) { - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableExperimentalExtensionApis); - - // We need 2 ResultCatchers because we'll be running the same test in both - // regular and incognito mode. - ResultCatcher catcher, catcher_incognito; - catcher.RestrictToProfile(browser()->profile()); - catcher_incognito.RestrictToProfile( - browser()->profile()->GetOffTheRecordProfile()); - - LoadAndReplyWhenSatisfied( - "assertNoNotifications", "assertNoNotifications", "split_incognito"); - ReplyWhenSatisfied("noop", "setFoo"); - ReplyWhenSatisfied("assertAddFooNotification", "assertNoNotifications"); - ReplyWhenSatisfied("clearNotifications", "noop"); - ReplyWhenSatisfied("removeFoo", "noop"); - FinalReplyWhenSatisfied( - "assertNoNotifications", "assertDeleteFooNotification"); - - EXPECT_TRUE(catcher.GetNextResult()) << catcher.message(); - EXPECT_TRUE(catcher_incognito.GetNextResult()) << catcher.message(); -} - -#if defined(OS_WIN) -// Very flaky on Windows, so just disable. crbug.com/101110 -#define MAYBE_OnChangedNotificationsFromSync \ - DISABLED_OnChangedNotificationsFromSync -#else -// Possibly flaky on other platforms, though haven't observed it yet. -#define MAYBE_OnChangedNotificationsFromSync \ - FLAKY_OnChangedNotificationsFromSync -#endif - -IN_PROC_BROWSER_TEST_F(ExtensionSettingsApiTest, - MAYBE_OnChangedNotificationsFromSync) { - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableExperimentalExtensionApis); - - // We need 2 ResultCatchers because we'll be running the same test in both - // regular and incognito mode. - ResultCatcher catcher, catcher_incognito; - catcher.RestrictToProfile(browser()->profile()); - catcher_incognito.RestrictToProfile( - browser()->profile()->GetOffTheRecordProfile()); - - const Extension* extension = - LoadAndReplyWhenSatisfied( - "assertNoNotifications", "assertNoNotifications", "split_incognito"); - const std::string& extension_id = extension->id(); - - NoopSyncChangeProcessor sync_processor; - InitSync(&sync_processor); - - // Set "foo" to "bar" via sync. - SyncChangeList sync_changes; - StringValue bar("bar"); - sync_changes.push_back(extension_settings_sync_util::CreateAdd( - extension_id, "foo", bar)); - SendChanges(sync_changes); - - ReplyWhenSatisfied("assertAddFooNotification", "assertAddFooNotification"); - ReplyWhenSatisfied("clearNotifications", "clearNotifications"); - - // Remove "foo" via sync. - sync_changes.clear(); - sync_changes.push_back(extension_settings_sync_util::CreateDelete( - extension_id, "foo")); - SendChanges(sync_changes); - - FinalReplyWhenSatisfied( - "assertDeleteFooNotification", "assertDeleteFooNotification"); - - EXPECT_TRUE(catcher.GetNextResult()) << catcher.message(); - EXPECT_TRUE(catcher_incognito.GetNextResult()) << catcher.message(); -} diff --git a/chrome/browser/extensions/extension_settings_backend.cc b/chrome/browser/extensions/extension_settings_backend.cc index 56bd944..ef97825 100644 --- a/chrome/browser/extensions/extension_settings_backend.cc +++ b/chrome/browser/extensions/extension_settings_backend.cc @@ -36,12 +36,8 @@ const size_t kMaxSettingKeys = 512; } // namespace -ExtensionSettingsBackend::ExtensionSettingsBackend( - const FilePath& base_path, - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> >& - observers) +ExtensionSettingsBackend::ExtensionSettingsBackend(const FilePath& base_path) : base_path_(base_path), - observers_(observers), sync_processor_(NULL) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); } @@ -87,10 +83,7 @@ ExtensionSettingsBackend::GetOrCreateStorageWithSyncData( syncable_storage = linked_ptr<SyncableExtensionSettingsStorage>( - new SyncableExtensionSettingsStorage( - observers_, - extension_id, - storage)); + new SyncableExtensionSettingsStorage(extension_id, storage)); if (sync_processor_) { // TODO(kalman): do something if StartSyncing fails. ignore_result(syncable_storage->StartSyncing(sync_data, sync_processor_)); @@ -115,17 +108,6 @@ void ExtensionSettingsBackend::DeleteExtensionData( storage_objs_.erase(maybe_storage); } -void ExtensionSettingsBackend::TriggerOnSettingsChanged( - Profile* profile, - const std::string& extension_id, - const ExtensionSettingChanges& changes) const { - observers_->Notify( - &ExtensionSettingsObserver::OnSettingsChanged, - profile, - extension_id, - changes); -} - std::set<std::string> ExtensionSettingsBackend::GetKnownExtensionIDs() const { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); std::set<std::string> result; @@ -179,7 +161,7 @@ SyncDataList ExtensionSettingsBackend::GetAllSyncData( continue; } - const DictionaryValue* settings = maybe_settings.GetSettings(); + DictionaryValue* settings = maybe_settings.GetSettings(); for (DictionaryValue::key_iterator key_it = settings->begin_keys(); key_it != settings->end_keys(); ++key_it) { Value *value = NULL; diff --git a/chrome/browser/extensions/extension_settings_backend.h b/chrome/browser/extensions/extension_settings_backend.h index 81c1067..28b0e8a 100644 --- a/chrome/browser/extensions/extension_settings_backend.h +++ b/chrome/browser/extensions/extension_settings_backend.h @@ -10,9 +10,7 @@ #include "base/file_path.h" #include "base/memory/linked_ptr.h" #include "base/memory/ref_counted.h" -#include "base/observer_list_threadsafe.h" #include "base/task.h" -#include "chrome/browser/extensions/extension_settings_observer.h" #include "chrome/browser/extensions/syncable_extension_settings_storage.h" #include "chrome/browser/sync/api/syncable_service.h" @@ -21,13 +19,9 @@ // Lives entirely on the FILE thread. class ExtensionSettingsBackend : public SyncableService { public: - // |base_path| is the base of the extension settings directory, so the - // databases will be at base_path/extension_id. - // |observers| is the list of observers to settings changes. - explicit ExtensionSettingsBackend( - const FilePath& base_path, - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> >& - observers); + // File path is the base of the extension settings directory. + // The databases will be at base_path/extension_id. + explicit ExtensionSettingsBackend(const FilePath& base_path); virtual ~ExtensionSettingsBackend(); @@ -43,13 +37,6 @@ class ExtensionSettingsBackend : public SyncableService { // Deletes all setting data for an extension. Call on the FILE thread. void DeleteExtensionData(const std::string& extension_id); - // Sends a change event to the observer list. |profile| is the profile which - // generated the change. Must be called on the FILE thread. - void TriggerOnSettingsChanged( - Profile* profile, - const std::string& extension_id, - const ExtensionSettingChanges& changes) const; - // SyncableService implementation. virtual SyncDataList GetAllSyncData(syncable::ModelType type) const OVERRIDE; virtual SyncError MergeDataAndStartSyncing( @@ -77,10 +64,6 @@ class ExtensionSettingsBackend : public SyncableService { // The base file path to create any leveldb databases at. const FilePath base_path_; - // The list of observers to settings changes. - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> > - observers_; - // A cache of ExtensionSettingsStorage objects that have already been created. // Ensure that there is only ever one created per extension. typedef std::map<std::string, linked_ptr<SyncableExtensionSettingsStorage> > diff --git a/chrome/browser/extensions/extension_settings_frontend.cc b/chrome/browser/extensions/extension_settings_frontend.cc index 84bfaac..4963e92 100644 --- a/chrome/browser/extensions/extension_settings_frontend.cc +++ b/chrome/browser/extensions/extension_settings_frontend.cc @@ -6,117 +6,20 @@ #include "base/bind.h" #include "base/file_path.h" -#include "chrome/browser/extensions/extension_event_names.h" -#include "chrome/browser/extensions/extension_event_router.h" -#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_settings_backend.h" -#include "chrome/browser/profiles/profile.h" #include "content/browser/browser_thread.h" -#include "content/public/browser/notification_service.h" -class ExtensionSettingsFrontend::DefaultObserver - : public ExtensionSettingsObserver { - public: - explicit DefaultObserver(Profile* profile) : target_profile_(profile) {} - virtual ~DefaultObserver() {} - - virtual void OnSettingsChanged( - const Profile* origin_profile, - const std::string& extension_id, - const ExtensionSettingChanges& changes) OVERRIDE { - if (origin_profile != target_profile_) { - target_profile_->GetExtensionEventRouter()->DispatchEventToExtension( - extension_id, - extension_event_names::kOnSettingsChanged, - // This is the list of function arguments to pass to the onChanged - // handler of extensions, a single argument with the list of changes. - std::string("[") + changes.ToJson() + "]", - target_profile_, - GURL()); - } - } - - private: - Profile* target_profile_; -}; - -class ExtensionSettingsFrontend::Core - : public base::RefCountedThreadSafe<Core> { - public: - explicit Core( - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> >& - observers) - : observers_(observers), backend_(NULL) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - } - - // Does any FILE thread specific initialization, such as construction of - // |backend_|. Must be called before any call to - // RunWithBackendOnFileThread(). - void InitOnFileThread(const FilePath& base_path) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DCHECK(!backend_); - backend_ = new ExtensionSettingsBackend(base_path, observers_); - } - - // Runs |callback| with the extension backend. - void RunWithBackendOnFileThread(const BackendCallback& callback) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DCHECK(backend_); - callback.Run(backend_); - } - - private: - virtual ~Core() { - if (BrowserThread::CurrentlyOn(BrowserThread::FILE)) { - delete backend_; - } else if (BrowserThread::CurrentlyOn(BrowserThread::UI)) { - BrowserThread::DeleteSoon(BrowserThread::FILE, FROM_HERE, backend_); - } else { - NOTREACHED(); - } - } - - friend class base::RefCountedThreadSafe<Core>; - - // Observers to settings changes (thread safe). - scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> > - observers_; - - // Lives on the FILE thread. - ExtensionSettingsBackend* backend_; - - DISALLOW_COPY_AND_ASSIGN(Core); -}; - -ExtensionSettingsFrontend::ExtensionSettingsFrontend(Profile* profile) - : profile_(profile), - observers_(new ObserverListThreadSafe<ExtensionSettingsObserver>()), - core_(new ExtensionSettingsFrontend::Core(observers_.get())) { +ExtensionSettingsFrontend::ExtensionSettingsFrontend( + const FilePath& base_path) + : core_(new ExtensionSettingsFrontend::Core()) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(!profile->IsOffTheRecord()); - - // This class listens to all PROFILE_{CREATED,DESTROYED} events but we're - // only interested in those for the original Profile given on construction - // and its incognito version. - registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED, - content::NotificationService::AllBrowserContextsAndSources()); - registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED, - content::NotificationService::AllBrowserContextsAndSources()); - OnProfileCreated(profile); - BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, base::Bind( &ExtensionSettingsFrontend::Core::InitOnFileThread, core_.get(), - profile->GetPath().AppendASCII( - ExtensionService::kSettingsDirectoryName))); -} - -ExtensionSettingsFrontend::~ExtensionSettingsFrontend() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + base_path)); } void ExtensionSettingsFrontend::RunWithBackend( @@ -131,66 +34,34 @@ void ExtensionSettingsFrontend::RunWithBackend( callback)); } -void ExtensionSettingsFrontend::AddObserver( - ExtensionSettingsObserver* observer) { - observers_->AddObserver(observer); -} - -void ExtensionSettingsFrontend::RemoveObserver( - ExtensionSettingsObserver* observer) { - observers_->RemoveObserver(observer); -} - -void ExtensionSettingsFrontend::Observe( - int type, - const content::NotificationSource& source, - const content::NotificationDetails& details) { +ExtensionSettingsFrontend::~ExtensionSettingsFrontend() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - switch (type) { - case chrome::NOTIFICATION_PROFILE_CREATED: - OnProfileCreated(content::Source<Profile>(source).ptr()); - break; - case chrome::NOTIFICATION_PROFILE_DESTROYED: - OnProfileDestroyed(content::Source<Profile>(source).ptr()); - break; - default: - NOTREACHED(); - } } -void ExtensionSettingsFrontend::OnProfileCreated(Profile* new_profile) { +ExtensionSettingsFrontend::Core::Core() : backend_(NULL) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (new_profile == profile_) { - ClearDefaultObserver(&original_profile_observer); - SetDefaultObserver(new_profile, &original_profile_observer); - } else if (new_profile->GetOriginalProfile() == profile_) { - DCHECK(new_profile->IsOffTheRecord()); - ClearDefaultObserver(&incognito_profile_observer_); - SetDefaultObserver(new_profile, &incognito_profile_observer_); - } } -void ExtensionSettingsFrontend::OnProfileDestroyed(Profile* old_profile) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (old_profile == profile_) { - ClearDefaultObserver(&original_profile_observer); - } else if (old_profile->GetOriginalProfile() == profile_) { - DCHECK(old_profile->IsOffTheRecord()); - ClearDefaultObserver(&incognito_profile_observer_); - } +void ExtensionSettingsFrontend::Core::InitOnFileThread( + const FilePath& base_path) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + DCHECK(!backend_); + backend_ = new ExtensionSettingsBackend(base_path); } -void ExtensionSettingsFrontend::SetDefaultObserver( - Profile* profile, scoped_ptr<DefaultObserver>* observer) { - DCHECK(!observer->get()); - observer->reset(new DefaultObserver(profile)); - AddObserver(observer->get()); +void ExtensionSettingsFrontend::Core::RunWithBackendOnFileThread( + const BackendCallback& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + DCHECK(backend_); + callback.Run(backend_); } -void ExtensionSettingsFrontend::ClearDefaultObserver( - scoped_ptr<DefaultObserver>* observer) { - if (observer->get()) { - RemoveObserver(observer->get()); - observer->reset(); +ExtensionSettingsFrontend::Core::~Core() { + if (BrowserThread::CurrentlyOn(BrowserThread::FILE)) { + delete backend_; + } else if (BrowserThread::CurrentlyOn(BrowserThread::UI)) { + BrowserThread::DeleteSoon(BrowserThread::FILE, FROM_HERE, backend_); + } else { + NOTREACHED(); } } diff --git a/chrome/browser/extensions/extension_settings_frontend.h b/chrome/browser/extensions/extension_settings_frontend.h index 104da18..7fe3dc0 100644 --- a/chrome/browser/extensions/extension_settings_frontend.h +++ b/chrome/browser/extensions/extension_settings_frontend.h @@ -6,83 +6,56 @@ #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_FRONTEND_H_ #pragma once -#include "base/callback.h" #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" -#include "base/observer_list_threadsafe.h" -#include "chrome/browser/extensions/extension_settings_observer.h" +#include "base/callback.h" #include "chrome/browser/sync/api/syncable_service.h" -#include "content/public/browser/notification_observer.h" -#include "content/public/browser/notification_registrar.h" class FilePath; -class Profile; class ExtensionSettingsBackend; class ExtensionSettingsStorage; // The component of extension settings which runs on the UI thread, as opposed // to ExtensionSettingsBackend which lives on the FILE thread. -class ExtensionSettingsFrontend : public content::NotificationObserver { +class ExtensionSettingsFrontend { public: - explicit ExtensionSettingsFrontend(Profile* profile); - virtual ~ExtensionSettingsFrontend(); + explicit ExtensionSettingsFrontend(const FilePath& base_path); typedef base::Callback<void(ExtensionSettingsBackend*)> BackendCallback; // Runs |callback| on the FILE thread with the extension settings. void RunWithBackend(const BackendCallback& callback); - // Adds an observer to settings changes. - void AddObserver(ExtensionSettingsObserver* observer); - - // Removes an observer to settings changes. - void RemoveObserver(ExtensionSettingsObserver* observer); - - // NotificationObserver implementation. - virtual void Observe( - int type, - const content::NotificationSource& source, - const content::NotificationDetails& details) OVERRIDE; + ~ExtensionSettingsFrontend(); private: - // Observer which sends events to a target profile iff the profile isn't the - // originating profile for the event. - class DefaultObserver; - - // Called when a profile is created. - void OnProfileCreated(Profile* profile); - - // Called when a profile is destroyed. - void OnProfileDestroyed(Profile* profile); + // Ref-counted container for the ExtensionSettingsBackend object. + class Core : public base::RefCountedThreadSafe<Core> { + public: + // Constructed on UI thread. + Core(); - // Creates a scoped DefaultObserver and adds it as an Observer. - void SetDefaultObserver( - Profile* profile, scoped_ptr<DefaultObserver>* observer); + // Does any FILE thread specific initialization, such as construction of + // |backend_|. Must be called before any call to + // RunWithBackendOnFileThread(). + void InitOnFileThread(const FilePath& base_path); - // If a scoped DefaultObserver exists, clears it and removes it as an - // Observer. - void ClearDefaultObserver(scoped_ptr<DefaultObserver>* observer); + // Runs |callback| with the extension backend. + void RunWithBackendOnFileThread(const BackendCallback& callback); - // The (original) Profile this Frontend belongs to. Note that we don't store - // the incognito version of the profile because it will change as it's - // created and destroyed during the lifetime of Chrome. - Profile* const profile_; + private: + // Can be destroyed on either the UI or FILE thread. + virtual ~Core(); + friend class base::RefCountedThreadSafe<Core>; - // List of observers to settings changes. - scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> > observers_; + // Lives on the FILE thread. + ExtensionSettingsBackend* backend_; - // The default original and incognito mode profile observers. - scoped_ptr<DefaultObserver> original_profile_observer; - scoped_ptr<DefaultObserver> incognito_profile_observer_; + DISALLOW_COPY_AND_ASSIGN(Core); + }; - // Ref-counted container for the ExtensionSettingsBackend object. - class Core; scoped_refptr<Core> core_; - // For profile created/destroyed notifications. - content::NotificationRegistrar registrar_; - DISALLOW_COPY_AND_ASSIGN(ExtensionSettingsFrontend); }; diff --git a/chrome/browser/extensions/extension_settings_frontend_unittest.cc b/chrome/browser/extensions/extension_settings_frontend_unittest.cc index d245850..076eda3 100644 --- a/chrome/browser/extensions/extension_settings_frontend_unittest.cc +++ b/chrome/browser/extensions/extension_settings_frontend_unittest.cc @@ -23,13 +23,13 @@ class ExtensionSettingsFrontendTest : public testing::Test { virtual void SetUp() OVERRIDE { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + frontend_.reset(new ExtensionSettingsFrontend(temp_dir_.path())); profile_.reset(new TestingProfile(temp_dir_.path())); - frontend_.reset(new ExtensionSettingsFrontend(profile_.get())); } virtual void TearDown() OVERRIDE { - frontend_.reset(); profile_.reset(); + frontend_.reset(); } protected: @@ -45,8 +45,8 @@ class ExtensionSettingsFrontendTest : public testing::Test { } ScopedTempDir temp_dir_; - scoped_ptr<TestingProfile> profile_; scoped_ptr<ExtensionSettingsFrontend> frontend_; + scoped_ptr<TestingProfile> profile_; private: // Intended as a ExtensionSettingsFrontend::BackendCallback from GetBackend. @@ -77,7 +77,7 @@ TEST_F(ExtensionSettingsFrontendTest, SettingsPreservedAcrossReconstruction) { ASSERT_FALSE(result.HasError()); EXPECT_FALSE(result.GetSettings()->empty()); - frontend_.reset(new ExtensionSettingsFrontend(profile_.get())); + frontend_.reset(new ExtensionSettingsFrontend(temp_dir_.path())); GetBackend(&backend); storage = backend->GetStorage(id); diff --git a/chrome/browser/extensions/extension_settings_leveldb_storage.cc b/chrome/browser/extensions/extension_settings_leveldb_storage.cc index 647c232..5c86c02 100644 --- a/chrome/browser/extensions/extension_settings_leveldb_storage.cc +++ b/chrome/browser/extensions/extension_settings_leveldb_storage.cc @@ -100,7 +100,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Get( if (setting.get()) { settings->SetWithoutPathExpansion(key, setting.release()); } - return Result(settings, NULL, NULL); + return Result(settings, NULL); } ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Get( @@ -124,7 +124,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Get( } } - return Result(settings.release(), NULL, NULL); + return Result(settings.release(), NULL); } ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Get() { @@ -154,7 +154,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Get() { return Result(kGenericOnFailureMessage); } - return Result(settings.release(), NULL, NULL); + return Result(settings.release(), NULL); } ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Set( @@ -168,29 +168,24 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Set( ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Set( const DictionaryValue& settings) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - scoped_ptr<DictionaryValue> old_settings(new DictionaryValue()); scoped_ptr<std::set<std::string> > changed_keys(new std::set<std::string>()); std::string value_as_json; leveldb::WriteBatch batch; for (DictionaryValue::key_iterator it = settings.begin_keys(); it != settings.end_keys(); ++it) { - scoped_ptr<Value> old_value; - if (!ReadFromDb(leveldb::ReadOptions(), *it, &old_value)) { + scoped_ptr<Value> original_value; + if (!ReadFromDb(leveldb::ReadOptions(), *it, &original_value)) { return Result(kGenericOnFailureMessage); } Value* new_value = NULL; settings.GetWithoutPathExpansion(*it, &new_value); - if (!old_value.get() || !old_value->Equals(new_value)) { + if (!original_value.get() || !original_value->Equals(new_value)) { changed_keys->insert(*it); base::JSONWriter::Write(new_value, false, &value_as_json); batch.Put(*it, value_as_json); } - - if (old_value.get()) { - old_settings->SetWithoutPathExpansion(*it, old_value.release()); - } } leveldb::Status status = db_->Write(leveldb::WriteOptions(), &batch); @@ -199,8 +194,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Set( return Result(kGenericOnFailureMessage); } - return Result( - settings.DeepCopy(), old_settings.release(), changed_keys.release()); + return Result(settings.DeepCopy(), changed_keys.release()); } ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Remove( @@ -214,20 +208,18 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Remove( ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Remove( const std::vector<std::string>& keys) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - scoped_ptr<DictionaryValue> old_settings(new DictionaryValue()); scoped_ptr<std::set<std::string> > changed_keys(new std::set<std::string>()); leveldb::WriteBatch batch; for (std::vector<std::string>::const_iterator it = keys.begin(); it != keys.end(); ++it) { - scoped_ptr<Value> old_value; - if (!ReadFromDb(leveldb::ReadOptions(), *it, &old_value)) { + scoped_ptr<Value> original_value; + if (!ReadFromDb(leveldb::ReadOptions(), *it, &original_value)) { return Result(kGenericOnFailureMessage); } - if (old_value.get()) { + if (original_value.get()) { changed_keys->insert(*it); - old_settings->SetWithoutPathExpansion(*it, old_value.release()); batch.Delete(*it); } } @@ -238,12 +230,11 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Remove( return Result(kGenericOnFailureMessage); } - return Result(NULL, old_settings.release(), changed_keys.release()); + return Result(NULL, changed_keys.release()); } ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Clear() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - scoped_ptr<DictionaryValue> old_settings(new DictionaryValue()); scoped_ptr<std::set<std::string> > changed_keys(new std::set<std::string>()); leveldb::ReadOptions options; // All interaction with the db is done on the same thread, so snapshotting @@ -254,17 +245,8 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Clear() { options.snapshot = snapshot.get(); scoped_ptr<leveldb::Iterator> it(db_->NewIterator(options)); for (it->SeekToFirst(); it->Valid(); it->Next()) { - const std::string key = it->key().ToString(); - const std::string old_value_as_json = it->value().ToString(); - changed_keys->insert(key); - Value* old_value = - base::JSONReader().JsonToValue(old_value_as_json, false, false); - if (old_value) { - old_settings->SetWithoutPathExpansion(key, old_value); - } else { - LOG(ERROR) << "Invalid JSON in database: " << old_value_as_json; - } - batch.Delete(key); + changed_keys->insert(it->key().ToString()); + batch.Delete(it->key()); } if (!it->status().ok()) { @@ -278,7 +260,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsLeveldbStorage::Clear() { return Result(kGenericOnFailureMessage); } - return Result(NULL, old_settings.release(), changed_keys.release()); + return Result(NULL, changed_keys.release()); } bool ExtensionSettingsLeveldbStorage::ReadFromDb( diff --git a/chrome/browser/extensions/extension_settings_observer.cc b/chrome/browser/extensions/extension_settings_observer.cc deleted file mode 100644 index d25ae17..0000000 --- a/chrome/browser/extensions/extension_settings_observer.cc +++ /dev/null @@ -1,7 +0,0 @@ -// Copyright (c) 2011 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 "chrome/browser/extensions/extension_settings_observer.h" - -ExtensionSettingsObserver::~ExtensionSettingsObserver() {} diff --git a/chrome/browser/extensions/extension_settings_observer.h b/chrome/browser/extensions/extension_settings_observer.h deleted file mode 100644 index 7cc2461..0000000 --- a/chrome/browser/extensions/extension_settings_observer.h +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) 2011 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. - -#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_OBSERVER_H_ -#define CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_OBSERVER_H_ -#pragma once - -#include "chrome/browser/extensions/extension_setting_changes.h" - -class Profile; - -// Interface for classes that listen to changes to extension settings. -class ExtensionSettingsObserver { - public: - // Called when a list of settings have changed for an extension. - // - // |profile| is the profile of the event originator, or NULL if the event - // didn't originate from a background page. This is so that events can avoid - // being sent back to the background page which made the change. When the - // settings API is exposed to content scripts, this will need to be changed. - virtual void OnSettingsChanged( - const Profile* profile, - const std::string& extension_id, - const ExtensionSettingChanges& changes) = 0; - - protected: - virtual ~ExtensionSettingsObserver(); -}; - -#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_SETTINGS_OBSERVER_H_ diff --git a/chrome/browser/extensions/extension_settings_storage.cc b/chrome/browser/extensions/extension_settings_storage.cc index 3259748..882efe8 100644 --- a/chrome/browser/extensions/extension_settings_storage.cc +++ b/chrome/browser/extensions/extension_settings_storage.cc @@ -8,57 +8,27 @@ // Implementation of ExtensionSettingsStorage::Result ExtensionSettingsStorage::Result::Result( - DictionaryValue* settings, - DictionaryValue* old_settings, - std::set<std::string>* changed_keys) - : inner_(new Inner(settings, old_settings, changed_keys, std::string())) {} + DictionaryValue* settings, std::set<std::string>* changed_keys) + : inner_(new Inner(settings, changed_keys, std::string())) {} ExtensionSettingsStorage::Result::Result(const std::string& error) - : inner_(new Inner(NULL, NULL, new std::set<std::string>(), error)) { + : inner_(new Inner(NULL, new std::set<std::string>(), error)) { DCHECK(!error.empty()); } ExtensionSettingsStorage::Result::~Result() {} -const DictionaryValue* ExtensionSettingsStorage::Result::GetSettings() const { +DictionaryValue* ExtensionSettingsStorage::Result::GetSettings() const { DCHECK(!HasError()); return inner_->settings_.get(); } -const std::set<std::string>* +std::set<std::string>* ExtensionSettingsStorage::Result::GetChangedKeys() const { DCHECK(!HasError()); return inner_->changed_keys_.get(); } -const Value* ExtensionSettingsStorage::Result::GetOldValue( - const std::string& key) const { - DCHECK(!HasError()); - if (!inner_->changed_keys_.get()) { - return NULL; - } - Value* old_value = NULL; - if (inner_->changed_keys_->count(key)) { - inner_->old_settings_->GetWithoutPathExpansion(key, &old_value); - } else if (inner_->settings_.get()) { - inner_->settings_->GetWithoutPathExpansion(key, &old_value); - } - return old_value; -} - -const Value* ExtensionSettingsStorage::Result::GetNewValue( - const std::string& key) const { - DCHECK(!HasError()); - if (!inner_->changed_keys_.get()) { - return NULL; - } - Value* new_value = NULL; - if (inner_->settings_.get()) { - inner_->settings_->GetWithoutPathExpansion(key, &new_value); - } - return new_value; -} - bool ExtensionSettingsStorage::Result::HasError() const { return !inner_->error_.empty(); } @@ -70,12 +40,8 @@ const std::string& ExtensionSettingsStorage::Result::GetError() const { ExtensionSettingsStorage::Result::Inner::Inner( DictionaryValue* settings, - DictionaryValue* old_settings, std::set<std::string>* changed_keys, const std::string& error) - : settings_(settings), - old_settings_(old_settings), - changed_keys_(changed_keys), - error_(error) {} + : settings_(settings), changed_keys_(changed_keys), error_(error) {} ExtensionSettingsStorage::Result::Inner::~Inner() {} diff --git a/chrome/browser/extensions/extension_settings_storage.h b/chrome/browser/extensions/extension_settings_storage.h index 106f8fb..ba728bc 100644 --- a/chrome/browser/extensions/extension_settings_storage.h +++ b/chrome/browser/extensions/extension_settings_storage.h @@ -23,37 +23,25 @@ class ExtensionSettingsStorage { // Supports lightweight copying. class Result { public: - // Ownership of all arguments are taken. + // Ownership of |settings| and |changed_keys| taken. // |settings| may be NULL when the result is for an operation which // generates no setting values (e.g. Remove(), Clear()). - // |changed_keys| and |old_settings| may be NULL when the result is for an - // operation which cannot change settings (e.g. Get()). - Result( - DictionaryValue* settings, - DictionaryValue* old_settings, - std::set<std::string>* changed_keys); + // |changed_keys| may be NULL when the result is for an operation which + // cannot change settings (e.g. Get()). + Result(DictionaryValue* settings, std::set<std::string>* changed_keys); explicit Result(const std::string& error); ~Result(); // The dictionary result of the computation. NULL does not imply invalid; // HasError() should be used to test this. - // Ownership remains with this. - const DictionaryValue* GetSettings() const; + // Ownership remains with with the Result. + DictionaryValue* GetSettings() const; - // Gets the set of setting keys which changed as a result of the + // Gets the list of setting keys which changed as a result of the // computation. This includes all settings that existed and removed, all - // settings which changed when set, and all setting keys cleared. May be - // NULL for operations which cannot change settings, such as Get(). - const std::set<std::string>* GetChangedKeys() const; - - // Gets the value of a setting prior to the operation which generated - // this Result, or NULL if the setting had no original value or this Result - // is from an operation that didn't modify the settings (such as Get()). - // Ownerships remains with this. - const Value* GetOldValue(const std::string& key) const; - - // Like GetOldValue, but gets the new value. - const Value* GetNewValue(const std::string& key) const; + // settings which changed when set, and all setting keys cleared. + // May be NULL for operations which cannot change settings, such as Get(). + std::set<std::string>* GetChangedKeys() const; // Whether there was an error in the computation. If so, the results of // GetSettings and ReleaseSettings are not valid. @@ -67,13 +55,11 @@ class ExtensionSettingsStorage { // Empty error implies no error. Inner( DictionaryValue* settings, - DictionaryValue* old_settings, std::set<std::string>* changed_keys, const std::string& error); ~Inner(); const scoped_ptr<DictionaryValue> settings_; - const scoped_ptr<DictionaryValue> old_settings_; const scoped_ptr<std::set<std::string> > changed_keys_; const std::string error_; }; diff --git a/chrome/browser/extensions/extension_settings_storage_cache.cc b/chrome/browser/extensions/extension_settings_storage_cache.cc index 2df99ed..f20f068 100644 --- a/chrome/browser/extensions/extension_settings_storage_cache.cc +++ b/chrome/browser/extensions/extension_settings_storage_cache.cc @@ -15,7 +15,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsStorageCache::Get( if (GetFromCache(key, &value)) { DictionaryValue* settings = new DictionaryValue(); settings->SetWithoutPathExpansion(key, value); - return Result(settings, NULL, NULL); + return Result(settings, NULL); } Result result = delegate_->Get(key); @@ -43,7 +43,7 @@ ExtensionSettingsStorage::Result ExtensionSettingsStorageCache::Get( } if (missing_keys.empty()) { - return Result(from_cache.release(), NULL, NULL); + return Result(from_cache.release(), NULL); } Result result = delegate_->Get(keys); @@ -52,10 +52,8 @@ ExtensionSettingsStorage::Result ExtensionSettingsStorageCache::Get( } cache_.MergeDictionary(result.GetSettings()); - - DictionaryValue* settings_with_cache = result.GetSettings()->DeepCopy(); - settings_with_cache->MergeDictionary(from_cache.get()); - return Result(settings_with_cache, NULL, NULL); + result.GetSettings()->MergeDictionary(from_cache.get()); + return result; } ExtensionSettingsStorage::Result ExtensionSettingsStorageCache::Get() { @@ -82,9 +80,9 @@ ExtensionSettingsStorage::Result ExtensionSettingsStorageCache::Set( return result; } - const std::set<std::string>* changed_keys = result.GetChangedKeys(); + std::set<std::string>* changed_keys = result.GetChangedKeys(); DCHECK(changed_keys); - for (std::set<std::string>::const_iterator it = changed_keys->begin(); + for (std::set<std::string>::iterator it = changed_keys->begin(); it != changed_keys->end(); ++it) { Value* new_value = NULL; result.GetSettings()->GetWithoutPathExpansion(*it, &new_value); @@ -110,9 +108,9 @@ ExtensionSettingsStorage::Result ExtensionSettingsStorageCache::Remove( return result; } - const std::set<std::string>* changed_keys = result.GetChangedKeys(); + std::set<std::string>* changed_keys = result.GetChangedKeys(); DCHECK(changed_keys); - for (std::set<std::string>::const_iterator it = changed_keys->begin(); + for (std::set<std::string>::iterator it = changed_keys->begin(); it != changed_keys->end(); ++it) { cache_.RemoveWithoutPathExpansion(*it, NULL); } diff --git a/chrome/browser/extensions/extension_settings_storage_quota_enforcer.cc b/chrome/browser/extensions/extension_settings_storage_quota_enforcer.cc index 6186c87..91a62ce 100644 --- a/chrome/browser/extensions/extension_settings_storage_quota_enforcer.cc +++ b/chrome/browser/extensions/extension_settings_storage_quota_enforcer.cc @@ -90,8 +90,7 @@ ExtensionSettingsStorageQuotaEnforcer::ExtensionSettingsStorageQuotaEnforcer( return; } - const DictionaryValue* initial_settings = - maybe_initial_settings.GetSettings(); + DictionaryValue* initial_settings = maybe_initial_settings.GetSettings(); for (DictionaryValue::key_iterator it = initial_settings->begin_keys(); it != initial_settings->end_keys(); ++it) { Value *value; diff --git a/chrome/browser/extensions/extension_settings_storage_unittest.cc b/chrome/browser/extensions/extension_settings_storage_unittest.cc index f3080ac..a36bed9 100644 --- a/chrome/browser/extensions/extension_settings_storage_unittest.cc +++ b/chrome/browser/extensions/extension_settings_storage_unittest.cc @@ -33,86 +33,48 @@ std::string ToString(const std::set<std::string>& strings) { } // namespace -// Compares two possibly NULL values for equality, filling |error| with an -// appropriate error message if they're different. -bool ValuesEqual( - const Value* expected, const Value* actual, std::string* error) { - if (expected == actual) { - return true; - } - if (expected && !actual) { - *error = "Expected: " + GetJSON(*expected) + ", actual: NULL"; - return false; - } - if (actual && !expected) { - *error = "Expected: NULL, actual: " + GetJSON(*actual); - return false; - } - if (!expected->Equals(actual)) { - *error = - "Expected: " + GetJSON(*expected) + ", actual: " + GetJSON(*actual); - return false; - } - return true; -} - // Returns whether the result of a storage operation has the expected settings // and changed keys. testing::AssertionResult SettingsEq( - const char* _1, const char* _2, const char* _3, const char* _4, + const char* _1, const char* _2, const char* _3, DictionaryValue* expected_settings, - DictionaryValue* expected_old_settings, std::set<std::string>* expected_changed_keys, ExtensionSettingsStorage::Result actual) { - std::string error; - if (actual.HasError()) { return testing::AssertionFailure() << "Expected: " << GetJSON(*expected_settings) << ", " << ToString(*expected_changed_keys) << "\n" << ", actual has error: " << actual.GetError(); } - - if (!ValuesEqual(expected_settings, actual.GetSettings(), &error)) { - return testing::AssertionFailure() << "For settings, " << error; + if (expected_settings == NULL && actual.GetSettings() != NULL) { + return testing::AssertionFailure() << + "Expected NULL settings, actual: " << GetJSON(*actual.GetSettings()); } - - if (!expected_changed_keys && actual.GetChangedKeys()) { + if (expected_changed_keys == NULL && actual.GetChangedKeys() != NULL) { return testing::AssertionFailure() << "Expected NULL changed keys, actual: " << ToString(*actual.GetChangedKeys()); } - if (expected_changed_keys && !actual.GetChangedKeys()) { + if (expected_settings != NULL && actual.GetSettings() == NULL) { + return testing::AssertionFailure() << + "Expected: " << GetJSON(*expected_settings) << ", actual NULL"; + } + if (expected_changed_keys != NULL && actual.GetChangedKeys() == NULL) { return testing::AssertionFailure() << "Expected: " << ToString(*expected_changed_keys) << ", actual NULL"; } + if (expected_settings != actual.GetSettings() && + !expected_settings->Equals(actual.GetSettings())) { + return testing::AssertionFailure() << + "Expected: " << GetJSON(*expected_settings) << + ", actual: " << GetJSON(*actual.GetSettings()); + } if (expected_changed_keys != actual.GetChangedKeys() && *expected_changed_keys != *actual.GetChangedKeys()) { return testing::AssertionFailure() << "Expected: " << ToString(*expected_changed_keys) << ", actual: " << ToString(*actual.GetChangedKeys()); } - - const std::set<std::string>* changed_keys = actual.GetChangedKeys(); - if (changed_keys) { - for (std::set<std::string>::const_iterator it = changed_keys->begin(); - it != changed_keys->end(); ++it) { - Value *expected_old_value = NULL; - expected_old_settings->GetWithoutPathExpansion(*it, &expected_old_value); - if (!ValuesEqual(expected_old_value, actual.GetOldValue(*it), &error)) { - return testing::AssertionFailure() << "Old values not equal: " << error; - } - - Value *expected_new_value = NULL; - if (expected_settings) { - expected_settings->GetWithoutPathExpansion(*it, &expected_new_value); - } - if (!ValuesEqual(expected_new_value, actual.GetNewValue(*it), &error)) { - return testing::AssertionFailure() << "New values not equal: " << error; - } - } - } - return testing::AssertionSuccess(); } @@ -120,11 +82,10 @@ ExtensionSettingsStorageTest::ExtensionSettingsStorageTest() : key1_("foo"), key2_("bar"), key3_("baz"), - empty_dict_(new DictionaryValue()), - dict1_(new DictionaryValue()), - dict3_(new DictionaryValue()), - dict12_(new DictionaryValue()), - dict123_(new DictionaryValue()), + empty_dict_(new DictionaryValue), + dict1_(new DictionaryValue), + dict12_(new DictionaryValue), + dict123_(new DictionaryValue), ui_thread_(BrowserThread::UI, MessageLoop::current()), file_thread_(BrowserThread::FILE, MessageLoop::current()) { val1_.reset(Value::CreateStringValue(key1_ + "Value")); @@ -150,7 +111,6 @@ ExtensionSettingsStorageTest::ExtensionSettingsStorageTest() set123_.insert(list123_.begin(), list123_.end()); dict1_->Set(key1_, val1_->DeepCopy()); - dict3_->Set(key3_, val3_->DeepCopy()); dict12_->Set(key1_, val1_->DeepCopy()); dict12_->Set(key2_, val2_->DeepCopy()); dict123_->Set(key1_, val1_->DeepCopy()); @@ -171,179 +131,179 @@ void ExtensionSettingsStorageTest::TearDown() { } TEST_P(ExtensionSettingsStorageTest, GetWhenEmpty) { - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, GetWithSingleValue) { - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), empty_dict_.get(), &set1_, storage_->Set(key1_, *val1_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key2_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key3_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), &set1_, storage_->Set(key1_, *val1_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key2_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key3_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, GetWithMultipleValues) { - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), empty_dict_.get(), &set12_, storage_->Set(*dict12_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key3_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), &set12_, storage_->Set(*dict12_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key3_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, RemoveWhenEmpty) { - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Remove(key1_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Remove(key1_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, RemoveWithSingleValue) { - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), empty_dict_.get(), &set1_, storage_->Set(*dict1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, dict1_.get(), &set1_, storage_->Remove(key1_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key2_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), &set1_, storage_->Set(*dict1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set1_, storage_->Remove(key1_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key2_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, RemoveWithMultipleValues) { - EXPECT_PRED_FORMAT4(SettingsEq, - dict123_.get(), empty_dict_.get(), &set123_, storage_->Set(*dict123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, dict3_.get(), &set3_, storage_->Remove(key3_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key3_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(list1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get(list12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(list13_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get()); - - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, dict12_.get(), &set12_, storage_->Remove(list12_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key3_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list13_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + dict123_.get(), &set123_, storage_->Set(*dict123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set3_, storage_->Remove(key3_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key3_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(list1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get(list12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(list13_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get()); + + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set12_, storage_->Remove(list12_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key3_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list13_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, SetWhenOverwriting) { DictionaryValue key1_val2; key1_val2.Set(key1_, val2_->DeepCopy()); - EXPECT_PRED_FORMAT4(SettingsEq, - &key1_val2, empty_dict_.get(), &set1_, storage_->Set(key1_, *val2_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), &key1_val2, &set12_, storage_->Set(*dict12_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key3_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(list1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get(list12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), NULL, NULL, storage_->Get(list13_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + &key1_val2, &set1_, storage_->Set(key1_, *val2_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), &set12_, storage_->Set(*dict12_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key3_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(list1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get(list12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), NULL, storage_->Get(list13_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, ClearWhenEmpty) { - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Clear()); - - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Clear()); + + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, ClearWhenNotEmpty) { - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), empty_dict_.get(), &set12_, storage_->Set(*dict12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, dict12_.get(), &set12_, storage_->Clear()); - - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(empty_list_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(list123_)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), &set12_, storage_->Set(*dict12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set12_, storage_->Clear()); + + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(empty_list_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(list123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } // Dots should be allowed in key names; they shouldn't be interpreted as @@ -358,35 +318,33 @@ TEST_P(ExtensionSettingsStorageTest, DotsInKeyNames) { DictionaryValue dot_dict; dot_dict.SetWithoutPathExpansion(dot_key, dot_value.DeepCopy()); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(dot_key)); - - EXPECT_PRED_FORMAT4(SettingsEq, - &dot_dict, empty_dict_.get(), &dot_set, - storage_->Set(dot_key, dot_value)); - EXPECT_PRED_FORMAT4(SettingsEq, - &dot_dict, &dot_dict, &empty_set_, - storage_->Set(dot_key, dot_value)); - EXPECT_PRED_FORMAT4(SettingsEq, - &dot_dict, NULL, NULL, storage_->Get(dot_key)); - - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, &dot_dict, &dot_set, storage_->Remove(dot_key)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Remove(dot_key)); - EXPECT_PRED_FORMAT4(SettingsEq, - &dot_dict, empty_dict_.get(), &dot_set, storage_->Set(dot_dict)); - EXPECT_PRED_FORMAT4(SettingsEq, - &dot_dict, NULL, NULL, storage_->Get(dot_list)); - EXPECT_PRED_FORMAT4(SettingsEq, - &dot_dict, NULL, NULL, storage_->Get()); - - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, &dot_dict, &dot_set, storage_->Remove(dot_list)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get(dot_key)); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get()); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(dot_key)); + + EXPECT_PRED_FORMAT3(SettingsEq, + &dot_dict, &dot_set, storage_->Set(dot_key, dot_value)); + EXPECT_PRED_FORMAT3(SettingsEq, + &dot_dict, &empty_set_, storage_->Set(dot_key, dot_value)); + EXPECT_PRED_FORMAT3(SettingsEq, + &dot_dict, NULL, storage_->Get(dot_key)); + + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &dot_set, storage_->Remove(dot_key)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Remove(dot_key)); + EXPECT_PRED_FORMAT3(SettingsEq, + &dot_dict, &dot_set, storage_->Set(dot_dict)); + EXPECT_PRED_FORMAT3(SettingsEq, + &dot_dict, NULL, storage_->Get(dot_list)); + EXPECT_PRED_FORMAT3(SettingsEq, + &dot_dict, NULL, storage_->Get()); + + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &dot_set, storage_->Remove(dot_list)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get(dot_key)); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get()); } TEST_P(ExtensionSettingsStorageTest, DotsInKeyNamesWithDicts) { @@ -397,12 +355,12 @@ TEST_P(ExtensionSettingsStorageTest, DotsInKeyNamesWithDicts) { std::set<std::string> changed_keys; changed_keys.insert("foo"); - EXPECT_PRED_FORMAT4(SettingsEq, - &outer_dict, empty_dict_.get(), &changed_keys, storage_->Set(outer_dict)); - EXPECT_PRED_FORMAT4(SettingsEq, - &outer_dict, NULL, NULL, storage_->Get("foo")); - EXPECT_PRED_FORMAT4(SettingsEq, - empty_dict_.get(), NULL, NULL, storage_->Get("foo.bar")); + EXPECT_PRED_FORMAT3(SettingsEq, + &outer_dict, &changed_keys, storage_->Set(outer_dict)); + EXPECT_PRED_FORMAT3(SettingsEq, + &outer_dict, NULL, storage_->Get("foo")); + EXPECT_PRED_FORMAT3(SettingsEq, + empty_dict_.get(), NULL, storage_->Get("foo.bar")); } TEST_P(ExtensionSettingsStorageTest, ComplexChangedKeysScenarios) { @@ -413,82 +371,66 @@ TEST_P(ExtensionSettingsStorageTest, ComplexChangedKeysScenarios) { std::vector<std::string> complex_list; std::set<std::string> complex_set; DictionaryValue complex_dict; - DictionaryValue complex_changed_dict; - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), empty_dict_.get(), &set1_, storage_->Set(key1_, *val1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), dict1_.get(), &empty_set_, storage_->Set(key1_, *val1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), &set1_, storage_->Set(key1_, *val1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), &empty_set_, storage_->Set(key1_, *val1_)); complex_dict.Clear(); complex_dict.Set(key1_, val2_->DeepCopy()); - EXPECT_PRED_FORMAT4(SettingsEq, - &complex_dict, dict1_.get(), &set1_, storage_->Set(key1_, *val2_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, &complex_dict, &set1_, storage_->Remove(key1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Remove(key1_)); - - EXPECT_PRED_FORMAT4(SettingsEq, - dict1_.get(), empty_dict_.get(), &set1_, storage_->Set(key1_, *val1_)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, dict1_.get(), &set1_, storage_->Clear()); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Clear()); - - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), empty_dict_.get(), &set12_, storage_->Set(*dict12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict12_.get(), dict12_.get(), &empty_set_, storage_->Set(*dict12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - dict123_.get(), dict12_.get(), &set3_, storage_->Set(*dict123_)); + EXPECT_PRED_FORMAT3(SettingsEq, + &complex_dict, &set1_, storage_->Set(key1_, *val2_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set1_, storage_->Remove(key1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Remove(key1_)); + + EXPECT_PRED_FORMAT3(SettingsEq, + dict1_.get(), &set1_, storage_->Set(key1_, *val1_)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set1_, storage_->Clear()); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Clear()); + + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), &set12_, storage_->Set(*dict12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict12_.get(), &empty_set_, storage_->Set(*dict12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + dict123_.get(), &set3_, storage_->Set(*dict123_)); complex_dict.Clear(); complex_dict.Set(key1_, val2_->DeepCopy()); complex_dict.Set(key2_, val2_->DeepCopy()); complex_dict.Set("asdf", val1_->DeepCopy()); complex_dict.Set("qwerty", val3_->DeepCopy()); - complex_changed_dict.Clear(); - complex_changed_dict.Set(key1_, val1_->DeepCopy()); - complex_changed_dict.Set(key2_, val2_->DeepCopy()); complex_set.clear(); complex_set.insert(key1_); complex_set.insert("asdf"); complex_set.insert("qwerty"); - EXPECT_PRED_FORMAT4(SettingsEq, - &complex_dict, &complex_changed_dict, &complex_set, - storage_->Set(complex_dict)); - - complex_changed_dict.Clear(); - complex_changed_dict.Set(key1_, val2_->DeepCopy()); - complex_changed_dict.Set(key2_, val2_->DeepCopy()); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, &complex_changed_dict, &set12_, storage_->Remove(list12_)); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Remove(list12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + &complex_dict, &complex_set, storage_->Set(complex_dict)); + + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &set12_, storage_->Remove(list12_)); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Remove(list12_)); complex_list.clear(); complex_list.push_back(key1_); complex_list.push_back("asdf"); - complex_changed_dict.Clear(); - complex_changed_dict.Set("asdf", val1_->DeepCopy()); complex_set.clear(); complex_set.insert("asdf"); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, - &complex_changed_dict, - &complex_set, - storage_->Remove(complex_list)); - - complex_changed_dict.Clear(); - complex_changed_dict.Set(key3_, val3_->DeepCopy()); - complex_changed_dict.Set("qwerty", val3_->DeepCopy()); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &complex_set, storage_->Remove(complex_list)); + complex_set.clear(); complex_set.insert(key3_); complex_set.insert("qwerty"); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, &complex_changed_dict, &complex_set, storage_->Clear()); - EXPECT_PRED_FORMAT4(SettingsEq, - NULL, empty_dict_.get(), &empty_set_, storage_->Clear()); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &complex_set, storage_->Clear()); + EXPECT_PRED_FORMAT3(SettingsEq, + NULL, &empty_set_, storage_->Clear()); } diff --git a/chrome/browser/extensions/extension_settings_storage_unittest.h b/chrome/browser/extensions/extension_settings_storage_unittest.h index b78975e..8f306ed 100644 --- a/chrome/browser/extensions/extension_settings_storage_unittest.h +++ b/chrome/browser/extensions/extension_settings_storage_unittest.h @@ -8,15 +8,12 @@ #include "testing/gtest/include/gtest/gtest.h" -#include "base/bind.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" #include "base/scoped_temp_dir.h" #include "base/task.h" #include "chrome/browser/extensions/extension_settings_backend.h" -#include "chrome/browser/extensions/extension_settings_frontend.h" -#include "chrome/test/base/testing_profile.h" #include "content/browser/browser_thread.h" // Parameter type for the value-parameterized tests. @@ -64,7 +61,6 @@ class ExtensionSettingsStorageTest scoped_ptr<DictionaryValue> empty_dict_; scoped_ptr<DictionaryValue> dict1_; - scoped_ptr<DictionaryValue> dict3_; scoped_ptr<DictionaryValue> dict12_; scoped_ptr<DictionaryValue> dict123_; diff --git a/chrome/browser/extensions/extension_settings_sync_unittest.cc b/chrome/browser/extensions/extension_settings_sync_unittest.cc index 7073475..f9b34f9 100644 --- a/chrome/browser/extensions/extension_settings_sync_unittest.cc +++ b/chrome/browser/extensions/extension_settings_sync_unittest.cc @@ -12,12 +12,10 @@ #include "base/scoped_temp_dir.h" #include "base/task.h" #include "chrome/browser/extensions/extension_settings_backend.h" -#include "chrome/browser/extensions/extension_settings_frontend.h" #include "chrome/browser/extensions/extension_settings_storage_cache.h" #include "chrome/browser/extensions/extension_settings_sync_util.h" #include "chrome/browser/extensions/syncable_extension_settings_storage.h" #include "chrome/browser/sync/api/sync_change_processor.h" -#include "chrome/test/base/testing_profile.h" #include "content/browser/browser_thread.h" // TODO(kalman): Integration tests for sync. @@ -105,8 +103,7 @@ class MockSyncChangeProcessor : public SyncChangeProcessor { if (matching_changes.empty()) { ADD_FAILURE() << "No matching changes for " << extension_id << "/" << key << " (out of " << changes_.size() << ")"; - return ExtensionSettingSyncData( - SyncChange::ACTION_INVALID, "", "", new DictionaryValue()); + return ExtensionSettingSyncData(SyncChange::ACTION_INVALID, "", "", NULL); } if (matching_changes.size() != 1u) { ADD_FAILURE() << matching_changes.size() << " matching changes for " << @@ -119,27 +116,25 @@ class MockSyncChangeProcessor : public SyncChangeProcessor { ExtensionSettingSyncDataList changes_; }; -// To be called as a callback from ExtensionSettingsFrontend::RunWithSettings. -void AssignSettings( - ExtensionSettingsBackend** dst, ExtensionSettingsBackend* src) { - *dst = src; -} - } // namespace class ExtensionSettingsSyncTest : public testing::Test { public: ExtensionSettingsSyncTest() : ui_thread_(BrowserThread::UI, MessageLoop::current()), - file_thread_(BrowserThread::FILE, MessageLoop::current()), - frontend_(&profile_), - backend_(NULL) { - } + file_thread_(BrowserThread::FILE, MessageLoop::current()) {} virtual void SetUp() OVERRIDE { - frontend_.RunWithBackend(base::Bind(&AssignSettings, &backend_)); - MessageLoop::current()->RunAllPending(); - ASSERT_TRUE(backend_ != NULL); + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + // TODO(kalman): Use ExtensionSettingsFrontend here? It would test more + // code paths. + backend_.reset(new ExtensionSettingsBackend(temp_dir_.path())); + } + + virtual void TearDown() OVERRIDE { + // Must do this explicitly here so that it's destroyed before the + // message loops are. + backend_.reset(); } protected: @@ -151,7 +146,10 @@ class ExtensionSettingsSyncTest : public testing::Test { backend_->GetStorage(extension_id)); } - // Gets all the sync data from |backend_| as a map from extension id to its + MockSyncChangeProcessor sync_; + scoped_ptr<ExtensionSettingsBackend> backend_; + + // Gets all the sync data from backend_ as a map from extension id to its // sync data. std::map<std::string, ExtensionSettingSyncDataList> GetAllSyncData() { SyncDataList as_list = @@ -165,17 +163,13 @@ class ExtensionSettingsSyncTest : public testing::Test { return as_map; } + private: + ScopedTempDir temp_dir_; + // Need these so that the DCHECKs for running on FILE or UI threads pass. MessageLoop message_loop_; BrowserThread ui_thread_; BrowserThread file_thread_; - - MockSyncChangeProcessor sync_; - TestingProfile profile_; - ExtensionSettingsFrontend frontend_; - - // Get from frontend_->RunWithBackend, so weak reference. - ExtensionSettingsBackend* backend_; }; TEST_F(ExtensionSettingsSyncTest, NoDataDoesNotInvokeSync) { diff --git a/chrome/browser/extensions/in_memory_extension_settings_storage.cc b/chrome/browser/extensions/in_memory_extension_settings_storage.cc index ee50721..1cb58d5 100644 --- a/chrome/browser/extensions/in_memory_extension_settings_storage.cc +++ b/chrome/browser/extensions/in_memory_extension_settings_storage.cc @@ -29,11 +29,11 @@ ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Get( settings->SetWithoutPathExpansion(*it, value->DeepCopy()); } } - return Result(settings, NULL, NULL); + return Result(settings, NULL); } ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Get() { - return Result(storage_.DeepCopy(), NULL, NULL); + return Result(storage_.DeepCopy(), NULL); } ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Set( @@ -45,14 +45,11 @@ ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Set( ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Set( const DictionaryValue& settings) { - DictionaryValue* old_settings = new DictionaryValue(); std::set<std::string>* changed_keys = new std::set<std::string>(); for (DictionaryValue::key_iterator it = settings.begin_keys(); it != settings.end_keys(); ++it) { Value* old_value = NULL; - if (storage_.GetWithoutPathExpansion(*it, &old_value)) { - old_settings->SetWithoutPathExpansion(*it, old_value->DeepCopy()); - } + storage_.GetWithoutPathExpansion(*it, &old_value); Value* new_value = NULL; settings.GetWithoutPathExpansion(*it, &new_value); if (old_value == NULL || !old_value->Equals(new_value)) { @@ -60,7 +57,7 @@ ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Set( storage_.SetWithoutPathExpansion(*it, new_value->DeepCopy()); } } - return Result(settings.DeepCopy(), old_settings, changed_keys); + return Result(settings.DeepCopy(), changed_keys); } ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Remove( @@ -70,17 +67,14 @@ ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Remove( ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Remove( const std::vector<std::string>& keys) { - DictionaryValue* old_settings = new DictionaryValue(); std::set<std::string>* changed_keys = new std::set<std::string>(); for (std::vector<std::string>::const_iterator it = keys.begin(); it != keys.end(); ++it) { - Value* old_value = NULL; - if (storage_.RemoveWithoutPathExpansion(*it, &old_value)) { - old_settings->SetWithoutPathExpansion(*it, old_value); + if (storage_.RemoveWithoutPathExpansion(*it, NULL)) { changed_keys->insert(*it); } } - return Result(NULL, old_settings, changed_keys); + return Result(NULL, changed_keys); } ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Clear() { @@ -89,7 +83,6 @@ ExtensionSettingsStorage::Result InMemoryExtensionSettingsStorage::Clear() { it != storage_.end_keys(); ++it) { changed_keys->insert(*it); } - DictionaryValue* old_settings = new DictionaryValue(); - storage_.Swap(old_settings); - return Result(NULL, old_settings, changed_keys); + storage_.Clear(); + return Result(NULL, changed_keys); } diff --git a/chrome/browser/extensions/syncable_extension_settings_storage.cc b/chrome/browser/extensions/syncable_extension_settings_storage.cc index 8ec5d17..f9820f4 100644 --- a/chrome/browser/extensions/syncable_extension_settings_storage.cc +++ b/chrome/browser/extensions/syncable_extension_settings_storage.cc @@ -11,14 +11,8 @@ #include "content/browser/browser_thread.h" SyncableExtensionSettingsStorage::SyncableExtensionSettingsStorage( - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> >& - observers, - const std::string& extension_id, - ExtensionSettingsStorage* delegate) - : observers_(observers), - extension_id_(extension_id), - delegate_(delegate), - sync_processor_(NULL) { + std::string extension_id, ExtensionSettingsStorage* delegate) + : extension_id_(extension_id), delegate_(delegate), sync_processor_(NULL) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); } @@ -124,7 +118,7 @@ SyncError SyncableExtensionSettingsStorage::StartSyncing( syncable::EXTENSION_SETTINGS); } - const DictionaryValue* settings = maybe_settings.GetSettings(); + DictionaryValue* settings = maybe_settings.GetSettings(); if (sync_state.empty()) { return SendLocalSettingsToSync(*settings); } @@ -230,88 +224,43 @@ std::vector<SyncError> SyncableExtensionSettingsStorage::ProcessSyncChanges( DCHECK(!sync_changes.empty()) << "No sync changes for " << extension_id_; std::vector<SyncError> errors; - ExtensionSettingChanges::Builder changes; for (ExtensionSettingSyncDataList::const_iterator it = sync_changes.begin(); it != sync_changes.end(); ++it) { DCHECK_EQ(extension_id_, it->extension_id()); - - const std::string& key = it->key(); - const Value& value = it->value(); - - scoped_ptr<Value> current_value; - { - Result maybe_settings = Get(it->key()); - if (maybe_settings.HasError()) { - errors.push_back(SyncError( - FROM_HERE, - std::string("Error getting current sync state for ") + - extension_id_ + "/" + key + ": " + maybe_settings.GetError(), - syncable::EXTENSION_SETTINGS)); - continue; - } - const DictionaryValue* settings = maybe_settings.GetSettings(); - if (settings) { - Value* value = NULL; - if (settings->GetWithoutPathExpansion(key, &value)) { - current_value.reset(value->DeepCopy()); - } - } - } - - SyncError error; - switch (it->change_type()) { case SyncChange::ACTION_ADD: - if (!current_value.get()) { - error = OnSyncAdd(key, value.DeepCopy(), &changes); - } else { - // Already a value; hopefully a local change has beaten sync in a - // race and it's not a bug, so pretend it's an update. - LOG(WARNING) << "Got add from sync for existing setting " << - extension_id_ << "/" << key; - error = OnSyncUpdate( - key, current_value.release(), value.DeepCopy(), &changes); - } - break; - - case SyncChange::ACTION_UPDATE: - if (current_value.get()) { - error = OnSyncUpdate( - key, current_value.release(), value.DeepCopy(), &changes); - } else { - // Similarly, pretend it's an add. - LOG(WARNING) << "Got update from sync for nonexistent setting" << - extension_id_ << "/" << key; - error = OnSyncAdd(key, value.DeepCopy(), &changes); + case SyncChange::ACTION_UPDATE: { + synced_keys_.insert(it->key()); + Result result = delegate_->Set(it->key(), it->value()); + if (result.HasError()) { + errors.push_back(SyncError( + FROM_HERE, + std::string("Error pushing sync change to local settings: ") + + result.GetError(), + syncable::EXTENSION_SETTINGS)); } break; + } - case SyncChange::ACTION_DELETE: - if (current_value.get()) { - error = OnSyncDelete(key, current_value.release(), &changes); - } else { - // Similarly, ignore it. - LOG(WARNING) << "Got delete from sync for nonexistent setting " << - extension_id_ << "/" << key; + case SyncChange::ACTION_DELETE: { + synced_keys_.erase(it->key()); + Result result = delegate_->Remove(it->key()); + if (result.HasError()) { + errors.push_back(SyncError( + FROM_HERE, + std::string("Error removing sync change from local settings: ") + + result.GetError(), + syncable::EXTENSION_SETTINGS)); } break; + } default: NOTREACHED(); } - - if (error.IsSet()) { - errors.push_back(error); - } } - observers_->Notify( - &ExtensionSettingsObserver::OnSettingsChanged, - static_cast<Profile*>(NULL), - extension_id_, - changes.Build()); - return errors; } @@ -358,18 +307,11 @@ void SyncableExtensionSettingsStorage::SendDeletesToSync( SyncChangeList changes; for (std::set<std::string>::const_iterator it = keys.begin(); it != keys.end(); ++it) { - if (!synced_keys_.count(*it)) { - LOG(WARNING) << "Deleted " << *it << " but no entry in synced_keys"; - continue; - } + DCHECK(synced_keys_.count(*it)); changes.push_back( extension_settings_sync_util::CreateDelete(extension_id_, *it)); } - if (changes.empty()) { - return; - } - SyncError error = sync_processor_->ProcessSyncChanges(FROM_HERE, changes); if (error.IsSet()) { LOG(WARNING) << "Failed to send changes to sync: " << error.message(); @@ -381,58 +323,3 @@ void SyncableExtensionSettingsStorage::SendDeletesToSync( synced_keys_.erase(*it); } } - -SyncError SyncableExtensionSettingsStorage::OnSyncAdd( - const std::string& key, - Value* new_value, - ExtensionSettingChanges::Builder* changes) { - DCHECK(new_value); - synced_keys_.insert(key); - Result result = delegate_->Set(key, *new_value); - if (result.HasError()) { - return SyncError( - FROM_HERE, - std::string("Error pushing sync add to local settings: ") + - result.GetError(), - syncable::EXTENSION_SETTINGS); - } - changes->AppendChange(key, NULL, new_value); - return SyncError(); -} - -SyncError SyncableExtensionSettingsStorage::OnSyncUpdate( - const std::string& key, - Value* old_value, - Value* new_value, - ExtensionSettingChanges::Builder* changes) { - DCHECK(old_value); - DCHECK(new_value); - Result result = delegate_->Set(key, *new_value); - if (result.HasError()) { - return SyncError( - FROM_HERE, - std::string("Error pushing sync update to local settings: ") + - result.GetError(), - syncable::EXTENSION_SETTINGS); - } - changes->AppendChange(key, old_value, new_value); - return SyncError(); -} - -SyncError SyncableExtensionSettingsStorage::OnSyncDelete( - const std::string& key, - Value* old_value, - ExtensionSettingChanges::Builder* changes) { - DCHECK(old_value); - synced_keys_.erase(key); - Result result = delegate_->Remove(key); - if (result.HasError()) { - return SyncError( - FROM_HERE, - std::string("Error pushing sync remove to local settings: ") + - result.GetError(), - syncable::EXTENSION_SETTINGS); - } - changes->AppendChange(key, old_value, NULL); - return SyncError(); -} diff --git a/chrome/browser/extensions/syncable_extension_settings_storage.h b/chrome/browser/extensions/syncable_extension_settings_storage.h index 499d89c..39f4607 100644 --- a/chrome/browser/extensions/syncable_extension_settings_storage.h +++ b/chrome/browser/extensions/syncable_extension_settings_storage.h @@ -8,9 +8,7 @@ #include "base/compiler_specific.h" #include "base/memory/weak_ptr.h" -#include "base/observer_list_threadsafe.h" #include "base/values.h" -#include "chrome/browser/extensions/extension_settings_observer.h" #include "chrome/browser/extensions/extension_settings_storage.h" #include "chrome/browser/extensions/extension_setting_sync_data.h" #include "chrome/browser/sync/api/syncable_service.h" @@ -20,9 +18,7 @@ class SyncableExtensionSettingsStorage : public ExtensionSettingsStorage { public: explicit SyncableExtensionSettingsStorage( - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> >& - observers, - const std::string& extension_id, + std::string extension_id, // Ownership taken. ExtensionSettingsStorage* delegate); @@ -66,26 +62,6 @@ class SyncableExtensionSettingsStorage : public ExtensionSettingsStorage { SyncError OverwriteLocalSettingsWithSync( const DictionaryValue& sync_state, const DictionaryValue& settings); - // Called when an Add/Update/Remove comes from sync. Ownership of Value*s - // are taken. - SyncError OnSyncAdd( - const std::string& key, - Value* new_value, - ExtensionSettingChanges::Builder* changes); - SyncError OnSyncUpdate( - const std::string& key, - Value* old_value, - Value* new_value, - ExtensionSettingChanges::Builder* changes); - SyncError OnSyncDelete( - const std::string& key, - Value* old_value, - ExtensionSettingChanges::Builder* changes); - - // List of observers to settings changes. - const scoped_refptr<ObserverListThreadSafe<ExtensionSettingsObserver> > - observers_; - // Id of the extension these settings are for. std::string const extension_id_; @@ -96,8 +72,7 @@ class SyncableExtensionSettingsStorage : public ExtensionSettingsStorage { // StartSyncing and StopSyncing). SyncChangeProcessor* sync_processor_; - // Keys of the settings that are currently being synced. Only used to decide - // whether an ACTION_ADD or ACTION_UPDATE should be sent to sync on a Set(). + // Keys of the settings that are currently being synced. std::set<std::string> synced_keys_; DISALLOW_COPY_AND_ASSIGN(SyncableExtensionSettingsStorage); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index c9e46a9..30eb746 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1121,18 +1121,14 @@ 'browser/extensions/extension_rlz_module.h', 'browser/extensions/extension_save_page_api.cc', 'browser/extensions/extension_save_page_api.h', - 'browser/extensions/extension_setting_changes.cc', - 'browser/extensions/extension_setting_changes.h', 'browser/extensions/extension_setting_sync_data.cc', 'browser/extensions/extension_setting_sync_data.h', - 'browser/extensions/extension_settings_api.cc', - 'browser/extensions/extension_settings_api.h', 'browser/extensions/extension_settings_backend.cc', 'browser/extensions/extension_settings_backend.h', 'browser/extensions/extension_settings_frontend.cc', 'browser/extensions/extension_settings_frontend.h', - 'browser/extensions/extension_settings_observer.cc', - 'browser/extensions/extension_settings_observer.h', + 'browser/extensions/extension_settings_api.cc', + 'browser/extensions/extension_settings_api.h', 'browser/extensions/extension_settings_storage.cc', 'browser/extensions/extension_settings_storage.h', 'browser/extensions/extension_settings_storage_cache.cc', diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json index 69163cb..f01bc14 100644 --- a/chrome/common/extensions/api/extension_api.json +++ b/chrome/common/extensions/api/extension_api.json @@ -1005,25 +1005,7 @@ }, { "namespace": "experimental.settings", - "types": [ - { - "id": "SettingChange", - "type": "object", - "properties": { - "key": {"type": "string", "description": "The ID of the setting which changed."}, - "oldValue": { - "type": "any", - "description": "The old value of the setting, before it changed.", - "optional": true - }, - "newValue": { - "type": "any", - "description": "The new value of the setting.", - "optional": true - } - } - } - ], + "types": [], "functions": [ { "name": "get", @@ -1121,20 +1103,7 @@ ] } ], - "events": [ - { - "name": "onChanged", - "type": "function", - "description": "Fired when one or more settings change.", - "parameters": [ - { - "name": "changes", - "type": "array", - "items": {"$ref": "SettingChange"} - } - ] - } - ] + "events": [] }, { "namespace": "windows", diff --git a/chrome/common/extensions/docs/experimental.settings.html b/chrome/common/extensions/docs/experimental.settings.html index 2a1c1e7..f5371b6 100644 --- a/chrome/common/extensions/docs/experimental.settings.html +++ b/chrome/common/extensions/docs/experimental.settings.html @@ -290,19 +290,19 @@ </li> </ol> </li> - <li> - <a href="#global-events">Events</a> + <li style="display: none; "> + <a>Events</a> <ol> <li> - <a href="#event-onChanged">onChanged</a> + <a href="#event-anchor">eventName</a> </li> </ol> </li> - <li> + <li style="display: none; "> <a href="#types">Types</a> <ol> <li> - <a href="#type-SettingChange">SettingChange</a> + <a href="#id-anchor">id</a> </li> </ol> </li> @@ -1207,22 +1207,24 @@ </div> <!-- /apiGroup --> <!-- EVENTS --> - <div id="eventsTemplate" class="apiGroup"> - <a name="global-events"></a> + <div id="eventsTemplate" class="apiGroup" style="display: none; "> + <a></a> <h3>Events</h3> <!-- iterates over all events --> <div class="apiItem"> - <a name="event-onChanged"></a> - <h4>onChanged</h4> + <a></a> + <h4>event name</h4> <div class="summary"> <!-- Note: intentionally longer 80 columns --> - <span class="subdued">chrome.experimental.settings.</span><span>onChanged</span><span class="subdued">.addListener</span>(function(<span>array of SettingChange changes</span>) <span class="subdued">{...}</span><span></span>); + <span class="subdued">chrome.bookmarks</span><span>onEvent</span><span class="subdued">.addListener</span>(function(<span>Type param1, Type param2</span>) <span class="subdued">{...}</span><span>, Type opt_param1, Type opt_param2</span>); </div> <div class="description"> - <p class="todo" style="display: none; ">Undocumented.</p> - <p>Fired when one or more settings change.</p> + <p class="todo">Undocumented.</p> + <p> + A description from the json schema def of the event goes here. + </p> <!-- LISTENER PARAMETERS --> <div> @@ -1230,91 +1232,13 @@ <dl> <div> <div> - <dt> - <var>changes</var> - <em> - - <!-- TYPE --> - <div style="display:inline"> - ( - <span class="optional" style="display: none; ">optional</span> - <span class="enum" style="display: none; ">enumerated</span> - <span id="typeTemplate"> - <span style="display: none; "> - <a> Type</a> - </span> - <span> - <span> - array of <span><span> - <span> - <a href="experimental.settings.html#type-SettingChange">SettingChange</a> - </span> - <span style="display: none; "> - <span> - array of <span><span></span></span> - </span> - <span>paramType</span> - <span></span> - </span> - </span></span> - </span> - <span style="display: none; ">paramType</span> - <span style="display: none; "></span> - </span> - </span> - ) - </div> - - </em> - </dt> - <dd class="todo"> - Undocumented. - </dd> - <dd style="display: none; "> - Description of this parameter from the json schema. - </dd> - <dd style="display: none; "> - This parameter was added in version - <b><span></span></b>. - You must omit this parameter in earlier versions, - and you may omit it in any version. If you require this - parameter, the manifest key - <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a> - can ensure that your extension won't be run in an earlier browser version. - </dd> - - <!-- OBJECT PROPERTIES --> - <dd style="display: none; "> - <dl> - <div> - <div> - </div> - </div> - </dl> - </dd> - - <!-- OBJECT METHODS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- OBJECT EVENT FIELDS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- FUNCTION PARAMETERS --> - <dd style="display: none; "> - <div></div> - </dd> - - </div> + </div> </div> </dl> </div> <!-- EXTRA PARAMETERS --> - <div style="display: none; "> + <div> <h4>Extra parameters to addListener</h4> <dl> <div> @@ -1325,9 +1249,9 @@ </div> <!-- LISTENER RETURN VALUE --> - <h4 style="display: none; ">Listener returns</h4> + <h4>Listener returns</h4> <dl> - <div style="display: none; "> + <div> <div> </div> </div> @@ -1339,285 +1263,17 @@ </div> <!-- /apiGroup --> <!-- TYPES --> - <div class="apiGroup"> + <div class="apiGroup" style="display: none; "> <a name="types"></a> <h3 id="types">Types</h3> <!-- iterates over all types --> <div class="apiItem"> - <a name="type-SettingChange"></a> - <h4>SettingChange</h4> - - <div> - <dt> - <var style="display: none; ">paramName</var> - <em> - - <!-- TYPE --> - <div style="display:inline"> - ( - <span class="optional" style="display: none; ">optional</span> - <span class="enum" style="display: none; ">enumerated</span> - <span id="typeTemplate"> - <span style="display: none; "> - <a> Type</a> - </span> - <span> - <span style="display: none; "> - array of <span><span></span></span> - </span> - <span>object</span> - <span style="display: none; "></span> - </span> - </span> - ) - </div> - - </em> - </dt> - <dd class="todo"> - Undocumented. - </dd> - <dd style="display: none; "> - Description of this parameter from the json schema. - </dd> - <dd style="display: none; "> - This parameter was added in version - <b><span></span></b>. - You must omit this parameter in earlier versions, - and you may omit it in any version. If you require this - parameter, the manifest key - <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a> - can ensure that your extension won't be run in an earlier browser version. - </dd> - - <!-- OBJECT PROPERTIES --> - <dd> - <dl> - <div> - <div> - <dt> - <var>key</var> - <em> - - <!-- TYPE --> - <div style="display:inline"> - ( - <span class="optional" style="display: none; ">optional</span> - <span class="enum" style="display: none; ">enumerated</span> - <span id="typeTemplate"> - <span style="display: none; "> - <a> Type</a> - </span> - <span> - <span style="display: none; "> - array of <span><span></span></span> - </span> - <span>string</span> - <span style="display: none; "></span> - </span> - </span> - ) - </div> - - </em> - </dt> - <dd class="todo" style="display: none; "> - Undocumented. - </dd> - <dd>The ID of the setting which changed.</dd> - <dd style="display: none; "> - This parameter was added in version - <b><span></span></b>. - You must omit this parameter in earlier versions, - and you may omit it in any version. If you require this - parameter, the manifest key - <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a> - can ensure that your extension won't be run in an earlier browser version. - </dd> - - <!-- OBJECT PROPERTIES --> - <dd style="display: none; "> - <dl> - <div> - <div> - </div> - </div> - </dl> - </dd> - - <!-- OBJECT METHODS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- OBJECT EVENT FIELDS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- FUNCTION PARAMETERS --> - <dd style="display: none; "> - <div></div> - </dd> - - </div> - </div><div> - <div> - <dt> - <var>oldValue</var> - <em> - - <!-- TYPE --> - <div style="display:inline"> - ( - <span class="optional">optional</span> - <span class="enum" style="display: none; ">enumerated</span> - <span id="typeTemplate"> - <span style="display: none; "> - <a> Type</a> - </span> - <span> - <span style="display: none; "> - array of <span><span></span></span> - </span> - <span>any</span> - <span style="display: none; "></span> - </span> - </span> - ) - </div> - - </em> - </dt> - <dd class="todo" style="display: none; "> - Undocumented. - </dd> - <dd>The old value of the setting, before it changed.</dd> - <dd style="display: none; "> - This parameter was added in version - <b><span></span></b>. - You must omit this parameter in earlier versions, - and you may omit it in any version. If you require this - parameter, the manifest key - <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a> - can ensure that your extension won't be run in an earlier browser version. - </dd> - - <!-- OBJECT PROPERTIES --> - <dd style="display: none; "> - <dl> - <div> - <div> - </div> - </div> - </dl> - </dd> - - <!-- OBJECT METHODS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- OBJECT EVENT FIELDS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- FUNCTION PARAMETERS --> - <dd style="display: none; "> - <div></div> - </dd> - - </div> - </div><div> - <div> - <dt> - <var>newValue</var> - <em> - - <!-- TYPE --> - <div style="display:inline"> - ( - <span class="optional">optional</span> - <span class="enum" style="display: none; ">enumerated</span> - <span id="typeTemplate"> - <span style="display: none; "> - <a> Type</a> - </span> - <span> - <span style="display: none; "> - array of <span><span></span></span> - </span> - <span>any</span> - <span style="display: none; "></span> - </span> - </span> - ) - </div> - - </em> - </dt> - <dd class="todo" style="display: none; "> - Undocumented. - </dd> - <dd>The new value of the setting.</dd> - <dd style="display: none; "> - This parameter was added in version - <b><span></span></b>. - You must omit this parameter in earlier versions, - and you may omit it in any version. If you require this - parameter, the manifest key - <a href="manifest.html#minimum_chrome_version">minimum_chrome_version</a> - can ensure that your extension won't be run in an earlier browser version. - </dd> + <a></a> + <h4>type name</h4> - <!-- OBJECT PROPERTIES --> - <dd style="display: none; "> - <dl> <div> - <div> - </div> - </div> - </dl> - </dd> - - <!-- OBJECT METHODS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- OBJECT EVENT FIELDS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- FUNCTION PARAMETERS --> - <dd style="display: none; "> - <div></div> - </dd> - - </div> </div> - </dl> - </dd> - - <!-- OBJECT METHODS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- OBJECT EVENT FIELDS --> - <dd style="display: none; "> - <div></div> - </dd> - - <!-- FUNCTION PARAMETERS --> - <dd style="display: none; "> - <div></div> - </dd> - - </div> </div> <!-- /apiItem --> diff --git a/chrome/test/data/extensions/api_test/settings/split_incognito/background.html b/chrome/test/data/extensions/api_test/settings/split_incognito/background.html index bc2ef60..1a8fb7b 100644 --- a/chrome/test/data/extensions/api_test/settings/split_incognito/background.html +++ b/chrome/test/data/extensions/api_test/settings/split_incognito/background.html @@ -1,85 +1,59 @@ <script> + var settings = chrome.experimental.settings; -var assertEq = chrome.test.assertEq; -var inIncognitoContext = chrome.extension.inIncognitoContext; -// Returns a function that asserts a result is expected then runs a callback. -function assertResultEq(expected, callback) { +// A different "waiting" message depending on whether this background page runs +// in incognito mode or not. This is so that the C++ test can differentiate +// between messages. + +// Returns a function that asserts a result is expected. +function assertResultEq(expected) { return function(actual) { chrome.test.assertEq(expected, actual); - callback(); }; } -// All notifications received. -var notifications = []; -settings.onChanged.addListener(function(changes) { - changes.forEach(function(change) { - notifications.push(change); - }); -}); - // The test from C++ runs "actions", where each action is defined here. // This allows the test to be tightly controlled between incognito and // non-incognito modes. -// Each function accepts a callback which should be run when the settings -// operation fully completes. var testActions = { - noop: function(callback) { - settings.get("", callback); - }, - assertEmpty: function(callback) { - settings.get(null, assertResultEq({}, callback)); + noop: function() { + settings.get("", function() {}); }, - assertFoo: function(callback) { - settings.get(null, assertResultEq({foo: "bar"}, callback)); + assertEmpty: function() { + settings.get(null, assertResultEq({})); }, - setFoo: function(callback) { - settings.set({foo: "bar"}, assertResultEq({foo: "bar"}, callback)); + assertFoobar: function() { + settings.get(null, assertResultEq({foo: "bar"})); }, - removeFoo: function(callback) { - settings.remove("foo", assertResultEq(undefined, callback)); + setFoobar: function() { + settings.set({foo: "bar"}, assertResultEq({foo: "bar"})); }, - clear: function(callback) { - settings.clear(assertResultEq(undefined, callback)); + removeFoo: function() { + settings.remove("foo", assertResultEq(undefined)); }, - assertNoNotifications: function(callback) { - assertEq([], notifications); - callback(); - }, - clearNotifications: function(callback) { - notifications = []; - callback(); - }, - assertAddFooNotification: function(callback) { - assertEq(1, notifications.length); - assertEq("foo", notifications[0].key); - assertEq(undefined, notifications[0].oldValue); - assertEq("bar", notifications[0].newValue); - callback(); - }, - assertDeleteFooNotification: function(callback) { - assertEq(1, notifications.length); - assertEq("foo", notifications[0].key); - assertEq("bar", notifications[0].oldValue); - assertEq(undefined, notifications[0].newValue); - callback(); + clear: function() { + settings.clear(assertResultEq(undefined)); } }; // The only test we run. Runs "actions" (as defined above) until told // to stop (when the message has isFinalAction set to true). function testEverything() { - function next() { - var waiting = inIncognitoContext ? "waiting_incognito" : "waiting"; - chrome.test.sendMessage(waiting, function(messageJson) { - var message = JSON.parse(messageJson); - var action = testActions[message.action]; - action(message.isFinalAction ? chrome.test.succeed : next); - }); + var waiting = chrome.extension.inIncognitoContext ? + "waiting_incognito" : "waiting"; + function runTestAction(messageJson) { + var message = JSON.parse(messageJson); + testActions[message.action](); + if (message.isFinalAction) { + chrome.test.succeed(); + return; + } + chrome.test.sendMessage(waiting, runTestAction); } - next(); + chrome.test.sendMessage(waiting, runTestAction); } chrome.test.runTests([testEverything]); + </script> |