Tracks changes to zoom icon and zoom percentage. Notifies browser window if the icon should be chan…

chrome/browser/ui/zoom/zoom_tab_helper.h

+// Copyright (c) 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.
+
+#ifndef CHROME_BROWSER_UI_ZOOM_ZOOM_TAB_HELPER_H_
+#define CHROME_BROWSER_UI_ZOOM_ZOOM_TAB_HELPER_H_
+#pragma once
+
+#include "chrome/browser/prefs/pref_member.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/web_contents_observer.h"
+
+class TabContentsWrapper;
+class ZoomTabHelperDelegate;
+
+// Per-tab class to manage the Omnibox zoom icon.
+class ZoomTabHelper : public content::NotificationObserver,

[Ben Goodger (Google), 2012/05/29 18:04:41]

ZoomController

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+                      public content::WebContentsObserver {
+ public:
+  explicit ZoomTabHelper(TabContentsWrapper* tab_contents);
+  virtual ~ZoomTabHelper();
+
+  enum ZoomIconState {

[tfarina, 2012/05/29 18:09:12]

nit: this should goes first in this section (i.e, before the ctor and dtor).

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+    NONE = 0,
+    ZOOM_PLUS_ICON,
+    ZOOM_MINUS_ICON,
+  };
+
+  ZoomIconState zoom_icon_status() const { return zoom_icon_state_; }

[tfarina, 2012/05/29 18:09:12]

nit: zoom_icon_state()

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+  int zoom_percent() const { return zoom_percent_; }
+
+  ZoomTabHelperDelegate* delegate() const { return delegate_; }
+  void set_delegate(ZoomTabHelperDelegate* d) { delegate_ = d; }
+
+  // content::WebContentsObserver overrides:
+  virtual void DidNavigateMainFrame(

[tfarina, 2012/05/29 18:09:12]

I bet you can make this and Observe private.

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+      const content::LoadCommittedDetails& details,
+      const content::FrameNavigateParams& params) OVERRIDE;

[tfarina, 2012/05/29 18:09:12]

include compiler_specific.h for OVERRIDE

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+
+  // content::NotificationObserver overrides:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+ private:
+  // Updates the zoom icon and zoom percentage based on current values and
+  // notifies the delegate if changes have occurred.
+  void UpdateState();
+
+  // The current zoom icon state.
+  ZoomIconState zoom_icon_state_;
+
+  // The current zoom percentage.
+  int zoom_percent_;
+
+  // Registers and unregisters notifications.

[tfarina, 2012/05/29 18:09:12]

nit: I'd remove this comment, it doesn't add much, as the intention of
NotificationRegistrar is pretty obvious.

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+  content::NotificationRegistrar registrar_;
+
+  // Used to access the default zoom level preference.
+  DoublePrefMember default_zoom_level_;
+
+  // Owning TabContentsWrapper.

[tfarina, 2012/05/29 18:09:12]

I guess you don't own TabContentsWrapper.

[Kyle Horimoto, 2012/05/31 23:13:52]

No, it owns this.

[tfarina, 2012/05/31 23:21:00]

You don't own TabContentsWrapper. This is a weak pointer. You don't manage the
lifetime of this pointer, do you? Otherwise this would be TabContentsWrapper
wrapper_; or scoped_ptr<TabContentsWrapper> wrapper_;

[Kyle Horimoto, 2012/05/31 23:37:20]

Not quite sure what you're trying to say here. This TabContentsWrapper owns the
current object (i.e., has an instance of this class as an ivar). It allocates an
instance of this class in its constructor and destroys it in the destructor, so
it is always created before this object is created and destroyed after this
object is destroyed. Don't think we need to worry about the lifetime of the
pointer.

+  TabContentsWrapper* tab_contents_wrapper_;

[Ben Goodger (Google), 2012/05/29 18:04:41]

you don't appear to use this as a TabContentsWrapper, just as a
content::WebContents. So just store as a WebContents.

[Kyle Horimoto, 2012/05/31 23:13:52]

Actually, I do use it as a TabContentsWrapper. It gets passed back to Browser
through the delegate methods to ensure that the proper tab is being updated.

+
+  // Delegate receiving notifications on state changes.
+  ZoomTabHelperDelegate* delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(ZoomTabHelper);

[tfarina, 2012/05/29 18:09:12]

include basictypes.h for this.

[Kyle Horimoto, 2012/05/31 23:13:52]

Done.

+};
+
+#endif  // CHROME_BROWSER_UI_ZOOM_ZOOM_TAB_HELPER_H_