Commit aeffd18e authored by rdevlin.cronin's avatar rdevlin.cronin Committed by Commit bot

Remove NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED

We're trying to kill off notifications, and this one's no
different.
Also, move the logic for notifying the location bar of
page actions being updated to ExtensionActionAPI.

BUG=406590

Review URL: https://codereview.chromium.org/496403003

Cr-Commit-Position: refs/heads/master@{#291920}
parent f830881a
......@@ -10,6 +10,7 @@
#include "base/metrics/histogram.h"
#include "base/stl_util.h"
#include "chrome/browser/extensions/active_tab_permission_granter.h"
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
#include "chrome/browser/extensions/extension_action.h"
#include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/extensions/extension_util.h"
......@@ -218,8 +219,10 @@ void ActiveScriptController::RequestScriptInjection(
// If this was the first entry, notify the location bar that there's a new
// icon.
if (list.size() == 1u)
LocationBarController::NotifyChange(web_contents());
if (list.size() == 1u) {
ExtensionActionAPI::Get(web_contents()->GetBrowserContext())->
NotifyPageActionsChanged(web_contents());
}
}
void ActiveScriptController::RunPendingForExtension(
......@@ -261,7 +264,8 @@ void ActiveScriptController::RunPendingForExtension(
}
// Inform the location bar that the action is now gone.
LocationBarController::NotifyChange(web_contents());
ExtensionActionAPI::Get(web_contents()->GetBrowserContext())->
NotifyPageActionsChanged(web_contents());
}
void ActiveScriptController::OnRequestScriptInjectionPermission(
......
......@@ -19,6 +19,9 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sessions/session_tab_helper.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/extensions/api/extension_action/action_info.h"
#include "chrome/common/render_messages.h"
......@@ -231,6 +234,26 @@ scoped_ptr<base::DictionaryValue> DefaultsToValue(ExtensionAction* action) {
} // namespace
//
// ExtensionActionAPI::Observer
//
void ExtensionActionAPI::Observer::OnExtensionActionUpdated(
ExtensionAction* extension_action,
content::WebContents* web_contents,
content::BrowserContext* browser_context) {
}
void ExtensionActionAPI::Observer::OnPageActionsUpdated(
content::WebContents* web_contents) {
}
void ExtensionActionAPI::Observer::OnExtensionActionAPIShuttingDown() {
}
ExtensionActionAPI::Observer::~Observer() {
}
//
// ExtensionActionAPI
//
......@@ -371,6 +394,9 @@ void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action,
Observer,
observers_,
OnExtensionActionUpdated(extension_action, web_contents, context));
if (extension_action->action_type() == ActionInfo::TYPE_PAGE)
NotifyPageActionsChanged(web_contents);
}
void ExtensionActionAPI::ClearAllValuesForTab(
......@@ -441,6 +467,20 @@ void ExtensionActionAPI::ExtensionActionExecuted(
}
}
void ExtensionActionAPI::NotifyPageActionsChanged(
content::WebContents* web_contents) {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
if (!browser)
return;
LocationBar* location_bar =
browser->window() ? browser->window()->GetLocationBar() : NULL;
if (!location_bar)
return;
location_bar->UpdatePageActions();
FOR_EACH_OBSERVER(Observer, observers_, OnPageActionsUpdated(web_contents));
}
void ExtensionActionAPI::Shutdown() {
FOR_EACH_OBSERVER(Observer, observers_, OnExtensionActionAPIShuttingDown());
}
......
......@@ -44,14 +44,18 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI {
virtual void OnExtensionActionUpdated(
ExtensionAction* extension_action,
content::WebContents* web_contents,
content::BrowserContext* browser_context) = 0;
content::BrowserContext* browser_context);
// Called when the page actions have been refreshed do to a possible change
// in count or visibility.
virtual void OnPageActionsUpdated(content::WebContents* web_contents);
// Called when the ExtensionActionAPI is shutting down, giving observers a
// chance to unregister themselves if there is not a definitive lifecycle.
virtual void OnExtensionActionAPIShuttingDown() {}
virtual void OnExtensionActionAPIShuttingDown();
protected:
virtual ~Observer() {}
virtual ~Observer();
};
explicit ExtensionActionAPI(content::BrowserContext* context);
......@@ -83,14 +87,19 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI {
Browser* browser,
bool grant_active_tab_permissions);
// Notifies that there has been a change in the given |extension_action|.
void NotifyChange(ExtensionAction* extension_action,
content::WebContents* web_contents,
content::BrowserContext* browser_context);
// Clears the values for all ExtensionActions for the tab associated with the
// given |web_contents|.
// given |web_contents| (and signals that page actions changed).
void ClearAllValuesForTab(content::WebContents* web_contents);
// Notifies that the current set of page actions for |web_contents| has
// changed, and signals the browser to update.
void NotifyPageActionsChanged(content::WebContents* web_contents);
private:
friend class BrowserContextKeyedAPIFactory<ExtensionActionAPI>;
......
......@@ -21,11 +21,12 @@ using extensions::Extension;
namespace {
bool HasExtensionPageActionVisibilityReachedTarget(
LocationBarTesting* location_bar,
int target_visible_page_action_count) {
VLOG(1) << "Number of visible page actions: "
<< location_bar->PageActionVisibleCount();
// A callback that returns true if the condition has been met and takes no
// arguments.
typedef base::Callback<bool(void)> ConditionCallback;
bool HasPageActionVisibilityReachedTarget(
LocationBarTesting* location_bar, int target_visible_page_action_count) {
return location_bar->PageActionVisibleCount() ==
target_visible_page_action_count;
}
......@@ -42,7 +43,13 @@ bool HaveAllExtensionRenderViewHostsFinishedLoading(
return true;
}
class NotificationSet : public content::NotificationObserver {
} // namespace
////////////////////////////////////////////////////////////////////////////////
// ExtensionTestNotificationObserver::NotificationSet
class ExtensionTestNotificationObserver::NotificationSet
: public content::NotificationObserver {
public:
void Add(int type, const content::NotificationSource& source);
void Add(int type);
......@@ -63,52 +70,25 @@ class NotificationSet : public content::NotificationObserver {
base::CallbackList<void()> callback_list_;
};
void NotificationSet::Add(
void ExtensionTestNotificationObserver::NotificationSet::Add(
int type,
const content::NotificationSource& source) {
notification_registrar_.Add(this, type, source);
}
void NotificationSet::Add(int type) {
void ExtensionTestNotificationObserver::NotificationSet::Add(int type) {
Add(type, content::NotificationService::AllSources());
}
void NotificationSet::Observe(
void ExtensionTestNotificationObserver::NotificationSet::Observe(
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
callback_list_.Notify();
}
void MaybeQuit(content::MessageLoopRunner* runner,
const base::Callback<bool(void)>& condition) {
if (condition.Run())
runner->Quit();
}
void WaitForCondition(
const base::Callback<bool(void)>& condition,
NotificationSet* notification_set) {
if (condition.Run())
return;
scoped_refptr<content::MessageLoopRunner> runner(
new content::MessageLoopRunner);
scoped_ptr<base::CallbackList<void()>::Subscription> subscription =
notification_set->callback_list().Add(
base::Bind(&MaybeQuit, base::Unretained(runner.get()), condition));
runner->Run();
}
void WaitForCondition(
const base::Callback<bool(void)>& condition,
int type) {
NotificationSet notification_set;
notification_set.Add(type);
WaitForCondition(condition, &notification_set);
}
} // namespace
////////////////////////////////////////////////////////////////////////////////
// ExtensionTestNotificationObserver
ExtensionTestNotificationObserver::ExtensionTestNotificationObserver(
Browser* browser)
......@@ -147,10 +127,12 @@ bool ExtensionTestNotificationObserver::WaitForPageActionVisibilityChangeTo(
int count) {
LocationBarTesting* location_bar =
browser_->window()->GetLocationBar()->GetLocationBarForTesting();
extensions::ExtensionActionAPI::Get(GetProfile())->AddObserver(this);
WaitForCondition(
base::Bind(
&HasExtensionPageActionVisibilityReachedTarget, location_bar, count),
extensions::NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED);
base::Bind(&HasPageActionVisibilityReachedTarget, location_bar, count),
NULL);
extensions::ExtensionActionAPI::Get(GetProfile())->
RemoveObserver(this);
return true;
}
......@@ -271,3 +253,36 @@ void ExtensionTestNotificationObserver::Observe(
break;
}
}
void ExtensionTestNotificationObserver::OnPageActionsUpdated(
content::WebContents* web_contents) {
MaybeQuit();
}
void ExtensionTestNotificationObserver::WaitForCondition(
const ConditionCallback& condition,
NotificationSet* notification_set) {
if (condition.Run())
return;
condition_ = condition;
scoped_refptr<content::MessageLoopRunner> runner(
new content::MessageLoopRunner);
quit_closure_ = runner->QuitClosure();
scoped_ptr<base::CallbackList<void()>::Subscription> subscription;
if (notification_set) {
subscription = notification_set->callback_list().Add(
base::Bind(&ExtensionTestNotificationObserver::MaybeQuit,
base::Unretained(this)));
}
runner->Run();
condition_.Reset();
quit_closure_.Reset();
}
void ExtensionTestNotificationObserver::MaybeQuit() {
if (condition_.Run())
quit_closure_.Run();
}
......@@ -7,8 +7,10 @@
#include <string>
#include "base/callback.h"
#include "base/compiler_specific.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "content/public/browser/notification_details.h"
......@@ -20,14 +22,13 @@ class WindowedNotificationObserver;
}
// Test helper class for observing extension-related events.
class ExtensionTestNotificationObserver : public content::NotificationObserver {
class ExtensionTestNotificationObserver
: public content::NotificationObserver,
public extensions::ExtensionActionAPI::Observer {
public:
explicit ExtensionTestNotificationObserver(Browser* browser);
virtual ~ExtensionTestNotificationObserver();
// Wait for the total number of page actions to change to |count|.
bool WaitForPageActionCountChangeTo(int count);
// Wait for the number of visible page actions to change to |count|.
bool WaitForPageActionVisibilityChangeTo(int count);
......@@ -82,10 +83,25 @@ class ExtensionTestNotificationObserver : public content::NotificationObserver {
const content::NotificationDetails& details) OVERRIDE;
private:
class NotificationSet;
Profile* GetProfile();
void WaitForNotification(int notification_type);
// Wait for |condition_| to be met. |notification_set| is the set of
// notifications to wait for and to check |condition| when observing. This
// can be NULL if we are instead waiting for a different observer method, like
// OnPageActionsUpdated().
void WaitForCondition(const base::Callback<bool(void)>& condition,
NotificationSet* notification_set);
// Quits the message loop if |condition_| is met.
void MaybeQuit();
// extensions::ExtensionActionAPI::Observer:
virtual void OnPageActionsUpdated(content::WebContents* contents) OVERRIDE;
Browser* browser_;
Profile* profile_;
......@@ -96,6 +112,13 @@ class ExtensionTestNotificationObserver : public content::NotificationObserver {
int extension_installs_observed_;
int extension_load_errors_observed_;
int crx_installers_done_observed_;
// The condition for which we are waiting. This should be checked in any
// observing methods that could trigger it.
base::Callback<bool(void)> condition_;
// The closure to quit the currently-running message loop.
base::Closure quit_closure_;
};
#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_TEST_NOTIFICATION_OBSERVER_H_
......@@ -7,7 +7,6 @@
#include "base/scoped_observer.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/browser_action_test_util.h"
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_service.h"
......@@ -24,7 +23,6 @@
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/bookmarks/test/bookmark_test_helpers.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test_utils.h"
#include "extensions/browser/extension_host.h"
......@@ -172,9 +170,6 @@ IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest, BroadcastEvent) {
// Open a tab to a URL that will trigger the page action to show.
LazyBackgroundObserver page_complete;
content::WindowedNotificationObserver page_action_changed(
extensions::NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED,
content::NotificationService::AllSources());
ui_test_utils::NavigateToURL(
browser(), embedded_test_server()->GetURL("/extensions/test_file.html"));
page_complete.Wait();
......@@ -182,7 +177,7 @@ IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest, BroadcastEvent) {
EXPECT_FALSE(IsBackgroundPageAlive(last_loaded_extension_id()));
// Page action is shown.
page_action_changed.Wait();
WaitForPageActionVisibilityChangeTo(num_page_actions + 1);
EXPECT_EQ(num_page_actions + 1,
browser()->window()->GetLocationBar()->
GetLocationBarForTesting()->PageActionVisibleCount());
......
......@@ -6,18 +6,12 @@
#include "base/logging.h"
#include "chrome/browser/extensions/active_script_controller.h"
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
#include "chrome/browser/extensions/page_action_controller.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/common/extensions/api/extension_action/action_info.h"
#include "content/public/browser/invalidate_type.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/web_contents.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/notification_types.h"
namespace extensions {
......@@ -57,23 +51,6 @@ std::vector<ExtensionAction*> LocationBarController::GetCurrentActions() {
return current_actions;
}
// static
void LocationBarController::NotifyChange(content::WebContents* web_contents) {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
if (!browser)
return;
LocationBar* location_bar =
browser->window() ? browser->window()->GetLocationBar() : NULL;
if (!location_bar)
return;
location_bar->UpdatePageActions();
content::NotificationService::current()->Notify(
NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED,
content::Source<content::WebContents>(web_contents),
content::NotificationService::NoDetails());
}
void LocationBarController::DidNavigateMainFrame(
const content::LoadCommittedDetails& details,
const content::FrameNavigateParams& params) {
......@@ -98,8 +75,10 @@ void LocationBarController::OnExtensionUnloaded(
should_update = true;
}
if (should_update)
NotifyChange(web_contents());
if (should_update) {
ExtensionActionAPI::Get(browser_context)->
NotifyPageActionsChanged(web_contents());
}
}
} // namespace extensions
......@@ -44,8 +44,8 @@ class LocationBarController : public content::WebContentsObserver,
// A notification that the given |extension| has been unloaded, and any
// actions associated with it should be removed.
// The location bar controller will update itself after this if needed, so
// Providers should not call NotifyChange().
// The LocationBarController will handle notifying of page action changes,
// if any.
virtual void OnExtensionUnloaded(const Extension* extension) {}
};
......@@ -55,9 +55,6 @@ class LocationBarController : public content::WebContentsObserver,
// Returns the actions which should be displayed in the location bar.
std::vector<ExtensionAction*> GetCurrentActions();
// Notifies the window that the actions have changed.
static void NotifyChange(content::WebContents* web_contents);
ActiveScriptController* active_script_controller() {
return active_script_controller_.get();
}
......
......@@ -4,32 +4,14 @@
#include "chrome/browser/extensions/page_action_controller.h"
#include <set>
#include "base/lazy_instance.h"
#include "base/metrics/histogram.h"
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
#include "chrome/browser/extensions/extension_action.h"
#include "chrome/browser/extensions/extension_action_manager.h"
#include "chrome/browser/extensions/tab_helper.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sessions/session_tab_helper.h"
#include "content/public/browser/navigation_details.h"
#include "content/public/browser/web_contents.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/extension_set.h"
namespace extensions {
// Keeps track of the profiles for which we've sent UMA statistics.
base::LazyInstance<std::set<Profile*> > g_reported_for_profiles =
LAZY_INSTANCE_INITIALIZER;
PageActionController::PageActionController(content::WebContents* web_contents)
: web_contents_(web_contents),
extension_action_observer_(this) {
extension_action_observer_.Add(
ExtensionActionAPI::Get(web_contents_->GetBrowserContext()));
browser_context_(web_contents_->GetBrowserContext()) {
}
PageActionController::~PageActionController() {
......@@ -37,7 +19,8 @@ PageActionController::~PageActionController() {
ExtensionAction* PageActionController::GetActionForExtension(
const Extension* extension) {
return ExtensionActionManager::Get(GetProfile())->GetPageAction(*extension);
return ExtensionActionManager::Get(browser_context_)->GetPageAction(
*extension);
}
void PageActionController::OnNavigated() {
......@@ -45,17 +28,4 @@ void PageActionController::OnNavigated() {
// do here.
}
void PageActionController::OnExtensionActionUpdated(
ExtensionAction* extension_action,
content::WebContents* web_contents,
content::BrowserContext* browser_context) {
if (web_contents == web_contents_ &&
extension_action->action_type() == ActionInfo::TYPE_PAGE)
LocationBarController::NotifyChange(web_contents_);
}
Profile* PageActionController::GetProfile() {
return Profile::FromBrowserContext(web_contents_->GetBrowserContext());
}
} // namespace extensions
......@@ -5,21 +5,21 @@
#ifndef CHROME_BROWSER_EXTENSIONS_PAGE_ACTION_CONTROLLER_H_
#define CHROME_BROWSER_EXTENSIONS_PAGE_ACTION_CONTROLLER_H_
#include <string>
#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
#include "base/macros.h"
#include "chrome/browser/extensions/location_bar_controller.h"
class Profile;
namespace content {
class BrowserContext;
class WebContents;
}
namespace extensions {
class ExtensionRegistry;
class Extension;
// A LocationBarControllerProvider which populates the location bar with icons
// based on the page_action extension API.
// TODO(rdevlin.cronin): This isn't really a controller.
class PageActionController : public LocationBarController::ActionProvider,
public ExtensionActionAPI::Observer {
class PageActionController : public LocationBarController::ActionProvider {
public:
explicit PageActionController(content::WebContents* web_contents);
virtual ~PageActionController();
......@@ -30,20 +30,11 @@ class PageActionController : public LocationBarController::ActionProvider,
virtual void OnNavigated() OVERRIDE;
private:
// ExtensionActionAPI::Observer implementation.
virtual void OnExtensionActionUpdated(
ExtensionAction* extension_action,
content::WebContents* web_contents,
content::BrowserContext* browser_context) OVERRIDE;
// Returns the associated Profile.
Profile* GetProfile();
// The associated WebContents.
content::WebContents* web_contents_;
ScopedObserver<ExtensionActionAPI, ExtensionActionAPI::Observer>
extension_action_observer_;
// The associated browser context.
content::BrowserContext* browser_context_;
DISALLOW_COPY_AND_ASSIGN(PageActionController);
};
......
......@@ -143,10 +143,6 @@ LocationBarViewMac::LocationBarViewMac(AutocompleteTextField* field,
new ContentSettingDecoration(type, this, profile));
}
registrar_.Add(
this,
extensions::NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED,
content::NotificationService::AllSources());
content::Source<Profile> profile_source = content::Source<Profile>(profile);
registrar_.Add(this,
extensions::NOTIFICATION_EXTENSION_LOADED_DEPRECATED,
......@@ -227,6 +223,9 @@ void LocationBarViewMac::UpdateManagePasswordsIconAndBubble() {
void LocationBarViewMac::UpdatePageActions() {
RefreshPageActionDecorations();
Layout();
[field_ updateMouseTracking];
[field_ setNeedsDisplay:YES];
}
void LocationBarViewMac::InvalidatePageActions() {
......@@ -624,25 +623,10 @@ NSImage* LocationBarViewMac::GetKeywordImage(const base::string16& keyword) {
void LocationBarViewMac::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
switch (type) {
case extensions::NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED: {
if (content::Source<WebContents>(source).ptr() != GetWebContents())
return;
[field_ updateMouseTracking];
[field_ setNeedsDisplay:YES];
break;
}
case extensions::NOTIFICATION_EXTENSION_LOADED_DEPRECATED:
case extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED:
Update(NULL);
break;
DCHECK(type == extensions::NOTIFICATION_EXTENSION_LOADED_DEPRECATED ||
type == extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED);
default:
NOTREACHED() << "Unexpected notification";
break;
}
Update(NULL);
}
void LocationBarViewMac::ModelChanged(const SearchModel::State& old_state,
......
......@@ -126,10 +126,6 @@ enum NotificationType {
// extension's ID.
NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
// Sent when the page actions have been updated. The source is a WebContents*,
// and there are no details.
NOTIFICATION_EXTENSION_PAGE_ACTIONS_UPDATED,
// Sent when an extension command has been removed. The source is the
// BrowserContext* and the details is a std::pair of two std::string objects
// (an extension ID and the name of the command being removed).
......
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