Fix page zoom to be frame-centric for out-of-process frames.

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 298a66a8fc401fbfd5a3350b0ff125db9ec8d98d..f0d4472c5237d26375c2d87bc995fb8bdbfcf5b9 100644
--- a/content/browser/host_zoom_map_impl.cc
+++ b/content/browser/host_zoom_map_impl.cc
@@ -278,7 +278,59 @@ double HostZoomMapImpl::GetDefaultZoomLevel() const {
 
 void HostZoomMapImpl::SetDefaultZoomLevel(double level) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  if (ZoomValuesEqual(level, default_zoom_level_))
+      return;
+
   default_zoom_level_ = level;
+
+  // First, remove all entries that match the new default zoom level.
+  {
+    base::AutoLock auto_lock(lock_);
+    for (auto it = host_zoom_levels_.begin(); it != host_zoom_levels_.end(); ) {
+      if (ZoomValuesEqual(it->second, default_zoom_level_))
+        it = host_zoom_levels_.erase(it);
+      else
+        it++;
+    }
+  }
+
+  // Second, update zoom levels for all pages that do not have an overriding
+  // entry.
+  for (auto web_contents : WebContentsImpl::GetAllWebContents()) {
+    int render_process_id = web_contents->GetRenderProcessHost()->GetID();
+    int render_view_id = web_contents->GetRenderViewHost()->GetRoutingID();
+
+    // Get the url from the navigation controller directly, as calling
+    // WebContentsImpl::GetLastCommittedURL() may give us a virtual url that
+    // is different than the one stored in the map.
+    GURL url;
+    NavigationEntry* entry =
+        web_contents->GetController().GetLastCommittedEntry();
+    // It is possible for a WebContent's zoom level to be queried before
+    // a navigation has occurred.
+    if (entry)
+      url = GetURLFromEntry(entry);
+
+    bool uses_default_zoom =
+        !HasZoomLevel(url.scheme(), net::GetHostOrSpecFromURL(url)) &&

[ncarter (slow), 2016/04/11 22:17:03]

|url| may be empty here per the above comment. In that case GetHostOrSpecFromURL
would DCHECK before returning an empty string.

How hard would it be to write a test to exercise this case?

[wjmaclean, 2016/04/13 18:47:47]

Perhaps just easier to code it so that doesn't matter?

+        !UsesTemporaryZoomLevel(render_process_id, render_view_id);
+
+    // Only change zoom for WebContents tied to the StoragePartition this
+    // HostZoomMap serves.
+    if (GetForWebContents(web_contents) == this && uses_default_zoom) {

[ncarter (slow), 2016/04/11 22:17:03]

optional suggestion: consider moving the "GetForWebContents(web_contents) ==
this" filtering up to the top of the loop:

if (GetForWebContents(web_contents) != this)
  continue;

[wjmaclean, 2016/04/13 18:47:47]

Done.

+      web_contents->UpdateZoom(level);
+
+      HostZoomMap::ZoomLevelChange change;
+      change.mode = HostZoomMap::ZOOM_CHANGED_FOR_HOST;
+      change.host =
+          entry ? net::GetHostOrSpecFromURL(HostZoomMap::GetURLFromEntry(entry))

[ncarter (slow), 2016/04/11 22:17:03]

Seems like this is already computed, as |url|.

[wjmaclean, 2016/04/13 18:47:47]

Done.

+                : std::string();
+      change.zoom_level = level;
+
+      zoom_level_changed_callbacks_.Notify(change);
+    }
+  }
 }
 
 scoped_ptr<HostZoomMap::Subscription>
@@ -405,9 +457,10 @@ void HostZoomMapImpl::SetTemporaryZoomLevel(int render_process_id,
     temporary_zoom_levels_[key] = level;
   }
 
-  RenderViewHost* host =
-      RenderViewHost::FromID(render_process_id, render_view_id);
-  host->Send(new ViewMsg_SetZoomLevelForView(render_view_id, true, level));
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(WebContents::FromRenderViewHost(
+          RenderViewHost::FromID(render_process_id, render_view_id)));
+  web_contents->SetTemporaryZoomLevel(level, true);
 
   HostZoomMap::ZoomLevelChange change;
   change.mode = HostZoomMap::ZOOM_CHANGED_TEMPORARY_ZOOM;
@@ -457,29 +510,29 @@ void HostZoomMapImpl::ClearTemporaryZoomLevel(int render_process_id,
       return;
     temporary_zoom_levels_.erase(it);
   }
-  RenderViewHost* host =
-      RenderViewHost::FromID(render_process_id, render_view_id);
-  DCHECK(host);
-  // Send a new zoom level, host-specific if one exists.
-  host->Send(new ViewMsg_SetZoomLevelForView(
-      render_view_id,
-      false,
-      GetZoomLevelForHost(
-          GetHostFromProcessView(render_process_id, render_view_id))));
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(WebContents::FromRenderViewHost(
+          RenderViewHost::FromID(render_process_id, render_view_id)));
+  web_contents->SetTemporaryZoomLevel(GetZoomLevelForHost(
+          GetHostFromProcessView(render_process_id, render_view_id)), false);
 }
 
 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();
-    // TODO(wjmaclean) This will need to be cleaned up when
-    // RenderProcessHost::GetStoragePartition() goes away. Perhaps have
-    // RenderProcessHost expose a GetHostZoomMap() function?
-    if (render_process_host->GetStoragePartition()->GetHostZoomMap() == this) {
-      render_process_host->Send(
-          new ViewMsg_SetZoomLevelForCurrentURL(scheme, host, level));
+  // We'll only send to WebContents not using temporary zoom levels. The one
+  // other case of interest is where the renderer is hosting a plugin document;
+  // that should be reflected in our temporary zoom level map, but we will
+  // double check on the renderer side to avoid the possibility of any races.
+  for (auto web_contents : WebContentsImpl::GetAllWebContents()) {
+    // Only send zoom level changes to WebContents that are using this
+    // HostZoomMap.
+    int render_process_id = web_contents->GetRenderProcessHost()->GetID();
+    int render_view_id = web_contents->GetRenderViewHost()->GetRoutingID();
+
+    if (GetForWebContents(web_contents) == this &&

[ncarter (slow), 2016/04/11 22:17:03]

If you update the other loop to use a 'continue:', update this too.

[wjmaclean, 2016/04/13 18:47:47]

Done.

+        !UsesTemporaryZoomLevel(render_process_id, render_view_id)) {
+      web_contents->UpdateZoomIfNecessary(scheme, host, level);
     }
   }
 }