diff options
| -rw-r--r-- | chrome/browser/extensions/extension_bookmarks_module.cc | 9 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_bookmarks_module.h | 2 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_function.h | 2 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/build_commit_command.cc | 15 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/build_commit_command.h | 2 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/process_commit_response_command.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/process_commit_response_command.h | 9 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/syncer.cc | 30 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/syncer.h | 5 | ||||
| -rw-r--r-- | chrome/browser/sync/engine/syncer_session.h | 12 | ||||
| -rw-r--r-- | chrome/browser/sync/protocol/sync.proto | 21 | ||||
| -rw-r--r-- | chrome/browser/sync/util/extensions_activity_monitor.cc | 93 | ||||
| -rw-r--r-- | chrome/browser/sync/util/extensions_activity_monitor.h | 74 | ||||
| -rw-r--r-- | chrome/browser/sync/util/extensions_activity_monitor_unittest.cc | 234 | ||||
| -rwxr-xr-x | chrome/chrome.gyp | 11 | ||||
| -rw-r--r-- | chrome/common/notification_type.h | 10 |
16 files changed, 521 insertions, 18 deletions
diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc index 73cb032..ee5257c 100644 --- a/chrome/browser/extensions/extension_bookmarks_module.cc +++ b/chrome/browser/extensions/extension_bookmarks_module.cc @@ -114,7 +114,14 @@ void BookmarksFunction::Run() { ExtensionBookmarkEventRouter* event_router = ExtensionBookmarkEventRouter::GetSingleton(); event_router->Observe(model); - SendResponse(RunImpl()); + bool success = RunImpl(); + if (success) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_BOOKMARKS_API_INVOKED, + Source<const Extension>(GetExtension()), + Details<const BookmarksFunction>(this)); + } + SendResponse(success); } bool BookmarksFunction::GetBookmarkIdAsInt64( diff --git a/chrome/browser/extensions/extension_bookmarks_module.h b/chrome/browser/extensions/extension_bookmarks_module.h index 6d86bc8..acaf91d 100644 --- a/chrome/browser/extensions/extension_bookmarks_module.h +++ b/chrome/browser/extensions/extension_bookmarks_module.h @@ -69,7 +69,7 @@ class BookmarksFunction : public AsyncExtensionFunction, protected: // Helper to get the bookmark id as int64 from the given string id. - // Sets error_ to an errro string if the given id string can't be parsed + // Sets error_ to an error string if the given id string can't be parsed // as an int64. In case of error, doesn't change id and returns false. bool GetBookmarkIdAsInt64(const std::string& id_string, int64* id); diff --git a/chrome/browser/extensions/extension_function.h b/chrome/browser/extensions/extension_function.h index 6090f8a..e8a77dd 100644 --- a/chrome/browser/extensions/extension_function.h +++ b/chrome/browser/extensions/extension_function.h @@ -37,7 +37,7 @@ class ExtensionFunction : public base::RefCounted<ExtensionFunction> { // Specifies the name of the function. void set_name(const std::string& name) { name_ = name; } - const std::string name() { return name_; } + const std::string name() const { return name_; } // Specifies the raw arguments to the function, as a JSON value. virtual void SetArgs(const Value* args) = 0; diff --git a/chrome/browser/sync/engine/build_commit_command.cc b/chrome/browser/sync/engine/build_commit_command.cc index 46200c6..5058bba 100644 --- a/chrome/browser/sync/engine/build_commit_command.cc +++ b/chrome/browser/sync/engine/build_commit_command.cc @@ -30,6 +30,20 @@ namespace browser_sync { BuildCommitCommand::BuildCommitCommand() {} BuildCommitCommand::~BuildCommitCommand() {} +void BuildCommitCommand::AddExtensionsActivityToMessage( + SyncerSession* session, CommitMessage* message) { + const ExtensionsActivityMonitor::Records& records = + session->extensions_activity(); + for (ExtensionsActivityMonitor::Records::const_iterator it = records.begin(); + it != records.end(); ++it) { + sync_pb::CommitMessage_ChromiumExtensionsActivity* activity_message = + message->add_extensions_activity(); + activity_message->set_extension_id(it->second.extension_id); + activity_message->set_bookmark_writes_since_last_commit( + it->second.bookmark_write_count); + } +} + void BuildCommitCommand::ExecuteImpl(SyncerSession* session) { ClientToServerMessage message; message.set_share(ToUTF8(session->account_name()).get_string()); @@ -38,6 +52,7 @@ void BuildCommitCommand::ExecuteImpl(SyncerSession* session) { CommitMessage* commit_message = message.mutable_commit(); commit_message->set_cache_guid( session->write_transaction()->directory()->cache_guid()); + AddExtensionsActivityToMessage(session, commit_message); const vector<Id>& commit_ids = session->commit_ids(); for (size_t i = 0; i < commit_ids.size(); i++) { diff --git a/chrome/browser/sync/engine/build_commit_command.h b/chrome/browser/sync/engine/build_commit_command.h index 445024f..1c29590 100644 --- a/chrome/browser/sync/engine/build_commit_command.h +++ b/chrome/browser/sync/engine/build_commit_command.h @@ -19,6 +19,8 @@ class BuildCommitCommand : public SyncerCommand { virtual void ExecuteImpl(SyncerSession *session); private: + void AddExtensionsActivityToMessage(SyncerSession* session, + CommitMessage* message); DISALLOW_COPY_AND_ASSIGN(BuildCommitCommand); }; diff --git a/chrome/browser/sync/engine/process_commit_response_command.cc b/chrome/browser/sync/engine/process_commit_response_command.cc index 3deec6d..72fd0e6 100644 --- a/chrome/browser/sync/engine/process_commit_response_command.cc +++ b/chrome/browser/sync/engine/process_commit_response_command.cc @@ -52,11 +52,19 @@ void ResetErrorCounters(SyncerStatus status) { status.zero_consecutive_errors(); } -ProcessCommitResponseCommand::ProcessCommitResponseCommand() {} +ProcessCommitResponseCommand::ProcessCommitResponseCommand( + ExtensionsActivityMonitor* monitor) : extensions_monitor_(monitor) {} ProcessCommitResponseCommand::~ProcessCommitResponseCommand() {} void ProcessCommitResponseCommand::ModelChangingExecuteImpl( SyncerSession* session) { + ProcessCommitResponse(session); + if (!session->HadSuccessfulCommits()) + extensions_monitor_->PutRecords(session->extensions_activity()); +} + +void ProcessCommitResponseCommand::ProcessCommitResponse( + SyncerSession* session) { // TODO(sync): This function returns if it sees problems. We probably want // to flag the need for an update or similar. ScopedDirLookup dir(session->dirman(), session->account_name()); diff --git a/chrome/browser/sync/engine/process_commit_response_command.h b/chrome/browser/sync/engine/process_commit_response_command.h index 4e7c94b..9950ba4 100644 --- a/chrome/browser/sync/engine/process_commit_response_command.h +++ b/chrome/browser/sync/engine/process_commit_response_command.h @@ -23,7 +23,7 @@ namespace browser_sync { class ProcessCommitResponseCommand : public ModelChangingSyncerCommand { public: - ProcessCommitResponseCommand(); + explicit ProcessCommitResponseCommand(ExtensionsActivityMonitor* monitor); virtual ~ProcessCommitResponseCommand(); virtual void ModelChangingExecuteImpl(SyncerSession* session); @@ -36,6 +36,9 @@ class ProcessCommitResponseCommand : public ModelChangingSyncerCommand { std::set<syncable::Id>* deleted_folders, SyncerSession* const session); + // Actually does the work of execute. + void ProcessCommitResponse(SyncerSession* session); + void ProcessSuccessfulCommitResponse(syncable::WriteTransaction* trans, const CommitResponse_EntryResponse& server_entry, const syncable::Id& pre_commit_id, syncable::MutableEntry* local_entry, @@ -47,6 +50,10 @@ class ProcessCommitResponseCommand : public ModelChangingSyncerCommand { const CommitResponse_EntryResponse& server_entry, syncable::MutableEntry* local_entry); + // We may need to update this with records from a commit attempt if the + // attempt failed. + ExtensionsActivityMonitor* extensions_monitor_; + DISALLOW_COPY_AND_ASSIGN(ProcessCommitResponseCommand); }; diff --git a/chrome/browser/sync/engine/syncer.cc b/chrome/browser/sync/engine/syncer.cc index de8b1b2..cc73232 100644 --- a/chrome/browser/sync/engine/syncer.cc +++ b/chrome/browser/sync/engine/syncer.cc @@ -5,6 +5,8 @@ #include "chrome/browser/sync/engine/syncer.h" #include "base/format_macros.h" +#include "base/message_loop.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/sync/engine/apply_updates_command.h" #include "chrome/browser/sync/engine/build_and_process_conflict_sets_command.h" #include "chrome/browser/sync/engine/build_commit_command.h" @@ -66,12 +68,24 @@ Syncer::Syncer( syncer_event_channel_.reset(new SyncerEventChannel(shutdown)); shutdown_channel_.reset(new ShutdownChannel(this)); + extensions_monitor_ = new ExtensionsActivityMonitor( + ChromeThread::GetMessageLoop(ChromeThread::UI)); + ScopedDirLookup dir(dirman_, account_name_); // The directory must be good here. CHECK(dir.good()); } -Syncer::~Syncer() {} +Syncer::~Syncer() { + MessageLoop* ui_loop = ChromeThread::GetMessageLoop(ChromeThread::UI); + if (ui_loop) { + ui_loop->DeleteSoon(FROM_HERE, extensions_monitor_); + } else { + NOTREACHED(); + delete extensions_monitor_; + } + extensions_monitor_ = NULL; +} void Syncer::RequestNudge(int milliseconds) { SyncerEvent event; @@ -92,6 +106,17 @@ bool Syncer::SyncShare(SyncProcessState* process_state) { SyncerSession session(&cycle_state, process_state); session.set_source(TestAndSetUpdatesSource()); session.set_notifications_enabled(notifications_enabled()); + // This isn't perfect, as we can end up bundling extensions activity + // intended for the next session into the current one. We could do a + // test-and-reset as with the source, but note that also falls short if + // the commit request fails (due to lost connection, for example), as we will + // fall all the way back to the syncer thread main loop in that case, and + // wind up creating a new session when a connection is established, losing + // the records set here on the original attempt. This should provide us + // with the right data "most of the time", and we're only using this for + // analysis purposes, so Law of Large Numbers FTW. + extensions_monitor_->GetAndClearRecords( + session.mutable_extensions_activity()); SyncShare(&session, SYNCER_BEGIN, SYNCER_END); return session.HasMoreToSync(); } @@ -205,7 +230,8 @@ void Syncer::SyncShare(SyncerSession* session, } case PROCESS_COMMIT_RESPONSE: { LOG(INFO) << "Processing the commit response"; - ProcessCommitResponseCommand process_response_command; + ProcessCommitResponseCommand process_response_command( + extensions_monitor_); process_response_command.Execute(session); next_step = BUILD_AND_PROCESS_CONFLICT_SETS; break; diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h index 2d6ba6a..067193b 100644 --- a/chrome/browser/sync/engine/syncer.h +++ b/chrome/browser/sync/engine/syncer.h @@ -19,6 +19,7 @@ #include "chrome/browser/sync/syncable/directory_event.h" #include "chrome/browser/sync/util/event_sys-inl.h" #include "chrome/browser/sync/util/event_sys.h" +#include "chrome/browser/sync/util/extensions_activity_monitor.h" #include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST namespace syncable { @@ -180,6 +181,10 @@ class Syncer { // by the SyncerThread that created us. ModelSafeWorker* model_safe_worker_; + // We use this to stuff extensions activity into CommitMessages so the server + // can correlate commit traffic with extension-related bookmark mutations. + ExtensionsActivityMonitor* extensions_monitor_; + // The source of the last nudge. sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE updates_source_; diff --git a/chrome/browser/sync/engine/syncer_session.h b/chrome/browser/sync/engine/syncer_session.h index 83643d7..5e4710b 100644 --- a/chrome/browser/sync/engine/syncer_session.h +++ b/chrome/browser/sync/engine/syncer_session.h @@ -22,6 +22,7 @@ #include "chrome/browser/sync/engine/syncer_types.h" #include "chrome/browser/sync/engine/syncproto.h" #include "chrome/browser/sync/util/event_sys.h" +#include "chrome/browser/sync/util/extensions_activity_monitor.h" #include "chrome/browser/sync/util/sync_types.h" #include "testing/gtest/include/gtest/gtest_prod.h" // For FRIEND_TEST @@ -307,6 +308,14 @@ class SyncerSession { source_ = source; } + const ExtensionsActivityMonitor::Records& extensions_activity() const { + return extensions_activity_; + } + + ExtensionsActivityMonitor::Records* mutable_extensions_activity() { + return &extensions_activity_; + } + bool notifications_enabled() const { return notifications_enabled_; } @@ -355,6 +364,9 @@ class SyncerSession { // The source for initiating this syncer session. sync_pb::GetUpdatesCallerInfo::GET_UPDATES_SOURCE source_; + // Information about extensions activity since the last successful commit. + ExtensionsActivityMonitor::Records extensions_activity_; + // True if notifications are enabled when this session was created. bool notifications_enabled_; diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto index c9b50cc..ad7f61f 100644 --- a/chrome/browser/sync/protocol/sync.proto +++ b/chrome/browser/sync/protocol/sync.proto @@ -172,6 +172,22 @@ message CommitMessage { // A GUID that identifies the committing sync client. This value will be // returned as originator_cache_guid for any new items. optional string cache_guid = 2; + + // This message contains diagnostic information used to correlate + // commit-related traffic with extensions-related mutations to the + // data models in chromium. It plays no functional role in + // processing this CommitMessage. + message ChromiumExtensionsActivity { + // The human-readable ID identifying the extension responsible + // for the traffic reported in this ChromiumExtensionsActivity. + optional string extension_id = 1; + + // How many times the extension successfully invoked a write + // operation through the bookmarks API since the last CommitMessage. + optional uint32 bookmark_writes_since_last_commit = 2; + } + + repeated ChromiumExtensionsActivity extensions_activity = 3; }; message GetUpdatesCallerInfo { @@ -203,7 +219,7 @@ message AuthenticateMessage { message ClientToServerMessage { required string share = 1; - optional int32 protocol_version = 2 [default = 21]; + optional int32 protocol_version = 2 [default = 22]; enum CONTENTS { COMMIT = 1; GET_UPDATES = 2; @@ -243,8 +259,7 @@ message CommitResponse { optional string parent_id_string = 4; // This value is the same as the position_in_parent value returned within - // the SyncEntity message in GetUpdatesResponse. It is returned if the - // item was assigned a new position. + // the SyncEntity message in GetUpdatesResponse. optional int64 position_in_parent = 5; // The item's current version. diff --git a/chrome/browser/sync/util/extensions_activity_monitor.cc b/chrome/browser/sync/util/extensions_activity_monitor.cc new file mode 100644 index 0000000..e65dfb7 --- /dev/null +++ b/chrome/browser/sync/util/extensions_activity_monitor.cc @@ -0,0 +1,93 @@ +// Copyright (c) 2009 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/sync/util/extensions_activity_monitor.h" + +#include "base/task.h" +#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/extensions/extension_bookmarks_module.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/notification_service.h" + +namespace browser_sync { + +namespace { +// A helper task to register an ExtensionsActivityMonitor as an observer of +// events on the UI thread (even though the monitor may live on another thread). +// This liberates ExtensionsActivityMonitor from having to be ref counted. +class RegistrationTask : public Task { + public: + RegistrationTask(ExtensionsActivityMonitor* monitor, + MessageLoop* ui_loop, + NotificationRegistrar* registrar) + : monitor_(monitor), ui_loop_(ui_loop), registrar_(registrar) {} + virtual ~RegistrationTask() {} + + virtual void Run() { + DCHECK_EQ(MessageLoop::current(), + ChromeThread::GetMessageLoop(ChromeThread::UI)); + + // It would be nice if we could specify a Source for each specific function + // we wanted to observe, but the actual function objects are allocated on + // the fly so there is no reliable object to point to (same problem if we + // wanted to use the string name). Thus, we use all sources and filter in + // Observe. + registrar_->Add(monitor_, NotificationType::EXTENSION_BOOKMARKS_API_INVOKED, + NotificationService::AllSources()); + } + + private: + ExtensionsActivityMonitor* monitor_; + MessageLoop* const ui_loop_; + NotificationRegistrar* registrar_; + DISALLOW_COPY_AND_ASSIGN(RegistrationTask); +}; +} // namespace + +ExtensionsActivityMonitor::ExtensionsActivityMonitor(MessageLoop* ui_loop) + : ui_loop_(ui_loop) { + ui_loop_->PostTask(FROM_HERE, new RegistrationTask(this, ui_loop, + ®istrar_)); +} + +ExtensionsActivityMonitor::~ExtensionsActivityMonitor() { + DCHECK_EQ(MessageLoop::current(), ui_loop_); + // The registrar calls RemoveAll in its dtor (which would happen in a moment) + // but explicitly call this so it is clear why we need to be on the ui_loop_. + registrar_.RemoveAll(); +} + +void ExtensionsActivityMonitor::GetAndClearRecords(Records* buffer) { + AutoLock lock(records_lock_); + buffer->clear(); + buffer->swap(records_); +} + +void ExtensionsActivityMonitor::PutRecords(const Records& records) { + AutoLock lock(records_lock_); + for (Records::const_iterator i = records.begin(); i != records.end(); ++i) { + records_[i->first].extension_id = i->second.extension_id; + records_[i->first].bookmark_write_count += i->second.bookmark_write_count; + } +} + +void ExtensionsActivityMonitor::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + AutoLock lock(records_lock_); + DCHECK_EQ(MessageLoop::current(), ui_loop_); + const Extension* extension = Source<const Extension>(source).ptr(); + const BookmarksFunction* f = Details<const BookmarksFunction>(details).ptr(); + if (f->name() == "bookmarks.update" || + f->name() == "bookmarks.move" || + f->name() == "bookmarks.create" || + f->name() == "bookmarks.removeTree" || + f->name() == "bookmarks.remove") { + Record& record = records_[extension->id()]; + record.extension_id = extension->id(); + record.bookmark_write_count++; + } +} + +} // namespace browser_sync diff --git a/chrome/browser/sync/util/extensions_activity_monitor.h b/chrome/browser/sync/util/extensions_activity_monitor.h new file mode 100644 index 0000000..5e4e4ae --- /dev/null +++ b/chrome/browser/sync/util/extensions_activity_monitor.h @@ -0,0 +1,74 @@ +// Copyright (c) 2009 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_SYNC_UTIL_EXTENSIONS_ACTIVITY_MONITOR_H_ +#define CHROME_BROWSER_SYNC_UTIL_EXTENSIONS_ACTIVITY_MONITOR_H_ + +#include "base/lock.h" +#include "base/message_loop.h" +#include "base/ref_counted.h" +#include "chrome/common/notification_observer.h" +#include "chrome/common/notification_registrar.h" + +namespace browser_sync { + +// A class to monitor usage of extensions APIs to send to sync servers, with +// the ability to purge data once sync servers have acknowledged it (successful +// commit response). +// +// This can be used from any thread (it is a 'monitor' in the synchronization +// sense as well), HOWEVER +// +// *** IT MUST BE DELETED FROM THE UI LOOP *** +// +// Consider using MessageLoop::DeleteSoon. (Yes, this means if you allocate +// an ExtensionsActivityMonitor on a thread other than UI, you must 'new' it). +class ExtensionsActivityMonitor : public NotificationObserver { + public: + // A data record of activity performed by extension |extension_id|. + struct Record { + Record() : bookmark_write_count(0U) {} + + // The human-readable ID identifying the extension responsible + // for the activity reported in this Record. + std::string extension_id; + + // How many times the extension successfully invoked a write + // operation through the bookmarks API since the last CommitMessage. + uint32 bookmark_write_count; + }; + + typedef std::map<std::string, Record> Records; + + // Creates an ExtensionsActivityMonitor to monitor extensions activities on + // |ui_loop|. + explicit ExtensionsActivityMonitor(MessageLoop* ui_loop); + ~ExtensionsActivityMonitor(); + + // Fills |buffer| with snapshot of current records in constant time by + // swapping. This is done mutually exclusively w.r.t methods of this class. + void GetAndClearRecords(Records* buffer); + + // Add |records| piece-wise (by extension id) to the set of active records. + // This is done mutually exclusively w.r.t the methods of this class. + void PutRecords(const Records& records); + + // NotificationObserver implementation. Called on |ui_loop_|. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + private: + Records records_; + mutable Lock records_lock_; + + // Kept for convenience. + MessageLoop* const ui_loop_; + + // Used only from UI loop. + NotificationRegistrar registrar_; +}; + +} // namespace browser_sync + +#endif // CHROME_BROWSER_SYNC_UTIL_EXTENSIONS_ACTIVITY_MONITOR_H_
\ No newline at end of file diff --git a/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc b/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc new file mode 100644 index 0000000..ae917df --- /dev/null +++ b/chrome/browser/sync/util/extensions_activity_monitor_unittest.cc @@ -0,0 +1,234 @@ +// Copyright (c) 2009 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 entry. + +#include "chrome/browser/sync/util/extensions_activity_monitor.h" + +#include "base/file_path.h" +#include "base/string_util.h" +#include "base/waitable_event.h" +#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/extensions/extension_bookmarks_module.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" +#include "chrome/common/notification_service.h" +#include "testing/gtest/include/gtest/gtest.h" + +using browser_sync::ExtensionsActivityMonitor; +namespace keys = extension_manifest_keys; + +namespace { + +static const char* kTestExtensionPath1 = "c:\\testextension1"; +static const char* kTestExtensionPath2 = "c:\\testextension2"; +static const char* kTestExtensionVersion = "1.0.0.0"; +static const char* kTestExtensionName = "foo extension"; + +template <class FunctionType> +class BookmarkAPIEventTask : public Task { + public: + BookmarkAPIEventTask(FunctionType* t, Extension* e, size_t repeats, + base::WaitableEvent* done) : + function_(t), extension_(e), repeats_(repeats), done_(done) {} + virtual void Run() { + for (size_t i = 0; i < repeats_; i++) { + NotificationService::current()->Notify( + NotificationType::EXTENSION_BOOKMARKS_API_INVOKED, + Source<Extension>(extension_.get()), + Details<const BookmarksFunction>(function_.get())); + } + done_->Signal(); + } + private: + scoped_ptr<Extension> extension_; + scoped_refptr<FunctionType> function_; + size_t repeats_; + base::WaitableEvent* done_; + + DISALLOW_COPY_AND_ASSIGN(BookmarkAPIEventTask); +}; + +class BookmarkAPIEventGenerator { + public: + BookmarkAPIEventGenerator(MessageLoop* ui_loop) : ui_loop_(ui_loop) {} + virtual ~BookmarkAPIEventGenerator() {} + template <class T> + void NewEvent(const std::string& extension_path, + T* bookmarks_function, size_t repeats) { + FilePath path(UTF8ToWide(extension_path)); + Extension* extension = new Extension(path); + std::string error; + DictionaryValue input; + input.SetString(keys::kVersion, kTestExtensionVersion); + input.SetString(keys::kName, kTestExtensionName); + extension->InitFromValue(input, false, &error); + bookmarks_function->set_name(T::function_name()); + base::WaitableEvent done_event(false, false); + ui_loop_->PostTask(FROM_HERE, new BookmarkAPIEventTask<T>( + bookmarks_function, extension, repeats, &done_event)); + done_event.Wait(); + } + + private: + MessageLoop* ui_loop_; + DISALLOW_COPY_AND_ASSIGN(BookmarkAPIEventGenerator); +}; +} // namespace + +class DoUIThreadSetupTask : public Task { + public: + DoUIThreadSetupTask(NotificationService** service, + base::WaitableEvent* done) + : service_(service), signal_when_done_(done) {} + virtual ~DoUIThreadSetupTask() {} + virtual void Run() { + *service_ = new NotificationService(); + signal_when_done_->Signal(); + } + private: + NotificationService** service_; + base::WaitableEvent* signal_when_done_; + DISALLOW_COPY_AND_ASSIGN(DoUIThreadSetupTask); +}; + +class ExtensionsActivityMonitorTest : public testing::Test { + public: + ExtensionsActivityMonitorTest() : service_(NULL), + ui_thread_(ChromeThread::UI) { } + virtual ~ExtensionsActivityMonitorTest() {} + + virtual void SetUp() { + ui_thread_.Start(); + base::WaitableEvent service_created(false, false); + ui_thread_.message_loop()->PostTask(FROM_HERE, + new DoUIThreadSetupTask(&service_, &service_created)); + service_created.Wait(); + } + + virtual void TearDown() { + ui_thread_.message_loop()->DeleteSoon(FROM_HERE, service_); + ui_thread_.Stop(); + } + + MessageLoop* ui_loop() { return ui_thread_.message_loop(); } + + static std::string GetExtensionIdForPath(const std::string& extension_path) { + std::string error; + FilePath path(UTF8ToWide(extension_path)); + Extension e(path); + DictionaryValue input; + input.SetString(keys::kVersion, kTestExtensionVersion); + input.SetString(keys::kName, kTestExtensionName); + e.InitFromValue(input, false, &error); + EXPECT_EQ("", error); + return e.id(); + } + private: + NotificationService* service_; + ChromeThread ui_thread_; +}; + +TEST_F(ExtensionsActivityMonitorTest, Basic) { + ExtensionsActivityMonitor* monitor = new ExtensionsActivityMonitor(ui_loop()); + BookmarkAPIEventGenerator generator(ui_loop()); + + generator.NewEvent<RemoveBookmarkFunction>(kTestExtensionPath1, + new RemoveBookmarkFunction(), 1); + generator.NewEvent<MoveBookmarkFunction>(kTestExtensionPath1, + new MoveBookmarkFunction(), 1); + generator.NewEvent<UpdateBookmarkFunction>(kTestExtensionPath1, + new UpdateBookmarkFunction(), 2); + generator.NewEvent<CreateBookmarkFunction>(kTestExtensionPath1, + new CreateBookmarkFunction(), 3); + generator.NewEvent<SearchBookmarksFunction>(kTestExtensionPath1, + new SearchBookmarksFunction(), 5); + const int writes_by_extension1 = 1 + 1 + 2 + 3; + + generator.NewEvent<RemoveTreeBookmarkFunction>(kTestExtensionPath2, + new RemoveTreeBookmarkFunction(), 8); + generator.NewEvent<GetBookmarkTreeFunction>(kTestExtensionPath2, + new GetBookmarkTreeFunction(), 13); + generator.NewEvent<GetBookmarkChildrenFunction>(kTestExtensionPath2, + new GetBookmarkChildrenFunction(), 21); + generator.NewEvent<GetBookmarksFunction>(kTestExtensionPath2, + new GetBookmarksFunction(), 33); + const int writes_by_extension2 = 8; + + ExtensionsActivityMonitor::Records results; + monitor->GetAndClearRecords(&results); + + std::string id1 = GetExtensionIdForPath(kTestExtensionPath1); + std::string id2 = GetExtensionIdForPath(kTestExtensionPath2); + + EXPECT_EQ(2, results.size()); + EXPECT_TRUE(results.end() != results.find(id1)); + EXPECT_TRUE(results.end() != results.find(id2)); + EXPECT_EQ(writes_by_extension1, results[id1].bookmark_write_count); + EXPECT_EQ(writes_by_extension2, results[id2].bookmark_write_count); + + ui_loop()->DeleteSoon(FROM_HERE, monitor); +} + +TEST_F(ExtensionsActivityMonitorTest, Put) { + ExtensionsActivityMonitor* monitor = new ExtensionsActivityMonitor(ui_loop()); + BookmarkAPIEventGenerator generator(ui_loop()); + std::string id1 = GetExtensionIdForPath(kTestExtensionPath1); + std::string id2 = GetExtensionIdForPath(kTestExtensionPath2); + + generator.NewEvent<CreateBookmarkFunction>(kTestExtensionPath1, + new CreateBookmarkFunction(), 5); + generator.NewEvent<MoveBookmarkFunction>(kTestExtensionPath2, + new MoveBookmarkFunction(), 8); + + ExtensionsActivityMonitor::Records results; + monitor->GetAndClearRecords(&results); + + EXPECT_EQ(2, results.size()); + EXPECT_EQ(5, results[id1].bookmark_write_count); + EXPECT_EQ(8, results[id2].bookmark_write_count); + + generator.NewEvent<GetBookmarksFunction>(kTestExtensionPath2, + new GetBookmarksFunction(), 3); + generator.NewEvent<UpdateBookmarkFunction>(kTestExtensionPath2, + new UpdateBookmarkFunction(), 2); + + // Simulate a commit failure, which augments the active record set with the + // refugee records. + monitor->PutRecords(results); + ExtensionsActivityMonitor::Records new_records; + monitor->GetAndClearRecords(&new_records); + + EXPECT_EQ(2, results.size()); + EXPECT_EQ(id1, new_records[id1].extension_id); + EXPECT_EQ(id2, new_records[id2].extension_id); + EXPECT_EQ(5, new_records[id1].bookmark_write_count); + EXPECT_EQ(8 + 2, new_records[id2].bookmark_write_count); + ui_loop()->DeleteSoon(FROM_HERE, monitor); +} + +TEST_F(ExtensionsActivityMonitorTest, MultiGet) { + ExtensionsActivityMonitor* monitor = new ExtensionsActivityMonitor(ui_loop()); + BookmarkAPIEventGenerator generator(ui_loop()); + std::string id1 = GetExtensionIdForPath(kTestExtensionPath1); + + generator.NewEvent<CreateBookmarkFunction>(kTestExtensionPath1, + new CreateBookmarkFunction(), 5); + + ExtensionsActivityMonitor::Records results; + monitor->GetAndClearRecords(&results); + + EXPECT_EQ(1, results.size()); + EXPECT_EQ(5, results[id1].bookmark_write_count); + + monitor->GetAndClearRecords(&results); + EXPECT_TRUE(results.empty()); + + generator.NewEvent<CreateBookmarkFunction>(kTestExtensionPath1, + new CreateBookmarkFunction(), 3); + monitor->GetAndClearRecords(&results); + + EXPECT_EQ(1, results.size()); + EXPECT_EQ(3, results[id1].bookmark_write_count); + + ui_loop()->DeleteSoon(FROM_HERE, monitor); +}
\ No newline at end of file diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index 71486f6..8cbe8fa 100755 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -6737,6 +6737,7 @@ 'browser/sync/util/character_set_converters_unittest.cc', 'browser/sync/util/crypto_helpers_unittest.cc', 'browser/sync/util/event_sys_unittest.cc', + 'browser/sync/util/extensions_activity_monitor_unittest.cc', 'browser/sync/util/highres_timer_unittest.cc', 'browser/sync/util/path_helpers_unittest.cc', 'browser/sync/util/query_helpers_unittest.cc', @@ -6760,11 +6761,12 @@ '_USE_32BIT_TIME_T', ], 'dependencies': [ + 'common', + 'debugger', + '../skia/skia.gyp:skia', '../testing/gtest.gyp:gtest', '../third_party/libjingle/libjingle.gyp:libjingle', - 'notifier', - 'sync', - 'sync_proto', + 'syncapi', 'test_support_unit', ], 'conditions': [ @@ -6902,6 +6904,8 @@ 'browser/sync/util/dbgq.h', 'browser/sync/util/event_sys-inl.h', 'browser/sync/util/event_sys.h', + 'browser/sync/util/extensions_activity_monitor.cc', + 'browser/sync/util/extensions_activity_monitor.h', 'browser/sync/util/fast_dump.h', 'browser/sync/util/highres_timer.h', 'browser/sync/util/highres_timer_linux.cc', @@ -6934,6 +6938,7 @@ '_USE_32BIT_TIME_T', ], 'dependencies': [ + '../skia/skia.gyp:skia', '../third_party/libjingle/libjingle.gyp:libjingle', 'sync_proto', ], diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index fddbeb6..739a007 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -612,11 +612,6 @@ class NotificationType { // are all source and no details. SESSION_END, - // Personalization --------------------------------------------------------- - - PERSONALIZATION, - PERSONALIZATION_CREATED, - // Privacy blacklists ------------------------------------------------------ // Sent when a privacy blacklist path provider changes the list of its @@ -714,6 +709,11 @@ class NotificationType { EXTENSION_TEST_PASSED, EXTENSION_TEST_FAILED, + // Sent when an bookmarks extensions API function was successfully invoked. + // The source is the id of the extension that invoked the function, and the + // details are a pointer to the const BookmarksFunction in question. + EXTENSION_BOOKMARKS_API_INVOKED, + // Privacy Blacklist ------------------------------------------------------- // Sent by the resource dispatcher host when a resource is blocked. |