Make geolocation API and X-Geo header access consistent

chrome/browser/content_settings/host_content_settings_map_unittest.cc

 // Copyright (c) 2012 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 <memory>
 #include <string>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/content_settings/content_settings_mock_observer.h"
 #include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/mock_settings_observer.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/content_settings_details.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/browser/website_settings_info.h"
 #include "components/content_settings/core/browser/website_settings_registry.h"
 #include "components/content_settings/core/common/pref_names.h"
+#include "components/policy/core/common/policy_service.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/test_browser_thread.h"
 #include "net/base/static_cookie_policy.h"
 #include "ppapi/features/features.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "chrome/browser/extensions/extension_special_storage_policy.h"
+#endif

[raymes, 2017/01/11 08:30:12]

Are these 2 includes above needed?

[benwells, 2017/01/11 09:26:11]

Yes, they are needed because two of the nullptrs passed into TestingProfile
constructor are smart pointers. These can't be constructed unless they know the
size of the type etc.

+
 using content::BrowserThread;
 
 using ::testing::_;
 
 namespace {
 
 bool MatchPrimaryPattern(const ContentSettingsPattern& expected_primary,
                          const ContentSettingsPattern& primary_pattern,
                          const ContentSettingsPattern& secondary_pattern) {
   return expected_primary == primary_pattern;
@@ skipping 1198 matching lines @@
   host_content_settings_map->AddObserver(&mock_observer);
 
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 host, host, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
   host_content_settings_map->SetContentSettingDefaultScope(
       host, GURL(), CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
       CONTENT_SETTING_DEFAULT);
 }
 
 TEST_F(HostContentSettingsMapTest, GuestProfile) {

[msramek, 2017/01/11 09:51:10]

The guest profile is Desktop only (unless I'm really missing something).

I haven't looked deeper into Profile and TestingProfile, but my hunch is that
the previous code SetGuestSession(true) was somehow interpreted as incognito on
Android and so nothing was written to the disk, while your new code is
interpreted as regular mode and thus we do write to the disk.

In that sense, disabling this test on Android would actually be the right thing
to do.

-  TestingProfile profile;
-  profile.SetGuestSession(true);
+  TestingProfile profile(base::FilePath(),
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+                         nullptr,
+#endif
+                         nullptr, nullptr, nullptr, true /* guest_profile */,
+                         "", nullptr, TestingProfile::TestingFactories(),

[raymes, 2017/01/11 08:30:12]

nit: std::string() for empty strings

+                         "testing_profile");
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
 
   GURL host("http://example.com/");
   ContentSettingsPattern pattern =
       ContentSettingsPattern::FromString("[*.]example.com");
 
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 host, host, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 
   // Changing content settings should not result in any prefs being stored
   // however the value should be set in memory.
   host_content_settings_map->SetContentSettingDefaultScope(
       host, GURL(), CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
       CONTENT_SETTING_BLOCK);
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
                 host, host, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 
   const base::DictionaryValue* all_settings_dictionary =
       profile.GetPrefs()->GetDictionary(
           GetPrefName(CONTENT_SETTINGS_TYPE_COOKIES));
   EXPECT_TRUE(all_settings_dictionary->empty());
 }
 
 // Default settings should not be modifiable for the guest profile (there is no
 // UI to do this).
 TEST_F(HostContentSettingsMapTest, GuestProfileDefaultSetting) {
-  TestingProfile profile;
-  profile.SetGuestSession(true);
+  TestingProfile profile(base::FilePath(),
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+                         nullptr,
+#endif
+                         nullptr, nullptr, nullptr, true /* guest_profile */,
+                         "", nullptr, TestingProfile::TestingFactories(),
+                         "testing_profile");
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
 
   GURL host("http://example.com/");
 
   // There are no custom rules, so this should be the default.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 host, host, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 
   host_content_settings_map->SetDefaultContentSetting(
       CONTENT_SETTINGS_TYPE_COOKIES, CONTENT_SETTING_BLOCK);
 
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 host, host, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 }
 
 // We used to incorrectly store content settings in prefs for the guest profile.
 // We need to ensure these get deleted appropriately.
 TEST_F(HostContentSettingsMapTest, GuestProfileMigration) {
-  TestingProfile profile;
-  profile.SetGuestSession(true);
+  TestingProfile profile(base::FilePath(),
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+                         nullptr,
+#endif
+                         nullptr, nullptr, nullptr, true /* guest_profile */,
+                         "", nullptr, TestingProfile::TestingFactories(),
+                         "testing_profile");
 
   // Set a pref manually in the guest profile.
   std::unique_ptr<base::Value> value =
       base::JSONReader::Read("{\"[*.]\\xC4\\x87ira.com,*\":{\"setting\":1}}");
   profile.GetPrefs()->Set(GetPrefName(CONTENT_SETTINGS_TYPE_COOKIES), *value);
 
   // Test that during construction all the prefs get cleared.
   HostContentSettingsMapFactory::GetForProfile(&profile);
 
   const base::DictionaryValue* all_settings_dictionary =
@@ skipping 170 matching lines @@
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 https_host_narrower, https_host_narrower,
                 CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 }
 
 // Ensure that migration only happens once upon construction of the HCSM and
 // once after syncing (even when these events occur multiple times).
 TEST_F(HostContentSettingsMapTest, DomainToOriginMigrationStatus) {
   TestingProfile profile;
+
+  // Construct the map now to make the various platforms equivalent. On Android
+  // the map is created with the profile (due to dependencies), while on other
+  // platform it is created lazily. Note, migration should be run here.

[raymes, 2017/01/11 08:30:12]

nit: platforms

[benwells, 2017/01/12 03:10:43]

Done.

+  HostContentSettingsMap* host_content_settings_map =
+      HostContentSettingsMapFactory::GetForProfile(&profile);
+
   PrefService* prefs = profile.GetPrefs();
 
   GURL http_host("http://example.com/");
   GURL http_host_narrower("http://a.example.com/");
   std::string host_pattern_string =
       ContentSettingsPattern::FromURL(http_host).ToString();
 
   {
     DictionaryPrefUpdate update(prefs,
                                 GetPrefName(CONTENT_SETTINGS_TYPE_POPUPS));
     base::DictionaryValue* all_settings_dictionary = update.Get();
     ASSERT_TRUE(NULL != all_settings_dictionary);
 
     base::DictionaryValue* domain_setting = new base::DictionaryValue;
     domain_setting->SetInteger("setting", CONTENT_SETTING_ALLOW);
     all_settings_dictionary->SetWithoutPathExpansion(host_pattern_string + ",*",
                                                      domain_setting);
   }
 
   const base::DictionaryValue* all_settings_dictionary =
       prefs->GetDictionary(GetPrefName(CONTENT_SETTINGS_TYPE_POPUPS));
   const base::DictionaryValue* result = NULL;
   EXPECT_TRUE(all_settings_dictionary->GetDictionaryWithoutPathExpansion(
       "[*.]example.com,*", &result));
 
-  // Migration is done on construction of HostContentSettingsMap.
-  HostContentSettingsMap* host_content_settings_map =
-      HostContentSettingsMapFactory::GetForProfile(&profile);
+  // Migration is done on construction of HostContentSettingsMap. True to run it

[raymes, 2017/01/11 08:30:12]

nit: True->Try

[benwells, 2017/01/12 03:10:43]

Done.

+  // again. This should not do anything as it has already ran.
+  host_content_settings_map->MigrateDomainScopedSettings(
+      false /* after_sync */);
 
   // Change default setting to BLOCK.
   host_content_settings_map->SetDefaultContentSetting(
       CONTENT_SETTINGS_TYPE_POPUPS, CONTENT_SETTING_BLOCK);
+
   EXPECT_EQ(
       CONTENT_SETTING_ALLOW,
       host_content_settings_map->GetContentSetting(
+          http_host, http_host, CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
+
+  // The setting should still be allow as it hasn't been migrated.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                http_host_narrower, http_host_narrower,
+                CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
+
+  // Set the pref to its initial state so that migration can be triggered again,
+  // just for the sake of testing.
+  int default_value;
+  prefs->GetDefaultPrefValue(prefs::kDomainToOriginMigrationStatus)
+      ->GetAsInteger(&default_value);
+  prefs->SetInteger(prefs::kDomainToOriginMigrationStatus, default_value);
+
+  // Now, do the migration. This should work as we've cleared the pref back to
+  // its default value.
+  host_content_settings_map->MigrateDomainScopedSettings(false);
+
+  // Now the settings should be migrated.
+  EXPECT_EQ(
+      CONTENT_SETTING_ALLOW,
+      host_content_settings_map->GetContentSetting(
           http_host, http_host, CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
   // Settings only apply to origins. Migration got executed.
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
                 http_host_narrower, http_host_narrower,
                 CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
 
   GURL https_host("https://example.com/");
   GURL https_host_narrower("https://a.example.com/");
 
   host_content_settings_map->SetContentSettingCustomScope(
       ContentSettingsPattern::FromURL(https_host),
       ContentSettingsPattern::Wildcard(), CONTENT_SETTINGS_TYPE_POPUPS,
       std::string(), CONTENT_SETTING_ALLOW);
+
   EXPECT_EQ(
       CONTENT_SETTING_ALLOW,
       host_content_settings_map->GetContentSetting(
           https_host, https_host, CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
   // Settings apply to subdomains.
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 https_host_narrower, https_host_narrower,
                 CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
 
@@ skipping 155 matching lines @@
   EXPECT_TRUE(map->CanSetNarrowestContentSetting(
       valid_url, valid_url,
       CONTENT_SETTINGS_TYPE_POPUPS));
 
   GURL invalid_url("about:blank");
   EXPECT_FALSE(map->CanSetNarrowestContentSetting(
       invalid_url, invalid_url,
       CONTENT_SETTINGS_TYPE_POPUPS));
 }