Zoom Extension API (content changes)

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 0ad4fec46bf74ec0482d3b283488c0b0c265028b..bf813bfa3c414d4786dd509270c76a4ee5caf6ac 100644
--- a/content/browser/host_zoom_map_impl.h
+++ b/content/browser/host_zoom_map_impl.h
@@ -35,6 +35,8 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
   virtual double GetZoomLevelForHostAndScheme(
       const std::string& scheme,
       const std::string& host) const OVERRIDE;
+  virtual bool HasZoomLevel(const std::string& scheme,
+                            const std::string& host) const OVERRIDE;
   virtual ZoomLevelVector GetAllZoomLevels() const OVERRIDE;
   virtual void SetZoomLevelForHost(
       const std::string& host,
@@ -43,6 +45,14 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
       const std::string& scheme,
       const std::string& host,
       double level) OVERRIDE;
+  virtual bool UsesTemporaryZoomLevel(int render_process_id,
+                                      int render_view_id) const OVERRIDE;
+  virtual void SetTemporaryZoomLevel(int render_process_id,
+                                     int render_view_id,
+                                     double level) OVERRIDE;
+
+  virtual void ClearTemporaryZoomLevel(int render_process_id,
+                                       int render_view_id) OVERRIDE;
   virtual double GetDefaultZoomLevel() const OVERRIDE;
   virtual void SetDefaultZoomLevel(double level) OVERRIDE;
   virtual scoped_ptr<Subscription> AddZoomLevelChangedCallback(
@@ -66,17 +76,6 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
                            double level,
                            const std::string& host);
 
-  // Returns whether the view manages its zoom level independently of other tabs
-  // displaying content from the same host.
-  bool UsesTemporaryZoomLevel(int render_process_id, int render_view_id) const;
-
-  // Sets whether the view manages its zoom level independently of other tabs
-  // displaying content from the same host, based on whether
-  // |uses_temporary_zoom_level| is true.
-  void SetUsesTemporaryZoomLevel(int render_process_id,
-                                 int render_view_id,
-                                 bool uses_temporary_zoom_level);
-
   // 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.
@@ -85,14 +84,6 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
   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.
-  //
-  // This should only be called on the UI thread.
-  void SetTemporaryZoomLevel(int render_process_id,
-                             int render_view_id,
-                             double level);
-
   // NotificationObserver implementation.
   virtual void Observe(int type,
                        const NotificationSource& source,
@@ -102,20 +93,29 @@ class CONTENT_EXPORT HostZoomMapImpl : public NON_EXPORTED_BASE(HostZoomMap),
   typedef std::map<std::string, double> HostZoomLevels;
   typedef std::map<std::string, HostZoomLevels> SchemeHostZoomLevels;
 
-  struct TemporaryZoomLevel {
-    TemporaryZoomLevel(int process_id, int view_id, double level);
-    TemporaryZoomLevel(int process_id, int view_id);
-    bool operator==(const TemporaryZoomLevel& other) const;
-
+  struct RenderViewKey {
     int render_process_id;
     int render_view_id;
-    double zoom_level;
+    RenderViewKey(int render_process_id, int render_view_id)
+        : render_process_id(render_process_id),
+          render_view_id(render_view_id) {}
+    bool operator<(const RenderViewKey& other) const {
+      return render_process_id < other.render_process_id ||
+             ((render_process_id == other.render_process_id) &&
+              (render_view_id < other.render_view_id));
+    }
   };
 
-  typedef std::vector<TemporaryZoomLevel> TemporaryZoomLevels;
+  typedef std::map<RenderViewKey, double> TemporaryZoomLevels;

[jam, 2014/06/12 21:35:52]

this change is extraneous, why do you need it now? i'm assuming the original
code was written that way because very few pages are zoomed, so a vector is
enough. have you seen a reason that causes this assumption to be made not valid
anymore?

[Fady Samuel, 2014/06/12 21:43:49]

I suggested this change in code review:

1. It makes the code somewhat easier to read as we don't need an iteration of
the vector for every operation on temporary zoom. It also results in fewer lines
of code. See patchset #4 where it used a vector instead: there's a lot more code
and so I felt readability suffered a bit.

2. With the chrome extension zoom API, I would expect we would end up with more
pages than currently that are zoomed via temporary zoom.

I feel point #1 is more significant though. It's more of a readability win than
a performance issue.

[jam, 2014/06/13 15:25:49]

ok. i think this comes down to personal preference, but i dont feel too strongly

 
   double GetZoomLevelForHost(const std::string& host) const;
 
+  // Notifies the renderers from this browser context to change the zoom level
+  // for the specified host and scheme.
+  void SendZoomLevelChange(const std::string& scheme,
+                           const std::string& host,
+                           double level);
+
   // Callbacks called when zoom level changes.
   base::CallbackList<void(const ZoomLevelChange&)>
       zoom_level_changed_callbacks_;