content: convert zoom notifications to observer usage

chrome/browser/ui/zoom/zoom_controller.cc

Index: chrome/browser/ui/zoom/zoom_controller.cc
diff --git a/chrome/browser/ui/zoom/zoom_controller.cc b/chrome/browser/ui/zoom/zoom_controller.cc
index 24b33baad649d27d4287532503a79d6d1ad08c83..a333e7840e8aae23611ab20ff52fb865d6a6e149 100644
--- a/chrome/browser/ui/zoom/zoom_controller.cc
+++ b/chrome/browser/ui/zoom/zoom_controller.cc
@@ -24,7 +24,9 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(ZoomController);
 ZoomController::ZoomController(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       zoom_percent_(100),
-      observer_(NULL) {
+      observer_(NULL),
+      zoom_callback_(base::Bind(&ZoomController::OnZoomLevelChanged,
+                                base::Unretained(this))) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   default_zoom_level_.Init(prefs::kDefaultZoomLevel, profile->GetPrefs(),
@@ -32,15 +34,21 @@ ZoomController::ZoomController(content::WebContents* web_contents)
                                       base::Unretained(this),
                                       std::string()));
 
-  content::HostZoomMap* zoom_map =
-      content::HostZoomMap::GetForBrowserContext(profile);
-  registrar_.Add(this, content::NOTIFICATION_ZOOM_LEVEL_CHANGED,
-                 content::Source<content::HostZoomMap>(zoom_map));
+  content::HostZoomMap::GetForBrowserContext(
+      web_contents->GetBrowserContext())->AddZoomLevelChangedCallback(
+          zoom_callback_);
 
   UpdateState(std::string());
 }
 
-ZoomController::~ZoomController() {}
+ZoomController::~ZoomController() {
+  if (!web_contents())
+    return;

[Avi (use Gerrit), 2013/01/31 18:46:55]

This looks like a very, very bad idea.

Yes, as a WebContentsUserData the ZoomController is owned by a WebContents, but
during destruction the WebContents is destroyed first. web_contents() will never
be non null here. If the HostZoomMap outlives the WebContents, you're almost
guaranteeing a call through a busted callback.

If you need to deregister a callback, ZoomController is a WebContentsObserver.
Override WebContentsDestroyed.

[Paweł Hajdan Jr., 2013/02/01 12:36:11]

Thanks for catching that!
I used a slightly different solution, just saved pointer to BrowserContext. IMHO
it's easier to verify the symmetry of adding callback in ctor and removing in
dtor.

+
+  content::HostZoomMap::GetForBrowserContext(
+      web_contents()->GetBrowserContext())->RemoveZoomLevelChangedCallback(
+          zoom_callback_);
+}
 
 bool ZoomController::IsAtDefaultZoom() const {
   if (!web_contents())
@@ -67,11 +75,8 @@ void ZoomController::DidNavigateMainFrame(
   UpdateState(std::string());
 }
 
-void ZoomController::Observe(int type,
-                             const content::NotificationSource& source,
-                             const content::NotificationDetails& details) {
-  DCHECK_EQ(content::NOTIFICATION_ZOOM_LEVEL_CHANGED, type);
-  UpdateState(*content::Details<std::string>(details).ptr());
+void ZoomController::OnZoomLevelChanged(const std::string& host) {
+  UpdateState(host);
 }
 
 void ZoomController::UpdateState(const std::string& host) {