Zoom Extension API (content changes)

content/browser/host_zoom_map_impl.cc

Index: content/browser/host_zoom_map_impl.cc
diff --git a/content/browser/host_zoom_map_impl.cc b/content/browser/host_zoom_map_impl.cc
index a0a68dad95a2c860e42b0bb9dc3f519882a6a8b2..41fa7b453297ccc7f55d36ace16adfc28a9ec6b3 100644
--- a/content/browser/host_zoom_map_impl.cc
+++ b/content/browser/host_zoom_map_impl.cc
@@ -87,6 +87,22 @@ double HostZoomMapImpl::GetZoomLevelForHost(const std::string& host) const {
   return (i == host_zoom_levels_.end()) ? default_zoom_level_ : i->second;
 }
 
+bool HostZoomMapImpl::HasZoomLevel(const std::string& scheme,
+                                   const std::string& host) const {
+  base::AutoLock auto_lock(lock_);
+
+  SchemeHostZoomLevels::const_iterator scheme_iterator(
+      scheme_host_zoom_levels_.find(scheme));
+
+  const HostZoomLevels& zoom_levels =
+      (scheme_iterator != scheme_host_zoom_levels_.end())
+          ? scheme_iterator->second
+          : host_zoom_levels_;
+
+  HostZoomLevels::const_iterator i(zoom_levels.find(host));
+  return i != zoom_levels.end();
+}
+
 double HostZoomMapImpl::GetZoomLevelForHostAndScheme(
     const std::string& scheme,
     const std::string& host) const {
@@ -152,16 +168,8 @@ void HostZoomMapImpl::SetZoomLevelForHost(const std::string& host,
       host_zoom_levels_[host] = level;
   }
 
-  // Notify renderers from this browser context.
-  for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    RenderProcessHost* render_process_host = i.GetCurrentValue();
-    if (HostZoomMap::GetForBrowserContext(
-            render_process_host->GetBrowserContext()) == this) {
-      render_process_host->Send(
-          new ViewMsg_SetZoomLevelForCurrentURL(std::string(), host, level));
-    }
-  }
+  SendZoomLevelChange(std::string(), host, level);
+
   HostZoomMap::ZoomLevelChange change;
   change.mode = HostZoomMap::ZOOM_CHANGED_FOR_HOST;
   change.host = host;
@@ -179,16 +187,7 @@ void HostZoomMapImpl::SetZoomLevelForHostAndScheme(const std::string& scheme,
     scheme_host_zoom_levels_[scheme][host] = level;
   }
 
-  // Notify renderers from this browser context.
-  for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    RenderProcessHost* render_process_host = i.GetCurrentValue();
-    if (HostZoomMap::GetForBrowserContext(
-            render_process_host->GetBrowserContext()) == this) {
-      render_process_host->Send(
-          new ViewMsg_SetZoomLevelForCurrentURL(scheme, host, level));
-    }
-  }
+  SendZoomLevelChange(scheme, host, level);
 
   HostZoomMap::ZoomLevelChange change;
   change.mode = HostZoomMap::ZOOM_CHANGED_FOR_SCHEME_AND_HOST;
@@ -240,18 +239,20 @@ void HostZoomMapImpl::SetZoomLevelForWebContents(
     double level) {
   int render_process_id = web_contents_impl.GetRenderProcessHost()->GetID();
   int render_view_id = web_contents_impl.GetRenderViewHost()->GetRoutingID();
+  // Get the url from the navigation controller directly, as calling
+  // WebContentsImpl::GetLastCommittedURL() may give us a virtual url that
+  // is different than what the render view is using. If the two don't match,
+  // the attempt to set the zoom will fail.
+  NavigationEntry* entry =

[Fady Samuel, 2014/06/10 20:05:13]

It looks like this is only used in the else clause. Move this down to the else
clause.

[wjmaclean, 2014/06/10 20:18:54]

Done.

+      web_contents_impl.GetController().GetLastCommittedEntry();
+  DCHECK(entry);
+  GURL url = entry->GetURL();
   if (UsesTemporaryZoomLevel(render_process_id, render_view_id)) {
-    SetTemporaryZoomLevel(render_process_id, render_view_id, level);
+    SetTemporaryZoomLevel(render_process_id,
+                          render_view_id,
+                          level);
+    SendTemporaryZoomLevelChange(render_process_id, render_view_id);
   } else {
-    // Get the url from the navigation controller directly, as calling
-    // WebContentsImpl::GetLastCommittedURL() may give us a virtual url that
-    // is different than what the render view is using. If the two don't match,
-    // the attempt to set the zoom will fail.
-    GURL url;
-    NavigationEntry* entry =
-        web_contents_impl.GetController().GetLastCommittedEntry();
-    DCHECK(entry);
-    url = entry->GetURL();
     SetZoomLevelForHost(net::GetHostOrSpecFromURL(url), level);
   }
 }
