Clean up NOTIFICATION_EXTENSION_PAGE_ACTION_[ COUNT | VISIBILITY ]_CHANGED

chrome/browser/extensions/location_bar_controller.cc

 // Copyright 2014 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/location_bar_controller.h"
 
 #include "base/logging.h"
 #include "chrome/browser/extensions/active_script_controller.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 {
 
 LocationBarController::LocationBarController(
     content::WebContents* web_contents)
     : WebContentsObserver(web_contents),
       web_contents_(web_contents),
       active_script_controller_(new ActiveScriptController(web_contents_)),
       page_action_controller_(new PageActionController(web_contents_)),
       extension_registry_observer_(this) {
@@ skipping 50 matching lines @@
 // 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());

[Avi (use Gerrit), 2014/08/20 01:06:52]

We're trying to kill off notifications. Overall this is an improvement, I
suppose, but can we move to some kind of registration? Can we have the
controller use an observer or callback interface? (base/observer_list.h or
base/callback_list.h)

[Devlin, 2014/08/20 03:21:00]

TL;DR: That's the eventual goal, but I'd like to push this through in the
meantime (for reasons described in the bug and CL description, I think it's
still a big improvement).

Yeah, I know we're trying to kill off notifications...but... ;)

It's not a simple change like just making the LocationBarController have a
subscription/observer list because there are a bunch of different
LocationBarControllers (one per web contents), and not everything that cares
about PAGE_ACTION_VISIBILITY_CHANGED (old)/PAGE_ACTIONS_UPDATED (new) wants to
track each new tab created.

Eventually, I *would* like to move this to be an observer method on
ExtensionActionAPI::Observer (newly-added to take the place of other
notifications), but I want to hold off because:
a) I want to keep these patches close to the original implementation, since it's
juggling a lot of different parts.
b) I'd like to keep these patches small for ease comparing old vs new behavior.

In the meantime, I'd like to push this through so we have something to work off,
and because this change is still a step in the right direction towards moving
this towards an observer method.

Okay, done defending. :)  If you still would prefer me to move it all in this
patch, I can.  Just wanted to explain my reasoning, and assure you that it
wasn't gonna stay like this indefinitely.

 }
 
 void LocationBarController::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   if (details.is_in_page)
     return;
 
   page_action_controller_->OnNavigated();
   active_script_controller_->OnNavigated();
@@ skipping 11 matching lines @@
   if (active_script_controller_->GetActionForExtension(extension)) {
     active_script_controller_->OnExtensionUnloaded(extension);
     should_update = true;
   }
 
   if (should_update)
     NotifyChange(web_contents());
 }
 
 }  // namespace extensions