Factor out OriginToString into a common function in chrome/common/.

With that, notifications doesn't depend on geolocation code anymore.

(NOTE: this was a TODO for bulach).

BUG=None
TEST=out/Debug/unit_tests --gtest_filter=ContentSettingsHelperTest.*

Signed-off-by: Thiago Farina <tfarina@chromium.org>

Review URL: http://codereview.chromium.org/2910005

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

10 files changed, 109 insertions, 43 deletions

diff --git a/chrome/browser/geolocation/geolocation_content_settings_map.cc b/chrome/browser/geolocation/geolocation_content_settings_map.cc
index da9739d..2723870 100644
--- a/chrome/browser/geolocation/geolocation_content_settings_map.cc
+++ b/chrome/browser/geolocation/geolocation_content_settings_map.cc
@@ -43,15 +43,6 @@ void GeolocationContentSettingsMap::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterDictionaryPref(prefs::kGeolocationContentSettings);
 }
 
-// static
-std::string GeolocationContentSettingsMap::OriginToString(const GURL& origin) {
-  std::string port_component((origin.IntPort() != url_parse::PORT_UNSPECIFIED) ?
-      ":" + origin.port() : "");
-  std::string scheme_component(!origin.SchemeIs(chrome::kHttpScheme) ?
-      origin.scheme() + chrome::kStandardSchemeSeparator : "");
-  return scheme_component + origin.host() + port_component;
-}
-
 ContentSetting GeolocationContentSettingsMap::GetDefaultContentSetting() const {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   const PrefService* prefs = profile_->GetPrefs();
diff --git a/chrome/browser/geolocation/geolocation_content_settings_map.h b/chrome/browser/geolocation/geolocation_content_settings_map.h
index a5512de..05dd1a6 100644
--- a/chrome/browser/geolocation/geolocation_content_settings_map.h
+++ b/chrome/browser/geolocation/geolocation_content_settings_map.h
@@ -34,10 +34,6 @@ class GeolocationContentSettingsMap
 
   static void RegisterUserPrefs(PrefService* prefs);
 
-  // Return simplified string representing origin.  If origin is using http or
-  // the standard port, those parts are not included in the output.
-  static std::string OriginToString(const GURL& origin);
-
   // Returns the default setting.
   //
   // This should only be called on the UI thread.
diff --git a/chrome/browser/geolocation/geolocation_exceptions_table_model.cc b/chrome/browser/geolocation/geolocation_exceptions_table_model.cc
index b921ff1..2494465 100644
--- a/chrome/browser/geolocation/geolocation_exceptions_table_model.cc
+++ b/chrome/browser/geolocation/geolocation_exceptions_table_model.cc
@@ -8,6 +8,7 @@
 #include "app/l10n_util_collator.h"
 #include "app/table_model_observer.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/common/content_settings_helper.h"
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
 
@@ -122,12 +123,11 @@ int GeolocationExceptionsTableModel::RowCount() {
 }
 
 std::wstring GeolocationExceptionsTableModel::GetText(int row,
-                                                           int column_id) {
+                                                      int column_id) {
   const Entry& entry = entries_[row];
   if (column_id == IDS_EXCEPTIONS_HOSTNAME_HEADER) {
     if (entry.origin == entry.embedding_origin) {
-      return UTF8ToWide(GeolocationContentSettingsMap::OriginToString(
-          entry.origin));
+      return content_settings_helper::OriginToWString(entry.origin);
     }
     std::wstring indent(L"    ");
     if (entry.embedding_origin.is_empty()) {
@@ -141,8 +141,7 @@ std::wstring GeolocationExceptionsTableModel::GetText(int row,
     }
     return indent + l10n_util::GetStringF(
         IDS_EXCEPTIONS_GEOLOCATION_EMBEDDED_ON_HOST,
-        UTF8ToWide(GeolocationContentSettingsMap::OriginToString(
-            entry.embedding_origin)));
+        content_settings_helper::OriginToWString(entry.embedding_origin));
   }
 
   if (column_id == IDS_EXCEPTIONS_ACTION_HEADER) {
@@ -170,8 +169,8 @@ void GeolocationExceptionsTableModel::SetObserver(
 }
 
 int GeolocationExceptionsTableModel::CompareValues(int row1,
-                                                        int row2,
-                                                        int column_id) {
+                                                   int row2,
+                                                   int column_id) {
   DCHECK(row1 >= 0 && row1 < RowCount() &&
          row2 >= 0 && row2 < RowCount());
 
@@ -239,9 +238,9 @@ void GeolocationExceptionsTableModel::AddEntriesForOrigin(
   }
 }
 
-GeolocationExceptionsTableModel::Entry::Entry(
-    const GURL& in_origin, const GURL& in_embedding_origin,
-    ContentSetting in_setting)
+GeolocationExceptionsTableModel::Entry::Entry(const GURL& in_origin,
+                                              const GURL& in_embedding_origin,
+                                              ContentSetting in_setting)
     : origin(in_origin),
       embedding_origin(in_embedding_origin),
       setting(in_setting) {
diff --git a/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc b/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc
index d6d1f5e..a20bc13 100644
--- a/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc
+++ b/chrome/browser/geolocation/geolocation_exceptions_table_model_unittest.cc
@@ -5,29 +5,23 @@
 #include "chrome/browser/geolocation/geolocation_exceptions_table_model.h"
 
 #include "chrome/browser/renderer_host/test/test_render_view_host.h"
+#include "chrome/common/content_settings_helper.h"
 #include "chrome/test/testing_profile.h"
 #include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
-std::wstring OriginToWString(const GURL& origin) {
-  return UTF8ToWide(GeolocationContentSettingsMap::OriginToString(origin));
-}
 const GURL kUrl0("http://www.example.com");
 const GURL kUrl1("http://www.example1.com");
 const GURL kUrl2("http://www.example2.com");
 }  // namespace
 
-class GeolocationExceptionsTableModelTest
-  : public RenderViewHostTestHarness {
+class GeolocationExceptionsTableModelTest : public RenderViewHostTestHarness {
  public:
   GeolocationExceptionsTableModelTest()
-     : ui_thread_(ChromeThread::UI, MessageLoop::current()) {
-
-  }
+      : ui_thread_(ChromeThread::UI, MessageLoop::current()) {}
 
-  virtual ~GeolocationExceptionsTableModelTest() {
-  }
+  virtual ~GeolocationExceptionsTableModelTest() {}
 
   virtual void SetUp() {
     RenderViewHostTestHarness::SetUp();
@@ -133,14 +127,16 @@ TEST_F(GeolocationExceptionsTableModelTest, GetText) {
 
   // Ensure the parent doesn't have any indentation.
   std::wstring text(model_->GetText(0, IDS_EXCEPTIONS_HOSTNAME_HEADER));
-  EXPECT_EQ(OriginToWString(kUrl0), text);
+  EXPECT_EQ(content_settings_helper::OriginToWString(kUrl0), text);
 
   // Ensure there's some indentation on the children nodes.
   text = model_->GetText(1, IDS_EXCEPTIONS_HOSTNAME_HEADER);
-  EXPECT_NE(OriginToWString(kUrl1), text);
-  EXPECT_NE(std::wstring::npos, text.find(OriginToWString(kUrl1)));
+  EXPECT_NE(content_settings_helper::OriginToWString(kUrl1), text);
+  EXPECT_NE(std::wstring::npos,
+            text.find(content_settings_helper::OriginToWString(kUrl1)));
 
   text = model_->GetText(2, IDS_EXCEPTIONS_HOSTNAME_HEADER);
-  EXPECT_NE(OriginToWString(kUrl2), text);
-  EXPECT_NE(std::wstring::npos, text.find(OriginToWString(kUrl2)));
+  EXPECT_NE(content_settings_helper::OriginToWString(kUrl2), text);
+  EXPECT_NE(std::wstring::npos,
+            text.find(content_settings_helper::OriginToWString(kUrl2)));
 }
diff --git a/chrome/browser/notifications/notification_exceptions_table_model.cc b/chrome/browser/notifications/notification_exceptions_table_model.cc
index f4e8d68..fdb08f4 100644
--- a/chrome/browser/notifications/notification_exceptions_table_model.cc
+++ b/chrome/browser/notifications/notification_exceptions_table_model.cc
@@ -8,7 +8,7 @@
 #include "app/l10n_util_collator.h"
 #include "app/table_model_observer.h"
 #include "base/utf_string_conversions.h"
-#include "chrome/browser/geolocation/geolocation_content_settings_map.h"
+#include "chrome/common/content_settings_helper.h"
 #include "chrome/common/url_constants.h"
 #include "grit/generated_resources.h"
 
@@ -64,10 +64,7 @@ std::wstring NotificationExceptionsTableModel::GetText(int row,
                                                        int column_id) {
   const Entry& entry = entries_[row];
   if (column_id == IDS_EXCEPTIONS_HOSTNAME_HEADER) {
-    // TODO(bulach): factor out in a common function so that Notifications won't
-    // depend on Geolocation.
-    return UTF8ToWide(GeolocationContentSettingsMap::OriginToString(
-        entry.origin));
+    return content_settings_helper::OriginToWString(entry.origin);
   }
 
   if (column_id == IDS_EXCEPTIONS_ACTION_HEADER) {
diff --git a/chrome/chrome_common.gypi b/chrome/chrome_common.gypi
index 34ba21b..6978ebe 100644
--- a/chrome/chrome_common.gypi
+++ b/chrome/chrome_common.gypi
@@ -44,6 +44,8 @@
           'common/common_param_traits.h',
           'common/content_settings.cc',
           'common/content_settings.h',
+	  'common/content_settings_helper.cc',
+	  'common/content_settings_helper.h',
           'common/content_settings_types.h',
           'common/debug_flags.cc',
           'common/debug_flags.h',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 6018221..977a90d 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1117,6 +1117,7 @@
         'common/bzip2_unittest.cc',
         'common/child_process_logging_mac_unittest.mm',
         'common/common_param_traits_unittest.cc',
+	'common/content_settings_helper_unittest.cc',
         'common/deprecated/event_sys_unittest.cc',
         'common/desktop_notifications/active_notification_tracker_unittest.cc',
         'common/extensions/extension_action_unittest.cc',
diff --git a/chrome/common/content_settings_helper.cc b/chrome/common/content_settings_helper.cc
new file mode 100644
index 0000000..315b5e4
--- /dev/null
+++ b/chrome/common/content_settings_helper.cc
@@ -0,0 +1,22 @@
+// Copyright (c) 2010 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 "chrome/common/content_settings_helper.h"
+
+#include "base/utf_string_conversions.h"
+#include "base/string_piece.h"
+#include "chrome/common/url_constants.h"
+#include "googleurl/src/gurl.h"
+
+namespace content_settings_helper {
+
+std::wstring OriginToWString(const GURL& origin) {
+  std::string port_component((origin.IntPort() != url_parse::PORT_UNSPECIFIED) ?
+      ":" + origin.port() : "");
+  std::string scheme_component(!origin.SchemeIs(chrome::kHttpScheme) ?
+      origin.scheme() + chrome::kStandardSchemeSeparator : "");
+  return UTF8ToWide(scheme_component + origin.host() + port_component);
+}
+
+}  // namespace content_settings_helper
diff --git a/chrome/common/content_settings_helper.h b/chrome/common/content_settings_helper.h
new file mode 100644
index 0000000..0aa2947
--- /dev/null
+++ b/chrome/common/content_settings_helper.h
@@ -0,0 +1,20 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_COMMON_CONTENT_SETTINGS_HELPER_H_
+#define CHROME_COMMON_CONTENT_SETTINGS_HELPER_H_
+
+#include <string>
+
+class GURL;
+
+namespace content_settings_helper {
+
+// Return simplified string representing origin. If origin is using http or
+// the standard port, those parts are not included in the output.
+std::wstring OriginToWString(const GURL& origin);
+
+}  // namespace content_settings_helper
+
+#endif  // CHROME_COMMON_CONTENT_SETTINGS_HELPER_H_
diff --git a/chrome/common/content_settings_helper_unittest.cc b/chrome/common/content_settings_helper_unittest.cc
new file mode 100644
index 0000000..d46eda6
--- /dev/null
+++ b/chrome/common/content_settings_helper_unittest.cc
@@ -0,0 +1,42 @@
+// Copyright (c) 2010 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 "chrome/common/content_settings_helper.h"
+
+#include "googleurl/src/gurl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(ContentSettingsHelperTest, OriginToWString) {
+  // Urls with "http":
+  const GURL kUrl0("http://www.foo.com/bar");
+  const GURL kUrl1("http://foo.com/bar");
+
+  const GURL kUrl2("http://www.foo.com:81/bar");
+  const GURL kUrl3("http://foo.com:81/bar");
+
+  // Urls with "https":
+  const GURL kUrl4("https://www.foo.com/bar");
+  const GURL kUrl5("https://foo.com/bar");
+
+  const GURL kUrl6("https://www.foo.com:81/bar");
+  const GURL kUrl7("https://foo.com:81/bar");
+
+  // Now check the first group of urls with just "http":
+  EXPECT_EQ(L"www.foo.com", content_settings_helper::OriginToWString(kUrl0));
+  EXPECT_EQ(L"foo.com", content_settings_helper::OriginToWString(kUrl1));
+
+  EXPECT_EQ(L"www.foo.com:81", content_settings_helper::OriginToWString(kUrl2));
+  EXPECT_EQ(L"foo.com:81", content_settings_helper::OriginToWString(kUrl3));
+
+  // Now check the second group of urls with "https":
+  EXPECT_EQ(L"https://www.foo.com",
+            content_settings_helper::OriginToWString(kUrl4));
+  EXPECT_EQ(L"https://foo.com",
+            content_settings_helper::OriginToWString(kUrl5));
+
+  EXPECT_EQ(L"https://www.foo.com:81",
+            content_settings_helper::OriginToWString(kUrl6));
+  EXPECT_EQ(L"https://foo.com:81",
+            content_settings_helper::OriginToWString(kUrl7));
+}