Remove ViewMsg_SetZoomLevel

content/browser/host_zoom_map_impl.h

Index: content/browser/host_zoom_map_impl.h
diff --git a/content/browser/host_zoom_map_impl.h b/content/browser/host_zoom_map_impl.h
index 155f18e00fb2babd091a352d5df7b2b6974982ce..096e64de674594da3a84a122a308d5c7b9fa4547 100644
--- a/content/browser/host_zoom_map_impl.h
+++ b/content/browser/host_zoom_map_impl.h
@@ -46,6 +46,35 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
   virtual scoped_ptr<Subscription> AddZoomLevelChangedCallback(
       const ZoomLevelChangedCallback& callback) OVERRIDE;
 
+  // Returns the current zoom level for the specified WebContents. This may
+  // be a temporary zoom level, depending on UsesTemporarySoomLevel().

[Peter Kasting, 2014/05/28 22:49:58]

Nit: Soom -> Zoom

[wjmaclean, 2014/05/29 14:49:45]

Done.

+  double GetZoomLevelForWebContents(
+      const WebContents& web_contents) const;
+
+  // Sets the zoom level for this WebContents. If this WebContents is using
+  // a temporary zoom level, then level is only applied to this WebContents.
+  // Otherwise, the level will be applied on a host level.
+  void SetZoomLevelForWebContents(const WebContents& web_contents,
+                                  double level);
+
+  // Sets the zoom level for the specified view. The level may be set for only
+  // this view, or for the host, depending on UsesTemporaryZoomLevel().
+  void SetZoomLevelForView(int render_process_id,
+                           int render_view_id,
+                           double level,
+                           const std::string& host);
+
+  // Returns true if the view specified has its zoom level managed separately
+  // from the host specification.

[Peter Kasting, 2014/05/28 22:49:58]

Nit: How about:

Returns whether the view manages its zoom level independently of other tabs
displaying content from the same host.

[wjmaclean, 2014/05/29 14:49:45]

Sure, sounds good.

+  bool UsesTemporaryZoomLevel(int render_process_id, int render_view_id) const;
+
+  // Causes the view specified to have its zoom level set independently of other
+  // tabs displaying content from the same host, if uses_temporary_zoom_settings
+  // is true.

[Peter Kasting, 2014/05/28 22:49:58]

Nit: How about:

Sets whether the view manages its zoom level independently of other tabs
displaying content from the same host, based on whether
|uses_temporary_zoom_settings| is true.

[wjmaclean, 2014/05/29 14:49:45]

Sure, good wording!

+  void SetUsesTemporaryZoomLevel(int render_process_id,
+                                 int render_view_id,
+                                 bool uses_temporary_zoom_settings);

[Peter Kasting, 2014/05/28 22:49:58]

Nit: I'd call this arg "...level" and not "...settings", for consistency with
the function names, and similarly for all other places in .h and .cc files that
use this name

[wjmaclean, 2014/05/29 14:49:45]

Done.

+
   // Returns the temporary zoom level that's only valid for the lifetime of
   // the given WebContents (i.e. isn't saved and doesn't affect other
   // WebContentses) if it exists, the default zoom level otherwise.
@@ -86,11 +115,27 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
     int render_process_id;

[Peter Kasting, 2014/05/28 22:49:58]

Nit: Members below functions

[wjmaclean, 2014/05/29 14:49:45]

Done.

     int render_view_id;
     double zoom_level;
+
+    TemporaryZoomLevel(int process_id, int view_id, double level)
+        : render_process_id(process_id),
+          render_view_id(view_id),
+          zoom_level(level) {}

[Peter Kasting, 2014/05/28 22:49:58]

Nit: Since you have a .cc file anyway, I'd just move these definitions there
(based on the Chromium "don't inline unless you need to" rule)

[wjmaclean, 2014/05/29 14:49:45]

Done.

+
+    TemporaryZoomLevel(int process_id, int view_id)
+        : render_process_id(process_id),
+          render_view_id(view_id),
+          zoom_level(0.0) {}
+
+    bool operator==(const TemporaryZoomLevel& other) const {
+      return other.render_process_id == render_process_id &&
+             other.render_view_id == render_view_id;
+    }
   };
 
   // Don't expect more than a couple of tabs that are using a temporary zoom
   // level, so vector is fine for now.
-  std::vector<TemporaryZoomLevel> temporary_zoom_levels_;
+  typedef std::vector<TemporaryZoomLevel> TemporaryZoomLevelList;

[Peter Kasting, 2014/05/28 22:49:58]

Don't put "list" in the name, especially when the underlying type is not a list.

We would normally call this TemporaryZoomLevels.

[wjmaclean, 2014/05/29 14:49:45]

Done.

+  TemporaryZoomLevelList temporary_zoom_levels_;
 
   // Used around accesses to |host_zoom_levels_|, |default_zoom_level_| and
   // |temporary_zoom_levels_| to guarantee thread safety.