diff options
8 files changed, 79 insertions, 26 deletions
diff --git a/chrome/browser/automation/automation_extension_function.cc b/chrome/browser/automation/automation_extension_function.cc index 776866e..57690fd 100644 --- a/chrome/browser/automation/automation_extension_function.cc +++ b/chrome/browser/automation/automation_extension_function.cc @@ -46,7 +46,7 @@ void AutomationExtensionFunction::Run() { std::string message; JSONWriter::Write(&message_to_host, false, &message); - dispatcher_->render_view_host_->delegate()->ProcessExternalHostMessage( + dispatcher()->render_view_host_->delegate()->ProcessExternalHostMessage( message, keys::kAutomationOrigin, keys::kAutomationRequestTarget); } diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc index bced32f..0b538bc 100644 --- a/chrome/browser/extensions/extension_bookmarks_module.cc +++ b/chrome/browser/extensions/extension_bookmarks_module.cc @@ -12,6 +12,7 @@ #include "chrome/browser/extensions/extension_bookmarks_module_constants.h" #include "chrome/browser/extensions/extension_message_service.h" #include "chrome/browser/profile.h" +#include "chrome/common/notification_service.h" #include "chrome/common/pref_names.h" #include "chrome/common/pref_service.h" @@ -66,11 +67,28 @@ class ExtensionBookmarks { void BookmarksFunction::Run() { // TODO(erikkay) temporary hack until adding an event listener can notify the // browser. + BookmarkModel* model = profile()->GetBookmarkModel(); + if (!model->IsLoaded()) { + // Bookmarks are not ready yet. We'll wait. + registrar_.Add(this, NotificationType::BOOKMARK_MODEL_LOADED, + NotificationService::AllSources()); + AddRef(); // balanced in Observe() + return; + } + ExtensionBookmarkEventRouter* event_router = ExtensionBookmarkEventRouter::GetSingleton(); - BookmarkModel* model = profile()->GetBookmarkModel(); event_router->Observe(model); - SyncExtensionFunction::Run(); + SendResponse(RunImpl()); +} + +void BookmarksFunction::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK(type == NotificationType::BOOKMARK_MODEL_LOADED); + DCHECK(profile()->GetBookmarkModel()->IsLoaded()); + Run(); + Release(); // balanced in Run() } // static diff --git a/chrome/browser/extensions/extension_bookmarks_module.h b/chrome/browser/extensions/extension_bookmarks_module.h index a2b6a084..5a22f58 100644 --- a/chrome/browser/extensions/extension_bookmarks_module.h +++ b/chrome/browser/extensions/extension_bookmarks_module.h @@ -12,6 +12,7 @@ #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/extensions/extension_function.h" #include "chrome/browser/extensions/extension_tabs_module.h" +#include "chrome/common/notification_registrar.h" // Observes BookmarkModel and then routes the notifications as events to // the extension system. @@ -68,8 +69,16 @@ class ExtensionBookmarkEventRouter : public BookmarkModelObserver { DISALLOW_COPY_AND_ASSIGN(ExtensionBookmarkEventRouter); }; -class BookmarksFunction : public SyncExtensionFunction { +class BookmarksFunction : public AsyncExtensionFunction, + public NotificationObserver { virtual void Run(); + virtual bool RunImpl() = 0; + + private: + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + NotificationRegistrar registrar_; }; class GetBookmarksFunction : public BookmarksFunction { diff --git a/chrome/browser/extensions/extension_function.cc b/chrome/browser/extensions/extension_function.cc index a846d96..26adefb 100644 --- a/chrome/browser/extensions/extension_function.cc +++ b/chrome/browser/extensions/extension_function.cc @@ -33,17 +33,21 @@ const std::string AsyncExtensionFunction::GetResult() { } void AsyncExtensionFunction::SendResponse(bool success) { + if (!dispatcher()) + return; if (bad_message_) { - dispatcher_->HandleBadMessage(this); + dispatcher()->HandleBadMessage(this); } else { - dispatcher_->SendResponse(this, success); + dispatcher()->SendResponse(this, success); } } std::string AsyncExtensionFunction::extension_id() { - return dispatcher_->extension_id(); + DCHECK(dispatcher()); + return dispatcher()->extension_id(); } Profile* AsyncExtensionFunction::profile() { - return dispatcher_->profile(); + DCHECK(dispatcher()); + return dispatcher()->profile(); } diff --git a/chrome/browser/extensions/extension_function.h b/chrome/browser/extensions/extension_function.h index 5d0ddb0..7798a19 100644 --- a/chrome/browser/extensions/extension_function.h +++ b/chrome/browser/extensions/extension_function.h @@ -9,6 +9,8 @@ #include "base/values.h" #include "base/scoped_ptr.h" +#include "base/ref_counted.h" +#include "chrome/browser/extensions/extension_function_dispatcher.h" class ExtensionFunctionDispatcher; class Profile; @@ -25,7 +27,7 @@ class Profile; // // TODO(aa): This will have to become reference counted when we introduce // APIs that live beyond a single stack frame. -class ExtensionFunction { +class ExtensionFunction : public base::RefCounted<ExtensionFunction> { public: ExtensionFunction() : request_id_(-1), has_callback_(false) {} virtual ~ExtensionFunction() {} @@ -43,8 +45,11 @@ class ExtensionFunction { // Retrieves any error string from the function. virtual const std::string GetError() = 0; - void set_dispatcher(ExtensionFunctionDispatcher* dispatcher) { - dispatcher_ = dispatcher; + void set_dispatcher_peer(ExtensionFunctionDispatcher::Peer* peer) { + peer_ = peer; + } + ExtensionFunctionDispatcher* dispatcher() { + return peer_->dispatcher_; } void set_request_id(int request_id) { request_id_ = request_id; } @@ -60,8 +65,8 @@ class ExtensionFunction { virtual void Run() = 0; protected: - // The dispatcher that will service this extension function call. - ExtensionFunctionDispatcher* dispatcher_; + // The peer to the dispatcher that will service this extension function call. + scoped_refptr<ExtensionFunctionDispatcher::Peer> peer_; // Id of this request, used to map the response back to the caller. int request_id_; @@ -76,6 +81,8 @@ class ExtensionFunction { // Base class for an extension function that runs asynchronously *relative to // the browser's UI thread*. +// Note that once Run() returns, dispatcher() can be NULL, so be sure to +// NULL-check. // TODO(aa) Remove this extra level of inheritance once the browser stops // parsing JSON (and instead uses custom serialization of Value objects). class AsyncExtensionFunction : public ExtensionFunction { @@ -91,8 +98,9 @@ class AsyncExtensionFunction : public ExtensionFunction { protected: void SendResponse(bool success); + // Note: After Run() returns, dispatcher() can be NULL. Since these getters + // rely on dispatcher(), make sure it is valid before using them. std::string extension_id(); - Profile* profile(); // The arguments to the API. Only non-null if argument were specified. diff --git a/chrome/browser/extensions/extension_function_dispatcher.cc b/chrome/browser/extensions/extension_function_dispatcher.cc index 93a34db..6e560e16 100644 --- a/chrome/browser/extensions/extension_function_dispatcher.cc +++ b/chrome/browser/extensions/extension_function_dispatcher.cc @@ -172,7 +172,8 @@ ExtensionFunctionDispatcher::ExtensionFunctionDispatcher( const std::string& extension_id) : render_view_host_(render_view_host), delegate_(delegate), - extension_id_(extension_id) { + extension_id_(extension_id), + ALLOW_THIS_IN_INITIALIZER_LIST(peer_(new Peer(this))) { RenderProcessHost* process = render_view_host_->process(); ExtensionMessageService* message_service = ExtensionMessageService::GetInstance(profile()->GetRequestContext()); @@ -181,6 +182,10 @@ ExtensionFunctionDispatcher::ExtensionFunctionDispatcher( message_service->RegisterExtension(extension_id, process->pid()); } +ExtensionFunctionDispatcher::~ExtensionFunctionDispatcher() { + peer_->dispatcher_ = NULL; +} + Browser* ExtensionFunctionDispatcher::GetBrowser() { DCHECK(delegate_); @@ -192,11 +197,9 @@ void ExtensionFunctionDispatcher::HandleRequest(const std::string& name, const std::string& args, int request_id, bool has_callback) { - // TODO(aa): This will get a bit more complicated when we support functions - // that live longer than the stack frame. - scoped_ptr<ExtensionFunction> function( + scoped_refptr<ExtensionFunction> function( FactoryRegistry::instance()->NewFunction(name)); - function->set_dispatcher(this); + function->set_dispatcher_peer(peer_); function->SetArgs(args); function->set_request_id(request_id); function->set_has_callback(has_callback); diff --git a/chrome/browser/extensions/extension_function_dispatcher.h b/chrome/browser/extensions/extension_function_dispatcher.h index a9af552..8375ae4 100644 --- a/chrome/browser/extensions/extension_function_dispatcher.h +++ b/chrome/browser/extensions/extension_function_dispatcher.h @@ -9,6 +9,7 @@ #include <string> #include <vector> +#include "base/ref_counted.h" #include "base/values.h" class Browser; @@ -30,6 +31,13 @@ class ExtensionFunctionDispatcher { virtual Browser* GetBrowser() = 0; }; + // The peer object allows us to notify ExtensionFunctions when we are + // destroyed. + struct Peer : public base::RefCounted<Peer> { + Peer(ExtensionFunctionDispatcher* dispatcher) : dispatcher_(dispatcher) {} + ExtensionFunctionDispatcher* dispatcher_; + }; + // Gets a list of all known extension function names. static void GetAllFunctionNames(std::vector<std::string>* names); @@ -44,6 +52,7 @@ class ExtensionFunctionDispatcher { ExtensionFunctionDispatcher(RenderViewHost* render_view_host, Delegate* delegate, const std::string& extension_id); + ~ExtensionFunctionDispatcher(); // Handle a request to execute an extension function. void HandleRequest(const std::string& name, const std::string& args, @@ -73,6 +82,8 @@ class ExtensionFunctionDispatcher { std::string extension_id_; + scoped_refptr<Peer> peer_; + // AutomationExtensionFunction requires access to the RenderViewHost // associated with us. We make it a friend rather than exposing the // RenderViewHost as a public method as we wouldn't want everyone to diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc index ea1d3be..761e874 100644 --- a/chrome/browser/extensions/extension_tabs_module.cc +++ b/chrome/browser/extensions/extension_tabs_module.cc @@ -163,7 +163,7 @@ bool GetWindowFunction::RunImpl() { } bool GetCurrentWindowFunction::RunImpl() { - Browser* browser = dispatcher_->GetBrowser(); + Browser* browser = dispatcher()->GetBrowser(); if (!browser) { error_ = keys::kNoCurrentWindowError; return false; @@ -231,7 +231,7 @@ bool CreateWindowFunction::RunImpl() { // NOTE(rafaelw): It's ok if dispatcher_->GetBrowser() returns NULL here. // GetBrowserWindowBounds will default to saved "default" values for the app. WindowSizer::GetBrowserWindowBounds(std::wstring(), empty_bounds, - dispatcher_->GetBrowser(), &bounds, + dispatcher()->GetBrowser(), &bounds, &maximized); // Any part of the bounds can optionally be set by the caller. @@ -263,7 +263,7 @@ bool CreateWindowFunction::RunImpl() { } } - Browser* new_window = Browser::Create(dispatcher_->profile()); + Browser* new_window = Browser::Create(dispatcher()->profile()); new_window->AddTabWithURL(*(url.get()), GURL(), PageTransition::LINK, true, -1, false, NULL); @@ -351,7 +351,7 @@ bool GetSelectedTabFunction::RunImpl() { EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&window_id)); browser = GetBrowserInProfileWithId(profile(), window_id, &error_); } else { - browser = dispatcher_->GetBrowser(); + browser = dispatcher()->GetBrowser(); if (!browser) error_ = keys::kNoCurrentWindowError; } @@ -361,7 +361,7 @@ bool GetSelectedTabFunction::RunImpl() { TabStripModel* tab_strip = browser->tabstrip_model(); TabContents* contents = tab_strip->GetSelectedTabContents(); if (!contents) { - error_ = keys::kNoSelectedTabError; + error_ = keys::kNoSelectedTabError; return false; } result_.reset(ExtensionTabUtil::CreateTabValue(contents, tab_strip, @@ -377,7 +377,7 @@ bool GetAllTabsInWindowFunction::RunImpl() { EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&window_id)); browser = GetBrowserInProfileWithId(profile(), window_id, &error_); } else { - browser = dispatcher_->GetBrowser(); + browser = dispatcher()->GetBrowser(); if (!browser) error_ = keys::kNoCurrentWindowError; } @@ -401,7 +401,7 @@ bool CreateTabFunction::RunImpl() { keys::kWindowIdKey, &window_id)); browser = GetBrowserInProfileWithId(profile(), window_id, &error_); } else { - browser = dispatcher_->GetBrowser(); + browser = dispatcher()->GetBrowser(); if (!browser) error_ = keys::kNoCurrentWindowError; } |