Remove deprecated extension notification from Browser

chrome/browser/ui/browser.cc

 // Copyright 2012 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/browser.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #include <shellapi.h>
 #endif  // defined(OS_WIN)
@@ skipping 163 matching lines @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/page_zoom.h"
 #include "content/public/common/renderer_preferences.h"
 #include "content/public/common/webplugininfo.h"
 #include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "net/base/filename_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/url_request/url_request_context.h"
 #include "third_party/WebKit/public/web/WebWindowFeatures.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 127 matching lines @@
  private:
   Browser* browser_;
 
   DISALLOW_COPY_AND_ASSIGN(InterstitialObserver);
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, Constructors, Creation, Showing:
 
 Browser::Browser(const CreateParams& params)
-    : type_(params.type),
+    : extension_registry_observer_(this),
+      type_(params.type),
       profile_(params.profile),
       window_(NULL),
       tab_strip_model_delegate_(new chrome::BrowserTabStripModelDelegate(this)),
-      tab_strip_model_(new TabStripModel(tab_strip_model_delegate_.get(),
-                                         params.profile)),
+      tab_strip_model_(
+          new TabStripModel(tab_strip_model_delegate_.get(), params.profile)),
       app_name_(params.app_name),
       is_trusted_source_(params.trusted_source),
       cancel_download_confirmation_state_(NOT_PROMPTED),
       override_bounds_(params.initial_bounds),
       initial_show_state_(params.initial_show_state),
       is_session_restore_(params.is_session_restore),
-      host_desktop_type_(BrowserWindow::AdjustHostDesktopType(
-          params.host_desktop_type)),
+      host_desktop_type_(
+          BrowserWindow::AdjustHostDesktopType(params.host_desktop_type)),
       content_setting_bubble_model_delegate_(
           new BrowserContentSettingBubbleModelDelegate(this)),
       toolbar_model_delegate_(new BrowserToolbarModelDelegate(this)),
       tab_restore_service_delegate_(new BrowserTabRestoreServiceDelegate(this)),
       synced_window_delegate_(new BrowserSyncedWindowDelegate(this)),
       bookmark_bar_state_(BookmarkBar::HIDDEN),
       command_controller_(new chrome::BrowserCommandController(this)),
       window_has_shown_(false),
       chrome_updater_factory_(this),
       weak_factory_(this),
@@ skipping 13 matching lines @@
     unload_controller_.reset(new chrome::UnloadController(this));
 
   if (!app_name_.empty())
     chrome::RegisterAppPrefs(app_name_, profile_);
   tab_strip_model_->AddObserver(this);
 
   toolbar_model_.reset(new ToolbarModelImpl(toolbar_model_delegate_.get()));
   search_model_.reset(new SearchModel());
   search_delegate_.reset(new SearchDelegate(search_model_.get()));
 
-  registrar_.Add(this,
-                 extensions::NOTIFICATION_EXTENSION_LOADED_DEPRECATED,
-                 content::Source<Profile>(profile_->GetOriginalProfile()));
-  registrar_.Add(this,
-                 extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED,
-                 content::Source<Profile>(profile_->GetOriginalProfile()));
-  registrar_.Add(this,
-                 extensions::NOTIFICATION_EXTENSION_UNINSTALLED_DEPRECATED,
-                 content::Source<Profile>(profile_->GetOriginalProfile()));
+  extension_registry_observer_.Add(
+      extensions::ExtensionRegistry::Get(profile_));
+
   registrar_.Add(this,
                  extensions::NOTIFICATION_EXTENSION_PROCESS_TERMINATED,
                  content::NotificationService::AllSources());
+
 #if defined(ENABLE_THEMES)
   registrar_.Add(
       this, chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
       content::Source<ThemeService>(
           ThemeServiceFactory::GetForProfile(profile_)));
 #endif
   registrar_.Add(this, chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
                  content::NotificationService::AllSources());
 
   profile_pref_registrar_.Init(profile_->GetPrefs());
@@ skipping 1498 matching lines @@
   if (data.web_contents == tab_strip_model_->GetActiveWebContents()) {
     // Only show the zoom bubble for zoom changes in the active window.
     window_->ZoomChangedForActiveTab(data.can_show_bubble &&
                                      window_->IsActive());
     // Change the zoom commands state based on the zoom state
     command_controller_->ZoomStateChanged();
   }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, ui::SelectFileDialog::Listener implementation:

