Make ExtensionSettingsHandler only listen to notifications pertaining to its profile.

chrome/browser/ui/webui/options/extension_settings_handler.cc

 // Copyright (c) 2011 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/ui/webui/options/extension_settings_handler.h"
 
 #include "base/auto_reset.h"
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/browser/debugger/devtools_window.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_disabled_infobar_delegate.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_updater.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/background_contents.h"
 #include "chrome/browser/ui/webui/extension_icon_source.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/common/chrome_view_types.h"
+#include "content/browser/browsing_instance.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/browser/tab_contents/tab_contents_view.h"
 #include "content/common/content_notification_types.h"
 #include "content/common/notification_service.h"
 #include "grit/browser_resources.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
@@ skipping 148 matching lines @@
                                   results);
 
   MaybeRegisterForNotifications();
 }
 
 void ExtensionSettingsHandler::MaybeRegisterForNotifications() {
   if (registered_for_notifications_)
     return;
 
   registered_for_notifications_  = true;
+  Profile* profile = Profile::FromWebUI(web_ui_);
 
   // Register for notifications that we need to reload the page.
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
-      NotificationService::AllSources());
+                 Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_PROCESS_CREATED,
-      NotificationService::AllSources());
+                 Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
-      NotificationService::AllSources());
+                 Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED,
-      NotificationService::AllSources());
+                 Source<Profile>(profile));
   registrar_.Add(this,
-      content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-      NotificationService::AllSources());
+                 content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+                 NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
-      content::NOTIFICATION_RENDER_VIEW_HOST_CREATED,
-      NotificationService::AllSources());
+                 content::NOTIFICATION_RENDER_VIEW_HOST_CREATED,
+                 NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
-      content::NOTIFICATION_RENDER_VIEW_HOST_DELETED,
-      NotificationService::AllSources());
+                 content::NOTIFICATION_RENDER_VIEW_HOST_DELETED,
+                 NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
-      chrome::NOTIFICATION_BACKGROUND_CONTENTS_NAVIGATED,
-      NotificationService::AllSources());
+                 chrome::NOTIFICATION_BACKGROUND_CONTENTS_NAVIGATED,
+                 NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this,
-      chrome::NOTIFICATION_BACKGROUND_CONTENTS_DELETED,
-      NotificationService::AllSources());
-  registrar_.Add(this,
+                 chrome::NOTIFICATION_BACKGROUND_CONTENTS_DELETED,
+                 NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(
+      this,
       chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
-      NotificationService::AllSources());
+      Source<ExtensionPrefs>(profile->GetExtensionService()->
+                             extension_prefs()));
 }
 
 ExtensionUninstallDialog*
 ExtensionSettingsHandler::GetExtensionUninstallDialog() {
   if (!extension_uninstall_dialog_.get()) {
     extension_uninstall_dialog_.reset(
         ExtensionUninstallDialog::Create(Profile::FromWebUI(web_ui_), this));
   }
   return extension_uninstall_dialog_.get();
 }
