Page zoom improvements:

1. Change from using zoom levels to a range of preset zoom factors (percentages). Zoom levels are the values that WebKit uses for page zoom. While zoom levels are simple to implement, they result in undesirable percentages like 57%, 144%, 207%, etc. Changing to zoom factors gives us user-friendly zoom percentages.
2. Increase the range of supported zoom values. A user can now zoom between 25% to 500%.
3. Use an epsilon value when comparing zoom floating point values. The previous version was doing an equality comparison of double values which may not always work as expected.

BUG=71484
TEST=Exercise zoom in/out via the wrench menu; exercise default page zoom setting in options window
Review URL: http://codereview.chromium.org/8528011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111087 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 189 insertions, 8 deletions

diff --git a/content/browser/host_zoom_map.cc b/content/browser/host_zoom_map.cc
index b827433..c36aa4e 100644
--- a/content/browser/host_zoom_map.cc
+++ b/content/browser/host_zoom_map.cc
@@ -14,6 +14,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/common/page_zoom.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_util.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
@@ -56,7 +57,8 @@ void HostZoomMap::SetZoomLevel(std::string host, double level) {
 
   {
     base::AutoLock auto_lock(lock_);
-    if (level == default_zoom_level_)
+
+    if (content::ZoomValuesEqual(level, default_zoom_level_))
       host_zoom_levels_.erase(host);
     else
       host_zoom_levels_[host] = level;
diff --git a/content/browser/host_zoom_map_unittest.cc b/content/browser/host_zoom_map_unittest.cc
new file mode 100644
index 0000000..d151362
--- /dev/null
+++ b/content/browser/host_zoom_map_unittest.cc
@@ -0,0 +1,32 @@
+// Copyright (c) 2011 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.h"
+
+#include "base/memory/ref_counted.h"
+#include "base/message_loop.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/test/test_browser_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class HostZoomMapTest : public testing::Test {
+ public:
+  HostZoomMapTest() : ui_thread_(content::BrowserThread::UI, &message_loop_) {
+  }
+
+ protected:
+  MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
+};
+
+TEST_F(HostZoomMapTest, GetSetZoomLevel) {
+  scoped_refptr<HostZoomMap> host_zoom_map = new HostZoomMap;
+
+  double zoomed = 2.5;
+  host_zoom_map->SetZoomLevel("zoomed.com", zoomed);
+
+  EXPECT_DOUBLE_EQ(host_zoom_map->GetZoomLevel("normal.com"), 0);
+  EXPECT_DOUBLE_EQ(host_zoom_map->GetZoomLevel("zoomed.com"), zoomed);
+}
+
diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc
index eb1f934..5fe45a9 100644
--- a/content/browser/tab_contents/tab_contents.cc
+++ b/content/browser/tab_contents/tab_contents.cc
@@ -197,9 +197,9 @@ TabContents::TabContents(content::BrowserContext* browser_context,
       opener_web_ui_type_(WebUI::kNoWebUI),
       closed_by_user_gesture_(false),
       minimum_zoom_percent_(
-          static_cast<int>(WebKit::WebView::minTextSizeMultiplier * 100)),
+          static_cast<int>(content::kMinimumZoomFactor * 100)),
       maximum_zoom_percent_(
-          static_cast<int>(WebKit::WebView::maxTextSizeMultiplier * 100)),
+          static_cast<int>(content::kMaximumZoomFactor * 100)),
       temporary_zoom_settings_(false),
       content_restrictions_(0),
       view_type_(content::VIEW_TYPE_TAB_CONTENTS) {
@@ -862,8 +862,10 @@ double TabContents::GetZoomLevel() const {
 int TabContents::GetZoomPercent(bool* enable_increment,
                                 bool* enable_decrement) {
   *enable_decrement = *enable_increment = false;
+  // Calculate the zoom percent from the factor. Round up to the nearest whole
+  // number.
   int percent = static_cast<int>(
-      WebKit::WebView::zoomLevelToZoomFactor(GetZoomLevel()) * 100);
+      WebKit::WebView::zoomLevelToZoomFactor(GetZoomLevel()) * 100 + 0.5);
   *enable_decrement = percent > minimum_zoom_percent_;
   *enable_increment = percent < maximum_zoom_percent_;
   return percent;
diff --git a/content/browser/webui/web_ui.cc b/content/browser/webui/web_ui.cc
index 5febef8..38e9839 100644
--- a/content/browser/webui/web_ui.cc
+++ b/content/browser/webui/web_ui.cc
@@ -228,6 +228,15 @@ bool WebUIMessageHandler::ExtractIntegerValue(const ListValue* value,
   return false;
 }
 
+bool WebUIMessageHandler::ExtractDoubleValue(const ListValue* value,
+                                             double* out_value) {
+  std::string string_value;
+  if (value->GetString(0, &string_value))
+    return base::StringToDouble(string_value, out_value);
+  NOTREACHED();
+  return false;
+}
+
 string16 WebUIMessageHandler::ExtractStringValue(const ListValue* value) {
   string16 string16_value;
   if (value->GetString(0, &string16_value))
diff --git a/content/browser/webui/web_ui.h b/content/browser/webui/web_ui.h
index 998513f..5cba59f 100644
--- a/content/browser/webui/web_ui.h
+++ b/content/browser/webui/web_ui.h
@@ -219,6 +219,9 @@ class CONTENT_EXPORT WebUIMessageHandler {
   // Extract an integer value from a list Value.
   bool ExtractIntegerValue(const base::ListValue* value, int* out_int);
 
+  // Extract a floating point (double) value from a list Value.
+  bool ExtractDoubleValue(const base::ListValue* value, double* out_value);
+
   // Extract a string value from a list Value.
   string16 ExtractStringValue(const base::ListValue* value);
 
diff --git a/content/browser/webui/web_ui_unittest.cc b/content/browser/webui/web_ui_unittest.cc
new file mode 100644
index 0000000..93e597f
--- /dev/null
+++ b/content/browser/webui/web_ui_unittest.cc
@@ -0,0 +1,74 @@
+// Copyright (c) 2011 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/webui/web_ui.h"
+
+#include "base/string16.h"
+#include "base/values.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class TestWebUIMessageHandler : public WebUIMessageHandler {
+ public:
+  TestWebUIMessageHandler() {}
+  virtual ~TestWebUIMessageHandler() {}
+
+ protected:
+  virtual void RegisterMessages() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestWebUIMessageHandler);
+};
+
+TEST(WebUIMessageHandlerTest, ExtractIntegerValue) {
+  TestWebUIMessageHandler handler;
+
+  ListValue list;
+  int value, zero_value = 0, neg_value = -1234, pos_value = 1234;
+
+  list.Append(Value::CreateIntegerValue(zero_value));
+  EXPECT_TRUE(handler.ExtractIntegerValue(&list, &value));
+  EXPECT_EQ(value, zero_value);
+  list.Clear();
+
+  list.Append(Value::CreateIntegerValue(neg_value));
+  EXPECT_TRUE(handler.ExtractIntegerValue(&list, &value));
+  EXPECT_EQ(value, neg_value);
+  list.Clear();
+
+  list.Append(Value::CreateIntegerValue(pos_value));
+  EXPECT_TRUE(handler.ExtractIntegerValue(&list, &value));
+  EXPECT_EQ(value, pos_value);
+}
+
+TEST(WebUIMessageHandlerTest, ExtractDoubleValue) {
+  TestWebUIMessageHandler handler;
+
+  ListValue list;
+  double value, zero_value = 0.0, neg_value = -1234.5, pos_value = 1234.5;
+
+  list.Append(Value::CreateDoubleValue(zero_value));
+  EXPECT_TRUE(handler.ExtractDoubleValue(&list, &value));
+  EXPECT_EQ(value, zero_value);
+  list.Clear();
+
+  list.Append(Value::CreateDoubleValue(neg_value));
+  EXPECT_TRUE(handler.ExtractDoubleValue(&list, &value));
+  EXPECT_EQ(value, neg_value);
+  list.Clear();
+
+  list.Append(Value::CreateDoubleValue(pos_value));
+  EXPECT_TRUE(handler.ExtractDoubleValue(&list, &value));
+  EXPECT_EQ(value, pos_value);
+}
+
+TEST(WebUIMessageHandlerTest, ExtractStringValue) {
+  TestWebUIMessageHandler handler;
+
+  ListValue list;
+  string16 in_string = "The facts, though interesting, are irrelevant."
+  list.Append(Value::CreateStringValue(string));
+  string16 out_string = handler.ExtractStringValue(&list);
+  EXPECT_STREQ(in_string.c_str(), out_string.c_str());
+}
+
diff --git a/content/common/page_zoom.cc b/content/common/page_zoom.cc
new file mode 100644
index 0000000..d9b6b51
--- /dev/null
+++ b/content/common/page_zoom.cc
@@ -0,0 +1,25 @@
+// Copyright (c) 2011 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 <cmath>
+
+#include "content/public/common/page_zoom.h"
+
+namespace content {
+
+const double kMinimumZoomFactor = 0.25;
+const double kMaximumZoomFactor = 5.0;
+
+bool ZoomValuesEqual(double value_a, double value_b) {
+  // Epsilon value for comparing two floating-point zoom values. We don't use
+  // std::numeric_limits<> because it is too precise for zoom values. Zoom
+  // values lose precision due to factor/level conversions. A value of 0.001
+  // is precise enough for zoom value comparisons.
+  const double epsilon = 0.001;
+
+  return (std::fabs(value_a - value_b) <= epsilon);
+}
+
+}  // namespace content
+
diff --git a/content/common/page_zoom_unittest.cc b/content/common/page_zoom_unittest.cc
new file mode 100644
index 0000000..67dd5d5
--- /dev/null
+++ b/content/common/page_zoom_unittest.cc
@@ -0,0 +1,19 @@
+// Copyright (c) 2011 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/public/common/page_zoom.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(PageZoomTest, ZoomValuesEqual) {
+  // Test two identical values.
+  EXPECT_TRUE(content::ZoomValuesEqual(1.5, 1.5));
+
+  // Test two values that are close enough to be considered equal.
+  EXPECT_TRUE(content::ZoomValuesEqual(1.5, 1.49999999));
+
+  // Test two values that are close, but should not be considered equal.
+  EXPECT_FALSE(content::ZoomValuesEqual(1.5, 1.4));
+}
+
diff --git a/content/content_common.gypi b/content/content_common.gypi
index e42f568..f1f0328 100644
--- a/content/content_common.gypi
+++ b/content/content_common.gypi
@@ -192,6 +192,7 @@
     'common/npobject_util.h',
     'common/p2p_messages.h',
     'common/p2p_sockets.h',
+    'common/page_zoom.cc',
     'common/pepper_file_messages.cc',
     'common/pepper_file_messages.h',
     'common/pepper_messages.h',
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 520654a..e95c636 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -187,6 +187,7 @@
         'browser/geolocation/win7_location_api_unittest_win.cc',
         'browser/geolocation/win7_location_provider_unittest_win.cc',
         'browser/gpu/gpu_blacklist_unittest.cc',
+        'browser/host_zoom_map_unittest.cc',
         'browser/in_process_webkit/dom_storage_unittest.cc',
         'browser/in_process_webkit/indexed_db_quota_client_unittest.cc',
         'browser/in_process_webkit/webkit_context_unittest.cc',
@@ -224,6 +225,7 @@
         'common/gpu/gpu_info_unittest.cc',
         'common/hi_res_timer_manager_unittest.cc',
         'common/net/url_fetcher_impl_unittest.cc',
+        'common/page_zoom_unittest.cc',
         'common/process_watcher_unittest.cc',
         'common/property_bag_unittest.cc',
         'common/resource_dispatcher_unittest.cc',
diff --git a/content/public/common/page_zoom.h b/content/public/common/page_zoom.h
index 956169f..3abe75a 100644
--- a/content/public/common/page_zoom.h
+++ b/content/public/common/page_zoom.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -6,7 +6,7 @@
 #define CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_
 #pragma once
 
-#include "base/basictypes.h"
+#include "content/common/content_export.h"
 
 namespace content {
 
@@ -18,6 +18,18 @@ enum PageZoom {
   PAGE_ZOOM_IN    = 1,
 };
 
+// The minimum zoom factor permitted for a page. This is an alternative to
+// WebView::minTextSizeMultiplier.
+CONTENT_EXPORT extern const double kMinimumZoomFactor;
+
+// The maximum zoom factor permitted for a page. This is an alternative to
+// WebView::maxTextSizeMultiplier.
+CONTENT_EXPORT extern const double kMaximumZoomFactor;
+
+// Test if two zoom values (either zoom factors or zoom levels) should be
+// considered equal.
+CONTENT_EXPORT bool ZoomValuesEqual(double value_a, double value_b);
+
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 7eeaa8d..244bf8b 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1111,8 +1111,8 @@ void RenderViewImpl::UpdateURL(WebFrame* frame) {
     // will also call us back which will cause us to send a message to
     // update TabContents.
     webview()->zoomLimitsChanged(
-        WebView::zoomFactorToZoomLevel(WebView::minTextSizeMultiplier),
-        WebView::zoomFactorToZoomLevel(WebView::maxTextSizeMultiplier));
+        WebView::zoomFactorToZoomLevel(content::kMinimumZoomFactor),
+        WebView::zoomFactorToZoomLevel(content::kMaximumZoomFactor));
 
     // Update contents MIME type for main frame.
     params.contents_mime_type = ds->response().mimeType().utf8();