[Peter Kasting, 2014/09/10 18:40:20]

Nit: I'm not a fan of these big dividers between subclass implementation
sections, but we should be consistent; either add one before your new functions,
or remove all of them in this file.  (Personally, I'd do the latter.)

[Jitu( very slow this week), 2014/09/11 06:08:16]

Dear Peter,
I have not touched this function. So as per your comments , can you please
clarify where exactly i need to move this function.

[Bernhard Bauer, 2014/09/11 08:09:20]

IIUC, Peter's comment is not about the function, but about this comment that
separates different sections. To be consistent, there should be either one
before the ExtensionRegistryObserver methods as well, or none at all throughout
the whole file.

 
 void Browser::FileSelected(const base::FilePath& path, int index,
                            void* params) {
   FileSelectedWithExtraInfo(ui::SelectedFileInfo(path, path), index, params);
 }
 
 void Browser::FileSelectedWithExtraInfo(const ui::SelectedFileInfo& file_info,
                                         int index,
                                         void* params) {
   profile_->set_last_selected_directory(file_info.file_path.DirName());
 
   GURL url = net::FilePathToFileURL(file_info.local_path);
 
 #if defined(OS_CHROMEOS)
   drive::util::MaybeSetDriveURL(profile_, file_info.file_path, &url);
 #endif
 
   if (url.is_empty())
     return;
 
   OpenURL(OpenURLParams(
       url, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED, false));
 }
 
+// Called when an extension is uninstalled

[Peter Kasting, 2014/09/10 18:40:20]

Nit: Don't add comments like this.  If they're important, they belong in the .h
file, and they need to be complete sentences.  In this case, though, the comment
just restates the function name, so it doesn't add anything to the code.  Just
remove these outright.  (3 places)

[Jitu( very slow this week), 2014/09/11 06:08:16]

Done.

+void Browser::OnExtensionUninstalled(content::BrowserContext* browser_context,
+                                     const extensions::Extension* extension,
+                                     extensions::UninstallReason reason) {
+  if (window() && window()->GetLocationBar())
+    window()->GetLocationBar()->UpdatePageActions();
+}
+
+// Called when an extension is loaded
+void Browser::OnExtensionLoaded(content::BrowserContext* browser_context,
+                                const extensions::Extension* extension) {
+  chrome::UpdateCommandEnabled(
+      this,
+      IDC_BOOKMARK_PAGE,
+      !chrome::ShouldRemoveBookmarkThisPageUI(profile_));
+  chrome::UpdateCommandEnabled(
+      this,
+      IDC_BOOKMARK_ALL_TABS,
+      !chrome::ShouldRemoveBookmarkOpenPagesUI(profile_));
+}
+
+// Called when an extension is unloaded
+void Browser::OnExtensionUnloaded(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension,
+    extensions::UnloadedExtensionInfo::Reason reason) {
+  chrome::UpdateCommandEnabled(
+      this,
+      IDC_BOOKMARK_PAGE,
+      !chrome::ShouldRemoveBookmarkThisPageUI(profile_));
+  chrome::UpdateCommandEnabled(
+      this,
+      IDC_BOOKMARK_ALL_TABS,
+      !chrome::ShouldRemoveBookmarkOpenPagesUI(profile_));
+
+  if (window()->GetLocationBar())
+    window()->GetLocationBar()->UpdatePageActions();
+
+  // Close any tabs from the unloaded extension, unless it's terminated,
+  // in which case let the sad tabs remain.
+  if (reason != extensions::UnloadedExtensionInfo::REASON_TERMINATE) {
+    // Iterate backwards as we may remove items while iterating.
+    for (int i = tab_strip_model_->count() - 1; i >= 0; --i) {
+      WebContents* web_contents = tab_strip_model_->GetWebContentsAt(i);
+      // Two cases are handled here:
+      // - The scheme check is for when an extension page is loaded in a
+      //   tab, e.g. chrome-extension://id/page.html.
+      // - The extension_app check is for apps, which can have non-extension
+      //   schemes, e.g. https://mail.google.com if you have the Gmail app
+      //   installed.
+      if ((web_contents->GetURL().SchemeIs(extensions::kExtensionScheme) &&
+           web_contents->GetURL().host() == extension->id()) ||
+          (extensions::TabHelper::FromWebContents(web_contents)
+               ->extension_app() == extension)) {
+        tab_strip_model_->CloseWebContentsAt(i, TabStripModel::CLOSE_NONE);
+      }
+    }
+  }
+}
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, content::NotificationObserver implementation:
 
 void Browser::Observe(int type,
                       const content::NotificationSource& source,
                       const content::NotificationDetails& details) {
   switch (type) {
-    case extensions::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED: {
-      chrome::UpdateCommandEnabled(
-          this,
-          IDC_BOOKMARK_PAGE,
-          !chrome::ShouldRemoveBookmarkThisPageUI(profile_));
-      chrome::UpdateCommandEnabled(
-          this,
-          IDC_BOOKMARK_ALL_TABS,
-          !chrome::ShouldRemoveBookmarkOpenPagesUI(profile_));
-
-      if (window()->GetLocationBar())
-        window()->GetLocationBar()->UpdatePageActions();
-
-      const extensions::UnloadedExtensionInfo* extension_info =
-          content::Details<extensions::UnloadedExtensionInfo>(details).ptr();
-
-      // Close any tabs from the unloaded extension, unless it's terminated,
-      // in which case let the sad tabs remain.
-      if (extension_info->reason !=
-          extensions::UnloadedExtensionInfo::REASON_TERMINATE) {
-        const Extension* extension = extension_info->extension;
-        // Iterate backwards as we may remove items while iterating.
-        for (int i = tab_strip_model_->count() - 1; i >= 0; --i) {
-          WebContents* web_contents = tab_strip_model_->GetWebContentsAt(i);
-          // Two cases are handled here:
-          // - The scheme check is for when an extension page is loaded in a
-          //   tab, e.g. chrome-extension://id/page.html.
-          // - The extension_app check is for apps, which can have non-extension
-          //   schemes, e.g. https://mail.google.com if you have the Gmail app
-          //   installed.
-          if ((web_contents->GetURL().SchemeIs(extensions::kExtensionScheme) &&
-               web_contents->GetURL().host() == extension->id()) ||
-              (extensions::TabHelper::FromWebContents(
-                   web_contents)->extension_app() == extension)) {
-            tab_strip_model_->CloseWebContentsAt(i, TabStripModel::CLOSE_NONE);
-          }
-        }
-      }
-      break;
-    }
-
     case extensions::NOTIFICATION_EXTENSION_PROCESS_TERMINATED: {
       Profile* profile = content::Source<Profile>(source).ptr();
       if (profile_->IsSameProfile(profile) && window()->GetLocationBar())
         window()->GetLocationBar()->InvalidatePageActions();
       break;
     }
 
-    case extensions::NOTIFICATION_EXTENSION_LOADED_DEPRECATED:
-      chrome::UpdateCommandEnabled(
-          this,
-          IDC_BOOKMARK_PAGE,
-          !chrome::ShouldRemoveBookmarkThisPageUI(profile_));
-      chrome::UpdateCommandEnabled(
-          this,
-          IDC_BOOKMARK_ALL_TABS,
-          !chrome::ShouldRemoveBookmarkOpenPagesUI(profile_));
-    // fallthrough
-    case extensions::NOTIFICATION_EXTENSION_UNINSTALLED_DEPRECATED:
-      // During window creation on Windows we may end up calling into
-      // SHAppBarMessage, which internally spawns a nested message loop. This
-      // makes it possible for us to end up here before window creation has
-      // completed, at which point window_ is NULL. See 94752 for details.

[Peter Kasting, 2014/09/10 18:40:20]

Nit: Please copy this comment to your new code location.

[Jitu( very slow this week), 2014/09/11 06:08:16]

Done.

-      if (window() && window()->GetLocationBar())
-        window()->GetLocationBar()->UpdatePageActions();
-      break;
-
 #if defined(ENABLE_THEMES)
     case chrome::NOTIFICATION_BROWSER_THEME_CHANGED:
       window()->UserChangedTheme();
       break;
 #endif
 
     case chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED: {
       WebContents* web_contents = content::Source<WebContents>(source).ptr();
       if (web_contents == tab_strip_model_->GetActiveWebContents()) {
         LocationBar* location_bar = window()->GetLocationBar();
@@ skipping 455 matching lines @@
   if (contents && !allow_js_access) {
     contents->web_contents()->GetController().LoadURL(
         target_url,
         content::Referrer(),
         content::PAGE_TRANSITION_LINK,
         std::string());  // No extra headers.
   }
 
   return contents != NULL;
 }