Maintain HostZoom connection per-frame on browser side

HostZoomMapObserver is a per-WebContents class, while the HostZoom Mojo
connection is per-frame. Before this CL, HostZoomMapObserver was
maintaining one HostZoom connection and rebinding it every time a new
RenderFrame was created. This meant that HostZoomMapObserver was
continually losing connections to existing frames. This CL changes
HostZoomMapObserver to maintain one connection per-RenderFrame in a
map indexed by the corresponding RenderFrameHost.

BUG=673065
TEST=Visit news.ycombinator.com and increase the zoom level to 175%.
Click the top link. Hit back: news.ycombinator.com should still be
zoomed to 175%.

Review-Url: https://codereview.chromium.org/2581143002
Cr-Commit-Position: refs/heads/master@{#439800}

content/browser/host_zoom_map_observer.cc

@@ -23,23 +23,36 @@ void HostZoomMapObserver::ReadyToCommitNavigation(
   if (!navigation_handle->IsInMainFrame())
     return;
 
-  DCHECK(host_zoom_.is_bound());
-  if (!host_zoom_.is_bound())
-    return;
-
   RenderFrameHost* render_frame_host =
       navigation_handle->GetRenderFrameHost();
+  const auto& entry = host_zoom_ptrs_.find(render_frame_host);
+  if (entry == host_zoom_ptrs_.end())
+    return;
+
+  const mojom::HostZoomAssociatedPtr& host_zoom = entry->second;
+  DCHECK(host_zoom.is_bound());
+  if (host_zoom.encountered_error())
+    return;
+
   RenderProcessHost* render_process_host = render_frame_host->GetProcess();
   HostZoomMapImpl* host_zoom_map = static_cast<HostZoomMapImpl*>(
       render_process_host->GetStoragePartition()->GetHostZoomMap());
   double zoom_level = host_zoom_map->GetZoomLevelForView(
       navigation_handle->GetURL(), render_process_host->GetID(),
       render_frame_host->GetRenderViewHost()->GetRoutingID());
-  host_zoom_->SetHostZoomLevel(navigation_handle->GetURL(), zoom_level);
+  host_zoom->SetHostZoomLevel(navigation_handle->GetURL(), zoom_level);
 }
 
 void HostZoomMapObserver::RenderFrameCreated(RenderFrameHost* rfh) {
-  rfh->GetRemoteAssociatedInterfaces()->GetInterface(&host_zoom_);
+  mojom::HostZoomAssociatedPtr host_zoom;
+  rfh->GetRemoteAssociatedInterfaces()->GetInterface(&host_zoom);
+  host_zoom_ptrs_[rfh] = std::move(host_zoom);
+}
+
+void HostZoomMapObserver::RenderFrameDeleted(RenderFrameHost* rfh) {
+  const auto& entry = host_zoom_ptrs_.find(rfh);
+  DCHECK(entry != host_zoom_ptrs_.end());
+  host_zoom_ptrs_.erase(entry);
 }
 
 }  // namespace content

content/browser/host_zoom_map_observer.h

@@ -21,8 +21,9 @@ class HostZoomMapObserver : private WebContentsObserver {
   // WebContentsObserver implementation:
   void ReadyToCommitNavigation(NavigationHandle* navigation_handle) override;
   void RenderFrameCreated(RenderFrameHost* rfh) override;
+  void RenderFrameDeleted(RenderFrameHost* rfh) override;
 
-  mojom::HostZoomAssociatedPtr host_zoom_;
+  std::map<RenderFrameHost*, mojom::HostZoomAssociatedPtr> host_zoom_ptrs_;
 };
 
 }  // namespace content

content/browser/iframe_zoom_browsertest.cc

@@ -258,7 +258,7 @@ IN_PROC_BROWSER_TEST_F(IFrameZoomBrowserTest, SubframesDontZoomIndependently) {
 
   // We exclude the remainder of this test on Android since Android does not
   // set page zoom levels for loading pages.
-  // See RenderViewImpl::OnSetZoomLevelForLoadingURL().
+  // See RenderFrameImpl::SetHostZoomLevel().
 #if !defined(OS_ANDROID)
   // When we navigate so that b.com is the top-level site, then it has the
   // expected zoom.
@@ -478,4 +478,71 @@ IN_PROC_BROWSER_TEST_F(IFrameZoomBrowserTest,
 }
 #endif
 
+// Tests that on cross-site navigation from a page that has a subframe, the
+// appropriate zoom is applied to the new page.
+// crbug.com/673065
+// Note: We exclude the this test on Android since Android does not set page
+// zoom levels for loading pages.
+// See RenderFrameImpl::SetHostZoomLevel().
+#if !defined(OS_ANDROID)
+IN_PROC_BROWSER_TEST_F(IFrameZoomBrowserTest,
+                       SubframesDontBreakConnectionToRenderer) {
+  std::string top_level_host("a.com");
+  GURL main_url(embedded_test_server()->GetURL(
+      top_level_host, "/page_with_iframe_and_link.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  NavigationEntry* entry =
+      web_contents()->GetController().GetLastCommittedEntry();
+  ASSERT_TRUE(entry);
+  GURL loaded_url = HostZoomMap::GetURLFromEntry(entry);
+  EXPECT_EQ(top_level_host, loaded_url.host());
+
+  // The following calls must be made when the page's scale factor = 1.0.
+  double main_frame_window_border = GetMainframeWindowBorder(web_contents());
+
+  HostZoomMap* host_zoom_map = HostZoomMap::GetForWebContents(web_contents());
+  double default_zoom_level = host_zoom_map->GetDefaultZoomLevel();
+  EXPECT_EQ(0.0, default_zoom_level);
+  EXPECT_DOUBLE_EQ(
+      1.0, GetMainFrameZoomFactor(web_contents(), main_frame_window_border));
+
+  // Set a zoom for a host that will be navigated to below.
+  const double new_zoom_factor = 2.0;
+  const double new_zoom_level =
+      default_zoom_level + ZoomFactorToZoomLevel(new_zoom_factor);
+  host_zoom_map->SetZoomLevelForHost("foo.com", new_zoom_level);
+
+  // Navigate forward in the same RFH to a site with that host via a
+  // renderer-initiated navigation.
+  {
+    const char kReplacePortNumber[] =
+        "window.domAutomationController.send(setPortNumber(%d));";
+    uint16_t port_number = embedded_test_server()->port();
+    bool success = false;
+    EXPECT_TRUE(ExecuteScriptAndExtractBool(
+        shell(), base::StringPrintf(kReplacePortNumber, port_number),
+        &success));
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url = embedded_test_server()->GetURL("foo.com", "/title2.html");
+    success = false;
+    EXPECT_TRUE(ExecuteScriptAndExtractBool(
+        shell(), "window.domAutomationController.send(clickCrossSiteLink());",
+        &success));
+    EXPECT_TRUE(success);
+    EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  // Check that the requested zoom has been applied to the new site.
+  // NOTE: Local observation on Linux has shown that this comparison has to be
+  // approximate. As the common failure mode would be that the zoom is ~1
+  // instead of ~2, this approximation shouldn't be problematic.
+  EXPECT_NEAR(
+      new_zoom_factor,
+      GetMainFrameZoomFactor(web_contents(), main_frame_window_border),
+      .1);
+}
+#endif
+
 }  // namespace content

content/test/data/page_with_iframe_and_link.html

+<html>
+
+ <head><title>Page with iframe and link</title>
+ <script>
+  function simulateClick(target) {
+    var evt = document.createEvent("MouseEvents");
+    evt.initMouseEvent("click", true, true, window,
+                       0, 0, 0, 0, 0, false, false,
+                       false, false, 0, null);
+
+    return target.dispatchEvent(evt);
+  }
+
+  function setPortNumber(portNumber) {
+    var link = document.getElementById("cross_site_link");
+    link.setAttribute("href", "http://foo.com:" + portNumber + "/title2.html");
+    return true;
+  }
+
+  function clickCrossSiteLink() {
+    return simulateClick(document.getElementById("cross_site_link"));
+  }
+ </script>
+ </head>
+
+<a href="http://foo.com/title2.html" id="cross_site_link">cross-site</a><br>
+
+<p>This page has an iframe. Yay for iframes!
+<p><iframe src="about:blank"></iframe>
+
+</html>