Add scheme to HostZoomMap

content/public/browser/host_zoom_map.h

Index: content/public/browser/host_zoom_map.h
diff --git a/content/public/browser/host_zoom_map.h b/content/public/browser/host_zoom_map.h
index c399f699aec489aa3a1d524aab15b6ab08d120ce..8645559171f7534a969c293fc0fc98f8e9787410 100644
--- a/content/public/browser/host_zoom_map.h
+++ b/content/public/browser/host_zoom_map.h
@@ -21,8 +21,28 @@ class ResourceContext;
 // Maps hostnames to custom zoom levels.  Written on the UI thread and read on
 // any thread.  One instance per browser context. Must be created on the UI
 // thread, and it'll delete itself on the UI thread as well.
+// Zoom can be defined at three levels: default zoom, zoom for host, and zoom
+// for host with specific scheme.
+
 class HostZoomMap {
  public:
+  // Enum that indicates how

[Jói, 2013/03/06 20:43:17]

trailing sentence

[Denis Kuznetsov (DE-MUC), 2013/03/07 15:16:59]

Done.

+  enum ZoomLevelChangeMode {
+    ZOOM_CHANGED_FOR_HOST,           // Zoom level changed for host.
+    ZOOM_CHANGED_FOR_SCHEME_AND_HOST // Zoom level changed for scheme/host pair.
+    ZOOM_CHANGED_GENERIC,            // Some zoom level change happened,

[sky, 2013/03/06 22:21:11]

It is totally unclear what this is, and because of that it isn't clear what a
callback should do. This seems to only be invoked when the temporary zoom level
changes. Can we name this to match: ZOOM_CHANGED_TEMPORARY_ZOOM with a suitable
comment (and indicate host/scheme are not set).

[Denis Kuznetsov (DE-MUC), 2013/03/07 15:16:59]

Done.

+                                     // nothing else is known.
+  };
+
+  // Structure used to notify about zoom changes. Host and Scheme part can be
+  // empty if they do not make sense for particular mode.
+  struct ZoomLevelChange {
+    ZoomLevelChangeMode mode;
+    std::string host;
+    std::string scheme;
+    double zoom_level;
+  };
+
   CONTENT_EXPORT static HostZoomMap* GetForBrowserContext(
       BrowserContext* browser_context);
 
@@ -30,7 +50,9 @@ class HostZoomMap {
   // thread.
   virtual void CopyFrom(HostZoomMap* copy) = 0;
 
-  // Returns the zoom level for the host or spec for a given url. The zoom
+  // Returns the zoom level for the host or spec for a given url, taking
+  // |scheme| in account. If there is specific zoom level specified for given
+  // |scheme| and |host| pair, it will be returned. In other cases the zoom
   // level is determined by the host portion of the URL, or (in the absence of

[Jói, 2013/03/06 20:43:17]

"in the absence of a host" seems a bit unclear to me. Does it mean the absence
of a host in the |host| parameter, or does it mean the absence of a registration
for that particular host, when a registration for the full spec passed as |host|
does exist?

[Denis Kuznetsov (DE-MUC), 2013/03/07 15:16:59]

Done.

   // a host) the complete spec of the URL. In most cases, there is no custom
   // zoom level, and this returns the user's default zoom level.  Otherwise,
@@ -38,20 +60,29 @@ class HostZoomMap {
   // negative (to zoom out).
   //
   // This may be called on any thread.
-  virtual double GetZoomLevel(const std::string& host) const = 0;
+  virtual double GetZoomLevelForHostAndScheme(
+      const std::string& scheme,
+      const std::string& host) const = 0;
 
   // Sets the zoom level for the host or spec for a given url to |level|.  If
   // the level matches the current default zoom level, the host is erased
   // from the saved preferences; otherwise the new value is written out.
   //
   // This should only be called on the UI thread.
-  virtual void SetZoomLevel(const std::string& host, double level) = 0;
+  virtual void SetZoomLevelForHost(const std::string& host, double level) = 0;

[sky, 2013/03/06 22:21:11]

Can you document at the class level what the precedence is for these? By that I
mean things like setting the zoom level for a host does not override the zoom
level for scheme-host.

[Denis Kuznetsov (DE-MUC), 2013/03/07 15:16:59]

Done.

+
+  // Sets the zoom level for the |scheme|/|host| pair to |level|.

[Jói, 2013/03/06 20:43:17]

One point you might clarify in the documentation for this; |host| in this case
can still be a host or a spec, is that right?

[Denis Kuznetsov (DE-MUC), 2013/03/07 15:16:59]

Done.

+  //
+  // This should only be called on the UI thread.
+  virtual void SetZoomLevelForHostAndScheme(const std::string& scheme,
+                                            const std::string& host,
+                                            double level) = 0;
 
   // Get/Set the default zoom level for pages that don't override it.
   virtual double GetDefaultZoomLevel() const = 0;
   virtual void SetDefaultZoomLevel(double level) = 0;;
 
-  typedef base::Callback<void(const std::string&)> ZoomLevelChangedCallback;
+  typedef base::Callback<void(const ZoomLevelChange&)> ZoomLevelChangedCallback;
 
   // Add and remove zoom level changed callbacks.
   virtual void AddZoomLevelChangedCallback(