@@ skipping 331 matching lines @@
   extension_service_ = Profile::FromWebUI(web_ui_)
       ->GetOriginalProfile()->GetExtensionService();
 
   // Return result from the superclass.
   return handler;
 }
 
 void ExtensionSettingsHandler::Observe(int type,
                                        const NotificationSource& source,
                                        const NotificationDetails& details) {
+  Profile* this_profile = Profile::FromWebUI(web_ui_);
   switch (type) {
     // We listen for notifications that will result in the page being
     // repopulated with data twice for the same event in certain cases.
     // For instance, EXTENSION_LOADED & EXTENSION_PROCESS_CREATED because
     // we don't know about the views for an extension at EXTENSION_LOADED, but
     // if we only listen to EXTENSION_PROCESS_CREATED, we'll miss extensions
     // that don't have a process at startup. Similarly, NAV_ENTRY_COMMITTED &
     // RENDER_VIEW_HOST_CREATED because we want to handle both
     // the case of navigating from a non-extension page to an extension page in
     // a TabContents (which will generate NAV_ENTRY_COMMITTED) as well as
     // extension content being shown in popups and balloons (which will generate
     // RENDER_VIEW_CREATED but no NAV_ENTRY_COMMITTED).
     //
     // Doing it this way gets everything but causes the page to be rendered
     // more than we need. It doesn't seem to result in any noticeable flicker.
+    case content::NOTIFICATION_RENDER_VIEW_HOST_CREATED:
+      {
+        Profile* profile = Profile::FromBrowserContext(
+            Source<RenderViewHost>(source)->site_instance()->
+            browsing_instance()->browser_context());
+        if (!this_profile->IsSameProfile(profile))
+          return;
+        MaybeUpdateAfterNotification();
+        break;
+      }
     case content::NOTIFICATION_RENDER_VIEW_HOST_DELETED:
-      deleting_rvh_ = Source<RenderViewHost>(source).ptr();
-      MaybeUpdateAfterNotification();
-      break;
+      {
+        Profile* profile = Profile::FromBrowserContext(
+            Source<RenderViewHost>(source)->site_instance()->
+            browsing_instance()->browser_context());
+        if (!this_profile->IsSameProfile(profile))
+          return;
+        deleting_rvh_ = Source<RenderViewHost>(source).ptr();
+        MaybeUpdateAfterNotification();
+        break;
+      }
     case chrome::NOTIFICATION_BACKGROUND_CONTENTS_DELETED:
-      deleting_rvh_ = Details<BackgroundContents>(details)->render_view_host();
-      MaybeUpdateAfterNotification();
-      break;
+      {
+        Profile* profile = Profile::FromBrowserContext(
+            Details<BackgroundContents>(details)->render_view_host()->
+            site_instance()->browsing_instance()->browser_context());
+        if (!this_profile->IsSameProfile(profile))
+          return;
+        deleting_rvh_ =
+            Details<BackgroundContents>(details)->render_view_host();
+        MaybeUpdateAfterNotification();
+        break;
+      }
+    case chrome::NOTIFICATION_BACKGROUND_CONTENTS_NAVIGATED:
+      {
+        Profile* profile = Profile::FromBrowserContext(
+            Details<BackgroundContents>(details)->render_view_host()->
+            site_instance()->browsing_instance()->browser_context());
+        if (!this_profile->IsSameProfile(profile))
+          return;
+        MaybeUpdateAfterNotification();
+        break;
+      }
+    case content::NOTIFICATION_NAV_ENTRY_COMMITTED:
+      {
+        Profile* profile = Profile::FromBrowserContext(
+            Source<NavigationController>(source).ptr()->browser_context());
+        if (!this_profile->IsSameProfile(profile))
+          return;
+        MaybeUpdateAfterNotification();
+        break;
+      }

[Finnur, 2011/10/10 10:34:53]

There's a lot of duplicate code here... I'm wondering... can't you collapse this
into:

Profile* profile = NULL;
bool compare_profiles = true;
switch (type) {
  case content::NOTIFICATION_RENDER_VIEW_HOST_CREATED:
    profile = Profile::FromBrowserContext(
        Source<RenderViewHost>(source)->site_instance()->
        browsing_instance()->browser_context());
    break;
  case content::NOTIFICATION_RENDER_VIEW_HOST_DELETED:
    profile = ...;
    break;
  case chrome::NOTIFICATION_BACKGROUND_CONTENTS_DELETED:
    profile = ...;
    deleting_rvh = ...;
    break;
  ... etc ...
  case chrome::NOTIFICATION_EXTENSION_LOADED:
  case chrome::NOTIFICATION_EXTENSION_PROCESS_CREATED:
  ... etc ...
    compare_profiles = false;
    break;
  default:
    NOTREACHED();
    break;
}

if (compare_profiles && !this_profile->IsSameProfile(profile))
  return;

MaybeUpdateAfterNotification();


What do you think? Think this will improve things or make things worse? (I'll
leave it up to you).

     case chrome::NOTIFICATION_EXTENSION_LOADED:
     case chrome::NOTIFICATION_EXTENSION_PROCESS_CREATED:
     case chrome::NOTIFICATION_EXTENSION_UNLOADED:
     case chrome::NOTIFICATION_EXTENSION_UPDATE_DISABLED:
-    case content::NOTIFICATION_RENDER_VIEW_HOST_CREATED:
-    case content::NOTIFICATION_NAV_ENTRY_COMMITTED:
-    case chrome::NOTIFICATION_BACKGROUND_CONTENTS_NAVIGATED:
     case chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED:
       MaybeUpdateAfterNotification();
       break;
     default:
       NOTREACHED();
   }
 }
 
 const Extension* ExtensionSettingsHandler::GetExtension(const ListValue* args) {
   std::string extension_id = UTF16ToUTF8(ExtractStringValue(args));
@@ skipping 130 matching lines @@
         continue;
     } else if (!extension->web_extent().MatchesURL(url)) {
       continue;
     }
 
     result->push_back(
         ExtensionPage(url, process->id(), host->routing_id(),
                       process->browser_context()->IsOffTheRecord()));
   }
 }