Make geolocation API and X-Geo header access consistent

chrome/browser/android/search_geolocation/search_geolocation_service.h

Index: chrome/browser/android/search_geolocation/search_geolocation_service.h
diff --git a/chrome/browser/android/search_geolocation/search_geolocation_service.h b/chrome/browser/android/search_geolocation/search_geolocation_service.h
new file mode 100644
index 0000000000000000000000000000000000000000..11fa7db76f29ad47fb680651b85330f97712739d
--- /dev/null
+++ b/chrome/browser/android/search_geolocation/search_geolocation_service.h
@@ -0,0 +1,134 @@
+// Copyright 2017 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_BROWSER_ANDROID_SEARCH_GEOLOCATION_SEARCH_GEOLOCATION_SERVICE_H_
+#define CHROME_BROWSER_ANDROID_SEARCH_GEOLOCATION_SEARCH_GEOLOCATION_SERVICE_H_
+
+#include "base/memory/singleton.h"
+#include "components/content_settings/core/common/content_settings.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "components/keyed_service/core/keyed_service.h"
+#include "components/search_engines/template_url_service_observer.h"
+#include "url/gurl.h"

[raymes, 2017/01/09 05:47:26]

nit: I don't think this is needed anymore

[benwells, 2017/01/09 21:02:22]

Done.

+#include "url/origin.h"
+
+namespace content{
+class BrowserContext;
+}
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
+class HostContentSettingsMap;
+class PrefService;
+class Profile;
+class TemplateURLService;
+
+// Helper class to manage the DSE Geolocation setting. It keeps the setting
+// valid by watching change to the CCTLD and DSE, and also provides logic for
+// whether the setting should be used and it's current value.

[raymes, 2017/01/09 05:47:26]

nit: not sure if it's important to explain what CCTLD and DSE terms mean here.
Other readers might be confused.

[benwells, 2017/01/09 21:02:22]

Done.

+class SearchGeolocationService
+    : public TemplateURLServiceObserver,
+      public KeyedService {
+ public:
+  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
+  explicit SearchGeolocationService(Profile* profile);
+
+  // Returns whether the DSE geolocation setting is applicable for geolocation
+  // requests for the given origin.

[raymes, 2017/01/09 05:47:26]

nit: might want to clarify that this is the top level origin

[benwells, 2017/01/09 21:02:22]

Done.

+  bool UseDSEGeolocationSetting(const url::Origin& requesting_origin);
+
+  // Returns the DSE geolocation setting, after applying any updates needed to
+  // make it valid.
+  bool GetDSEGeolocationSetting();
+
+  // Changes the DSE geolocation setting.
+  void SetDSEGeolocationSetting(bool setting);
+
+  // KeyedService:
+  void Shutdown() override;
+
+  // Factory implementation will not create a service in incognito.
+  class Factory : public BrowserContextKeyedServiceFactory {

[raymes, 2017/01/09 05:47:26]

nit: I think inner classes tend to go before the member functions (in order).
The same in the .cc file

[benwells, 2017/01/09 21:02:22]

Done.

+   public:
+    static SearchGeolocationService* GetForBrowserContext(
+        content::BrowserContext* context);
+
+    static Factory* GetInstance();
+   private:
+    friend struct base::DefaultSingletonTraits<Factory>;
+
+    Factory();
+    ~Factory() override;
+
+    // BrowserContextKeyedServiceFactory
+    bool ServiceIsCreatedWithBrowserContext() const override;
+    KeyedService* BuildServiceInstanceFor(
+        content::BrowserContext* profile) const override;
+  };
+
+ private:
+  struct PrefValue;
+
+  ~SearchGeolocationService() override;
+
+  // TemplateURLServiceObserver
+  // When the DSE CCTLD changes (either by changing their DSE or by changing
+  // their CCTLD, and their DSE supports geolocation:
+  // * If the DSE CCTLD origin permission is BLOCK, but the DSE geolocation
+  //   setting is on, change the DSE geolocation setting to off
+  // * If the DSE CCTLD origin permission is ALLOW, but the DSE geolocation
+  //   setting is off, reset the DSE CCTLD origin permission to ASK.
+  // Also, if the previous DSE did not support geolocation, and the new one
+  // does, and the geolocation setting is on, reset whether the DSE geolocation
+  // disclosure has been shown.
+  void OnTemplateURLServiceChanged() override;
+
+  // Initialize the DSE geolocation setting if it hasn't already been
+  // initialized. Also, if it hasn't been initialized, reset whether the DSE
+  // geolocation disclosure has been shown to ensure user who may have seen it
+  // on earlier versions (due to Finch experiments) see it again.
+  void InitializeDSEGeolocationSettingIfNeeded();
+
+  // Check that the DSE geolocation setting is valid with respect to the content
+  // setting. The rules of vaidity are:
+  // * If the content setting is BLOCK, the DSE geolocation setting must
+  //   be false.
+  // * If the content setting is ALLOW, the DSE geolocation setting must be
+  //   true.
+  // * If the content setting is ASK, the DSE geolocation setting can be true or
+  //   false.
+  // One way the setting could become invalid is if the feature enabling the
+  // setting was disabled (via Finch or flags), content settings were changed,
+  // and the feature enabled again.
+  // Another way is if the setting was changed while enterprise policy is in

[raymes, 2017/01/09 05:47:27]

nit: fill the previous line to 80chars

[benwells, 2017/01/09 21:02:22]

Done. I also updated the text a bit.

+  // effect.
+  void EnsureDSEGeolocationSettingIsValid();
+
+  // Returns true if the current DSE is Google.

[raymes, 2017/01/09 05:47:26]

nit: might want to mention that Google is currently the only search engine that
supports DSE geolocation

[benwells, 2017/01/09 21:02:22]

Done.

+  bool IsGoogleSearchEngine();
+
+  // Returns the origin of the current CCTLD of the default search engine. If
+  // the default search engine is not Google, will return a unique Origin.
+  url::Origin GetDSECCTLD();
+
+  PrefValue GetDSEGeolocationPref();
+  void SetDSEGeolocationPref(const PrefValue& pref);
+
+  ContentSetting GetCurrentContentSetting();

[raymes, 2017/01/09 05:47:27]

nit: might want to clairfy that these apply to the current DSE CCTLD origin

[benwells, 2017/01/09 21:02:22]

Done.

+  void ResetContentSetting();
+
+  // Whether the feature/experiment setup is enabling the consistent search
+  // geolocation system.
+  bool UseConsistentSearchGeolocation();
+
+  Profile* profile_;
+  PrefService* pref_service_;
+  HostContentSettingsMap* host_content_settings_map_;
+  TemplateURLService* template_url_service_;
+};
+
+#endif  // CHROME_BROWSER_ANDROID_SEARCH_GEOLOCATION_SEARCH_GEOLOCATION_SERVICE_H_