@@ -260,10 +261,12 @@ void HostZoomMapImpl::SetZoomLevelForView(int render_process_id,
                                           int render_view_id,
                                           double level,
                                           const std::string& host) {
-  if (UsesTemporaryZoomLevel(render_process_id, render_view_id))
+  if (UsesTemporaryZoomLevel(render_process_id, render_view_id)) {
     SetTemporaryZoomLevel(render_process_id, render_view_id, level);
-  else
+    SendTemporaryZoomLevelChange(render_process_id, render_view_id);

[Fady Samuel, 2014/06/10 20:05:13]

This looks like the only call site for SetZoomLevelForView:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

+  } else {
     SetZoomLevelForHost(host, level);
+  }
 }
 
 bool HostZoomMapImpl::UsesTemporaryZoomLevel(int render_process_id,
@@ -309,6 +312,27 @@ double HostZoomMapImpl::GetTemporaryZoomLevel(int render_process_id,
   return 0;
 }
 
+namespace {
+
+std::string GetHostFromProcessView(int render_process_id, int render_view_id) {

[Fady Samuel, 2014/06/10 20:05:13]

DCHECK to make sure this is only called on the UI thread.

[wjmaclean, 2014/06/12 17:40:09]

Done.

+  RenderViewHost* render_view_host =
+      RenderViewHost::FromID(render_process_id, render_view_id);
+  if (!render_view_host)
+    return std::string();
+
+  WebContents* web_contents = WebContents::FromRenderViewHost(render_view_host);
+  DCHECK(web_contents);
+
+  NavigationEntry* entry =
+      web_contents->GetController().GetLastCommittedEntry();
+  if (!entry)
+    return std::string();
+
+  return net::GetHostOrSpecFromURL(entry->GetURL());
+}
+
+}  // namespace
+
 void HostZoomMapImpl::SetTemporaryZoomLevel(int render_process_id,
                                             int render_view_id,
                                             double level) {
@@ -320,23 +344,24 @@ void HostZoomMapImpl::SetTemporaryZoomLevel(int render_process_id,
     for (i = 0; i < temporary_zoom_levels_.size(); ++i) {
       if (temporary_zoom_levels_[i].render_process_id == render_process_id &&
           temporary_zoom_levels_[i].render_view_id == render_view_id) {
-        if (level) {
-          temporary_zoom_levels_[i].zoom_level = level;
-        } else {
-          temporary_zoom_levels_.erase(temporary_zoom_levels_.begin() + i);
-        }
+        temporary_zoom_levels_[i].zoom_level = level;

[Fady Samuel, 2014/06/10 20:05:13]

This becomes trivial once it's a map.

[wjmaclean, 2014/06/10 20:18:54]

Done.

         break;
       }
     }
 
-    if (level && i == temporary_zoom_levels_.size()) {
+    if (i == temporary_zoom_levels_.size()) {
       TemporaryZoomLevel temp(render_process_id, render_view_id, level);
       temporary_zoom_levels_.push_back(temp);
+      RenderViewHost* host =
+          RenderViewHost::FromID(render_process_id, render_view_id);
+      DCHECK(host);
+      host->Send(new ViewMsg_SetUsesTemporaryZoomLevel(render_view_id, true));

[Fady Samuel, 2014/06/10 20:05:13]

ViewMsg_SetTemporaryZoomLevel(render_view_id, true, level);

[wjmaclean, 2014/06/10 20:18:54]

But this will force us to send a level value for manual mode, where it is
irrelevant. I don't think that makes sense.

[wjmaclean, 2014/06/12 17:40:09]

Now that content will have no knowledge of manual mode, we'll merge these.

     }
   }
 
   HostZoomMap::ZoomLevelChange change;
   change.mode = HostZoomMap::ZOOM_CHANGED_TEMPORARY_ZOOM;
+  change.host = GetHostFromProcessView(render_process_id, render_view_id);
   change.zoom_level = level;
 
   zoom_level_changed_callbacks_.Notify(change);
@@ -347,18 +372,10 @@ void HostZoomMapImpl::Observe(int type,
                               const NotificationDetails& details) {
   switch (type) {
     case NOTIFICATION_RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW: {
-      base::AutoLock auto_lock(lock_);
       int render_view_id = Source<RenderViewHost>(source)->GetRoutingID();
       int render_process_id =
           Source<RenderViewHost>(source)->GetProcess()->GetID();
-
-      for (size_t i = 0; i < temporary_zoom_levels_.size(); ++i) {
-        if (temporary_zoom_levels_[i].render_process_id == render_process_id &&
-            temporary_zoom_levels_[i].render_view_id == render_view_id) {
-          temporary_zoom_levels_.erase(temporary_zoom_levels_.begin() + i);
-          break;
-        }
-      }
+      ClearTemporaryZoomLevel(render_process_id, render_view_id);
       break;
     }
     default:
@@ -366,6 +383,62 @@ void HostZoomMapImpl::Observe(int type,
   }
 }
 
+void HostZoomMapImpl::ClearTemporaryZoomLevel(int render_process_id,
+                                              int render_view_id) {
+  base::AutoLock auto_lock(lock_);
+  for (size_t i = 0; i < temporary_zoom_levels_.size(); ++i) {
+    if (temporary_zoom_levels_[i].render_process_id == render_process_id &&
+        temporary_zoom_levels_[i].render_view_id == render_view_id) {

[Fady Samuel, 2014/06/10 20:05:13]

With the map, the above becomes trivial.

[wjmaclean, 2014/06/10 20:18:54]

Done.

+      // TODO(wjmaclean) Do we need to lookup the appropriate per-origin
+      // zoom level (if any) and send it to the render view?
+      temporary_zoom_levels_.erase(temporary_zoom_levels_.begin() + i);
+      RenderViewHost* host =
+          RenderViewHost::FromID(render_process_id, render_view_id);
+      DCHECK(host);
+      host->Send(new ViewMsg_SetUsesTemporaryZoomLevel(render_view_id, false));

[Fady Samuel, 2014/06/10 20:05:13]

ViewMsg_SetTemporaryZoomLevel(render_view_id, false, /* grab the host zoom level
here to reset behavior */);

+      break;
+    }
+  }
+}
+
+void HostZoomMapImpl::SendTemporaryZoomLevelChange(int render_process_id,

[Fady Samuel, 2014/06/10 20:05:13]

I wish we didn't have to expose this publicly. Ideally manual would be another
bit we set in the renderer so that the behavior sticks for the lifetime of the
renderer. Ignore this comment for now, but we can get John's feedback here.

[wjmaclean, 2014/06/10 20:18:54]

Done.

+                                                   int render_view_id) const {
+  double level = 0.0;
+  bool found = false;
+  {
+    base::AutoLock auto_lock(lock_);
+    for (size_t i = 0; i < temporary_zoom_levels_.size(); ++i) {
+      if (temporary_zoom_levels_[i].render_process_id == render_process_id &&
+          temporary_zoom_levels_[i].render_view_id == render_view_id) {
+        level = temporary_zoom_levels_[i].zoom_level;
+        found = true;
+        break;
+      }
+    }
+  }

[Fady Samuel, 2014/06/10 20:05:13]

With the map, the above becomes trivial.

[wjmaclean, 2014/06/10 20:18:54]

Done.

+  if (!found)
+    return;
+
+  RenderViewHost* host =
+      RenderViewHost::FromID(render_process_id, render_view_id);
+  DCHECK(host);
+  host->Send(new ViewMsg_SetZoomLevelForView(render_view_id, level));

[Fady Samuel, 2014/06/10 20:05:13]

ViewMsg_SetTemporaryZoomLevel(render_view_id, true, level);

[wjmaclean, 2014/06/10 20:18:54]

See my previous comment about combining these.

+}
+
+void HostZoomMapImpl::SendZoomLevelChange(const std::string& scheme,
+                                          const std::string& host,
+                                          double level) {
+  for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
+       !i.IsAtEnd(); i.Advance()) {
+    RenderProcessHost* render_process_host = i.GetCurrentValue();
+    if (HostZoomMap::GetForBrowserContext(
+            render_process_host->GetBrowserContext()) == this) {
+      render_process_host->Send(
+          new ViewMsg_SetZoomLevelForCurrentURL(scheme, host, level));
+    }
+  }
+}
+
 HostZoomMapImpl::~HostZoomMapImpl() {
 }