Remove ViewMsg_SetZoomLevel

content/browser/host_zoom_map_impl.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 CONTENT_BROWSER_HOST_ZOOM_MAP_IMPL_H_
 #define CONTENT_BROWSER_HOST_ZOOM_MAP_IMPL_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 28 matching lines @@
       double level) OVERRIDE;
   virtual void SetZoomLevelForHostAndScheme(
       const std::string& scheme,
       const std::string& host,
       double level) OVERRIDE;
   virtual double GetDefaultZoomLevel() const OVERRIDE;
   virtual void SetDefaultZoomLevel(double level) OVERRIDE;
   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.
   //
   // This may be called on any thread.
   double GetTemporaryZoomLevel(int render_process_id,
                                int render_view_id) const;
 
   // Sets the temporary zoom level that's only valid for the lifetime of this
   // WebContents.
@@ skipping 17 matching lines @@
   // Callbacks called when zoom level changes.
   base::CallbackList<void(const ZoomLevelChange&)>
       zoom_level_changed_callbacks_;
 
   // Copy of the pref data, so that we can read it on the IO thread.
   HostZoomLevels host_zoom_levels_;
   SchemeHostZoomLevels scheme_host_zoom_levels_;
   double default_zoom_level_;
 
   struct TemporaryZoomLevel {
     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.
   mutable base::Lock lock_;
 
   NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(HostZoomMapImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_HOST_ZOOM_MAP_IMPL_H_