Commit 14fef327 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions Functions] Migrate BrowserAction API to UIThreadExtensionFunction

Migrate the browserAction API to use UIThreadExtensionFunction instead of
ChromeAsyncExtensionFunction.

Bug: 634140
Change-Id: Ic6af918ee5a3c968ad5afb6ec0d3700d450be0b9
Reviewed-on: https://chromium-review.googlesource.com/1067484
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560696}
parent 50518fc7
...@@ -543,13 +543,11 @@ ExtensionActionGetBadgeBackgroundColorFunction::RunExtensionAction() { ...@@ -543,13 +543,11 @@ ExtensionActionGetBadgeBackgroundColorFunction::RunExtensionAction() {
return RespondNow(OneArgument(std::move(list))); return RespondNow(OneArgument(std::move(list)));
} }
BrowserActionOpenPopupFunction::BrowserActionOpenPopupFunction() BrowserActionOpenPopupFunction::BrowserActionOpenPopupFunction() = default;
: response_sent_(false) {
}
bool BrowserActionOpenPopupFunction::RunAsync() { ExtensionFunction::ResponseAction BrowserActionOpenPopupFunction::Run() {
// We only allow the popup in the active window. // We only allow the popup in the active window.
Profile* profile = GetProfile(); Profile* profile = Profile::FromBrowserContext(browser_context());
Browser* browser = chrome::FindLastActiveWithProfile(profile); Browser* browser = chrome::FindLastActiveWithProfile(profile);
// It's possible that the last active browser actually corresponds to the // It's possible that the last active browser actually corresponds to the
// associated incognito profile, and this won't be returned by // associated incognito profile, and this won't be returned by
...@@ -566,13 +564,11 @@ bool BrowserActionOpenPopupFunction::RunAsync() { ...@@ -566,13 +564,11 @@ bool BrowserActionOpenPopupFunction::RunAsync() {
// Otherwise, try to open a popup in the active browser. // Otherwise, try to open a popup in the active browser.
// TODO(justinlin): Remove toolbar check when http://crbug.com/308645 is // TODO(justinlin): Remove toolbar check when http://crbug.com/308645 is
// fixed. // fixed.
if (!browser || if (!browser || !browser->window()->IsActive() ||
!browser->window()->IsActive() ||
!browser->window()->IsToolbarVisible() || !browser->window()->IsToolbarVisible() ||
!ExtensionActionAPI::Get(GetProfile())->ShowExtensionActionPopup( !ExtensionActionAPI::Get(profile)->ShowExtensionActionPopup(
extension_.get(), browser, false)) { extension_.get(), browser, false)) {
error_ = kOpenPopupError; return RespondNow(Error(kOpenPopupError));
return false;
} }
// Even if this is for an incognito window, we want to use the normal profile. // Even if this is for an incognito window, we want to use the normal profile.
...@@ -590,17 +586,15 @@ bool BrowserActionOpenPopupFunction::RunAsync() { ...@@ -590,17 +586,15 @@ bool BrowserActionOpenPopupFunction::RunAsync() {
FROM_HERE, FROM_HERE,
base::BindOnce(&BrowserActionOpenPopupFunction::OpenPopupTimedOut, this), base::BindOnce(&BrowserActionOpenPopupFunction::OpenPopupTimedOut, this),
base::TimeDelta::FromSeconds(10)); base::TimeDelta::FromSeconds(10));
return true; return RespondLater();
} }
void BrowserActionOpenPopupFunction::OpenPopupTimedOut() { void BrowserActionOpenPopupFunction::OpenPopupTimedOut() {
if (response_sent_) if (did_respond())
return; return;
DVLOG(1) << "chrome.browserAction.openPopup did not show a popup."; DVLOG(1) << "chrome.browserAction.openPopup did not show a popup.";
error_ = kOpenPopupError; Respond(Error(kOpenPopupError));
SendResponse(false);
response_sent_ = true;
} }
void BrowserActionOpenPopupFunction::Observe( void BrowserActionOpenPopupFunction::Observe(
...@@ -608,7 +602,7 @@ void BrowserActionOpenPopupFunction::Observe( ...@@ -608,7 +602,7 @@ void BrowserActionOpenPopupFunction::Observe(
const content::NotificationSource& source, const content::NotificationSource& source,
const content::NotificationDetails& details) { const content::NotificationDetails& details) {
DCHECK_EQ(NOTIFICATION_EXTENSION_HOST_DID_STOP_FIRST_LOAD, type); DCHECK_EQ(NOTIFICATION_EXTENSION_HOST_DID_STOP_FIRST_LOAD, type);
if (response_sent_) if (did_respond())
return; return;
ExtensionHost* host = content::Details<ExtensionHost>(details).ptr(); ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
...@@ -616,8 +610,7 @@ void BrowserActionOpenPopupFunction::Observe( ...@@ -616,8 +610,7 @@ void BrowserActionOpenPopupFunction::Observe(
host->extension()->id() != extension_->id()) host->extension()->id() != extension_->id())
return; return;
SendResponse(true); Respond(NoArguments());
response_sent_ = true;
registrar_.RemoveAll(); registrar_.RemoveAll();
} }
......
...@@ -9,12 +9,12 @@ ...@@ -9,12 +9,12 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "chrome/browser/extensions/chrome_extension_function.h"
#include "chrome/browser/extensions/extension_action.h" #include "chrome/browser/extensions/extension_action.h"
#include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_registrar.h"
#include "extensions/browser/browser_context_keyed_api_factory.h" #include "extensions/browser/browser_context_keyed_api_factory.h"
#include "extensions/browser/extension_event_histogram_value.h" #include "extensions/browser/extension_event_histogram_value.h"
#include "extensions/browser/extension_function.h"
#include "third_party/skia/include/core/SkColor.h" #include "third_party/skia/include/core/SkColor.h"
namespace base { namespace base {
...@@ -26,6 +26,8 @@ class BrowserContext; ...@@ -26,6 +26,8 @@ class BrowserContext;
class WebContents; class WebContents;
} }
class Browser;
namespace extensions { namespace extensions {
class ExtensionPrefs; class ExtensionPrefs;
...@@ -358,7 +360,7 @@ class BrowserActionDisableFunction : public ExtensionActionHideFunction { ...@@ -358,7 +360,7 @@ class BrowserActionDisableFunction : public ExtensionActionHideFunction {
~BrowserActionDisableFunction() override {} ~BrowserActionDisableFunction() override {}
}; };
class BrowserActionOpenPopupFunction : public ChromeAsyncExtensionFunction, class BrowserActionOpenPopupFunction : public UIThreadExtensionFunction,
public content::NotificationObserver { public content::NotificationObserver {
public: public:
DECLARE_EXTENSION_FUNCTION("browserAction.openPopup", DECLARE_EXTENSION_FUNCTION("browserAction.openPopup",
...@@ -369,7 +371,7 @@ class BrowserActionOpenPopupFunction : public ChromeAsyncExtensionFunction, ...@@ -369,7 +371,7 @@ class BrowserActionOpenPopupFunction : public ChromeAsyncExtensionFunction,
~BrowserActionOpenPopupFunction() override {} ~BrowserActionOpenPopupFunction() override {}
// ExtensionFunction: // ExtensionFunction:
bool RunAsync() override; ResponseAction Run() override;
void Observe(int type, void Observe(int type,
const content::NotificationSource& source, const content::NotificationSource& source,
...@@ -377,7 +379,6 @@ class BrowserActionOpenPopupFunction : public ChromeAsyncExtensionFunction, ...@@ -377,7 +379,6 @@ class BrowserActionOpenPopupFunction : public ChromeAsyncExtensionFunction,
void OpenPopupTimedOut(); void OpenPopupTimedOut();
content::NotificationRegistrar registrar_; content::NotificationRegistrar registrar_;
bool response_sent_;
DISALLOW_COPY_AND_ASSIGN(BrowserActionOpenPopupFunction); DISALLOW_COPY_AND_ASSIGN(BrowserActionOpenPopupFunction);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment