Check temporary zoom status when sending zoom level in navigation.

content/browser/host_zoom_map_content_browsertest.cc

Index: content/browser/host_zoom_map_content_browsertest.cc
diff --git a/content/browser/host_zoom_map_content_browsertest.cc b/content/browser/host_zoom_map_content_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9c725c6ed2e03c34d4f77ce79c06029e5ee18588
--- /dev/null
+++ b/content/browser/host_zoom_map_content_browsertest.cc
@@ -0,0 +1,63 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/host_zoom_map_impl.h"
+
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/shell/browser/shell.h"
+#include "url/gurl.h"
+
+namespace content {
+
+class HostZoomMapContentBrowserTest : public ContentBrowserTest {

[Charlie Reis, 2015/02/12 21:51:41]

nit: We should probably name this file host_zoom_map_impl_browsertest.cc,
similar to host_zoom_map_impl_unittest.cc.

[wjmaclean, 2015/02/13 15:57:53]

Done.

+};
+
+void RunTestForURL(const GURL& url,
+                   Shell* shell,
+                   double zoom_level_1,
+                   double zoom_level_2) {
+  shell->LoadURL(url);
+  WebContents* web_contents = shell->web_contents();
+
+  HostZoomMapImpl* host_zoom_map = static_cast<HostZoomMapImpl*>(
+      HostZoomMap::GetForWebContents(web_contents));
+
+  int view_id = web_contents->GetRoutingID();
+  int render_process_id = web_contents->GetRenderProcessHost()->GetID();
+
+  EXPECT_DOUBLE_EQ(zoom_level_1, host_zoom_map->GetZoomLevelForView(
+                                     url, render_process_id, view_id));
+
+  host_zoom_map->SetTemporaryZoomLevel(render_process_id, view_id,
+                                       zoom_level_2);
+  EXPECT_DOUBLE_EQ(zoom_level_2, host_zoom_map->GetZoomLevelForView(
+                                     url, render_process_id, view_id));
+  // Clear the temporary zoom level in case subsequent test calls use the same
+  // web_contents.
+  host_zoom_map->ClearTemporaryZoomLevel(render_process_id, view_id);
+}
+
+IN_PROC_BROWSER_TEST_F(HostZoomMapContentBrowserTest, GetZoomForView) {

[Charlie Reis, 2015/02/12 21:51:41]

This test does seem to be covering a lot of the interesting cases, but it's
pretty cryptic to read without stepping through it line-by-line.  Tests are a
form of documentation for how we expect the code to behave, so I think it's
worth restructuring this to make it easier to read.

I'd suggest things like:
 - Splitting it into two tests (one for SetZoomLevelForHost, one for
SZLFHostAndScheme)
 - Renaming zoom_level_2 to temp_zoom_level (and maybe zoom_level_1 to
host_zoom_level)
 - Adding test-level comments (etc) to guide the reader

[wjmaclean, 2015/02/13 15:57:53]

Done.

+  GURL url1("http://abc.com");
+  GURL url2("http://def.com");
+
+  HostZoomMap* host_zoom_map =
+      HostZoomMap::GetForWebContents(shell()->web_contents());
+
+  double default_zoom_level = host_zoom_map->GetDefaultZoomLevel();
+  double zoom_level_1 = default_zoom_level + 1.0;
+  double zoom_level_2 = default_zoom_level + 2.0;
+
+  host_zoom_map->SetZoomLevelForHost(url1.host(), zoom_level_1);
+  host_zoom_map->SetZoomLevelForHostAndScheme(url2.scheme(), url2.host(),
+                                             zoom_level_1);
+
+  RunTestForURL(url1, shell(), zoom_level_1, zoom_level_2);
+  RunTestForURL(url2, shell(), zoom_level_1, zoom_level_2);
+}
+
+}  // namespace content