Remove shell & webview AccessTokenStores

Content shell and Android webview don't make network geolocation
requests. Previously these embedders had dummy (no-op) implementations
of AccessTokenStore. This CL removes the dummy implementations and
instead uses the existing UseNetworkLocationProviders flag in
GeolocationDelegate as a gate.

This is part of the removal of all AccessTokenStores. Overall design:
https://docs.google.com/document/d/1A-ZYMadJkwdQad_pC3MbpuCVNOOnRP_UZjGnXl-FOqE

Details of reasoning why this is safe:

Content shell:
AccessTokenStore was being created but LoadAccessTokens was not being
called on it (after crrev.com/c/641175).

Android webview:
AccessTokenStore was being created and LoadAccessTokens was being called
on it, but to no consequence due to an "#if defined(OS_ANDROID)" block at
location_arbitrator.cc:174.

Bug: 748921
Change-Id: I10ed3668ee015f855acf604e445643617bbe7713
Reviewed-on: https://chromium-review.googlesource.com/658010
Commit-Queue: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Selim Gurun <sgurun@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501494}

android_webview/browser/aw_browser_main_parts.cc

@@ -48,33 +48,18 @@
 namespace android_webview {
 namespace {
 
-class AwAccessTokenStore : public device::AccessTokenStore {
- public:
-  AwAccessTokenStore() { }
-
-  // device::AccessTokenStore implementation
-  void LoadAccessTokens(const LoadAccessTokensCallback& request) override {
-    AccessTokenStore::AccessTokenMap access_token_map;
-    // AccessTokenMap and net::URLRequestContextGetter not used on Android,
-    // but Run needs to be called to finish the geolocation setup.
-    request.Run(access_token_map, NULL);
-  }
-  void SaveAccessToken(const GURL& server_url,
-                       const base::string16& access_token) override {}
-
- private:
-  ~AwAccessTokenStore() override {}
-
-  DISALLOW_COPY_AND_ASSIGN(AwAccessTokenStore);
-};
-
 // A provider of Geolocation services to override AccessTokenStore.
 class AwGeolocationDelegate : public device::GeolocationDelegate {
  public:
   AwGeolocationDelegate() = default;
 
+  // Android doesn't use NetworkLocationProvider (the capability is folded into
+  // the system location provider).
+  bool UseNetworkLocationProviders() override { return false; }
+
   scoped_refptr<device::AccessTokenStore> CreateAccessTokenStore() final {
-    return new AwAccessTokenStore();
+    NOTREACHED() << "No network geolocation for Android webview";
+    return nullptr;
   }
 
  private:

content/shell/BUILD.gn

@@ -102,8 +102,6 @@ static_library("content_shell_lib") {
     "browser/renderer_host/shell_render_widget_host_view_mac_delegate.mm",
     "browser/shell.cc",
     "browser/shell.h",
-    "browser/shell_access_token_store.cc",
-    "browser/shell_access_token_store.h",
     "browser/shell_android.cc",
     "browser/shell_application_mac.h",
     "browser/shell_application_mac.mm",

content/shell/browser/shell_access_token_store.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 "content/shell/browser/shell_access_token_store.h"
-
-#include "base/bind.h"
-#include "base/message_loop/message_loop.h"
-#include "base/strings/utf_string_conversions.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/storage_partition.h"
-#include "content/shell/browser/shell_browser_context.h"
-
-namespace content {
-
-ShellAccessTokenStore::ShellAccessTokenStore(
-    content::ShellBrowserContext* shell_browser_context)
-    : shell_browser_context_(shell_browser_context),
-      system_request_context_(NULL) {
-}
-
-ShellAccessTokenStore::~ShellAccessTokenStore() {
-}
-
-void ShellAccessTokenStore::LoadAccessTokens(
-    const LoadAccessTokensCallback& callback) {
-  BrowserThread::PostTaskAndReply(
-      BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&ShellAccessTokenStore::GetRequestContextOnUIThread, this,
-                     shell_browser_context_),
-      base::BindOnce(&ShellAccessTokenStore::RespondOnOriginatingThread, this,
-                     callback));
-}
-
-void ShellAccessTokenStore::GetRequestContextOnUIThread(
-    content::ShellBrowserContext* shell_browser_context) {
-  system_request_context_ =
-      BrowserContext::GetDefaultStoragePartition(shell_browser_context)->
-          GetURLRequestContext();
-}
-
-void ShellAccessTokenStore::RespondOnOriginatingThread(
-    const LoadAccessTokensCallback& callback) {
-  callback.Run(AccessTokenMap(), system_request_context_);
-  system_request_context_ = NULL;
-}
-
-void ShellAccessTokenStore::SaveAccessToken(
-    const GURL& server_url, const base::string16& access_token) {
-}
-
-}  // namespace content

content/shell/browser/shell_access_token_store.h

-// 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.
-
-#ifndef CONTENT_SHELL_BROWSER_SHELL_ACCESS_TOKEN_STORE_H_
-#define CONTENT_SHELL_BROWSER_SHELL_ACCESS_TOKEN_STORE_H_
-
-#include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "device/geolocation/access_token_store.h"
-#include "net/url_request/url_request_context_getter.h"
-
-namespace content {
-class ShellBrowserContext;
-
-// Dummy access token store used to initialise the network location provider.
-class ShellAccessTokenStore : public device::AccessTokenStore {
- public:
-  explicit ShellAccessTokenStore(
-      content::ShellBrowserContext* shell_browser_context);
-
- private:
-  ~ShellAccessTokenStore() override;
-
-  void GetRequestContextOnUIThread(
-      content::ShellBrowserContext* shell_browser_context);
-  void RespondOnOriginatingThread(const LoadAccessTokensCallback& callback);
-
-  // AccessTokenStore
-  void LoadAccessTokens(const LoadAccessTokensCallback& callback) override;
-
-  void SaveAccessToken(const GURL& server_url,
-                       const base::string16& access_token) override;
-
-  content::ShellBrowserContext* shell_browser_context_;
-  scoped_refptr<net::URLRequestContextGetter> system_request_context_;
-
-  DISALLOW_COPY_AND_ASSIGN(ShellAccessTokenStore);
-};
-
-}  // namespace content
-
-#endif  // CONTENT_SHELL_BROWSER_SHELL_ACCESS_TOKEN_STORE_H_

content/shell/browser/shell_browser_main_parts.cc

@@ -20,12 +20,12 @@
 #include "content/public/common/url_constants.h"
 #include "content/shell/android/shell_descriptors.h"
 #include "content/shell/browser/shell.h"
-#include "content/shell/browser/shell_access_token_store.h"
 #include "content/shell/browser/shell_browser_context.h"
 #include "content/shell/browser/shell_devtools_manager_delegate.h"
 #include "content/shell/browser/shell_net_log.h"
 #include "content/shell/common/shell_switches.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
+#include "device/geolocation/access_token_store.h"
 #include "device/geolocation/geolocation_delegate.h"
 #include "device/geolocation/geolocation_provider.h"
 #include "net/base/filename_util.h"
@@ -66,19 +66,18 @@ namespace {
 // A provider of services for Geolocation.
 class ShellGeolocationDelegate : public device::GeolocationDelegate {
  public:
-  explicit ShellGeolocationDelegate(ShellBrowserContext* context)
-      : context_(context) {}
+  ShellGeolocationDelegate() = default;
 
   // Since content shell is a test executable, rather than an end-user program,
   // don't make calls to the network geolocation API.
   bool UseNetworkLocationProviders() override { return false; }
 
   scoped_refptr<device::AccessTokenStore> CreateAccessTokenStore() final {
-    return new ShellAccessTokenStore(context_);
+    NOTREACHED() << "No network geolocation for content shell";
+    return nullptr;
   }
 
  private:
-  ShellBrowserContext* context_;
   DISALLOW_COPY_AND_ASSIGN(ShellGeolocationDelegate);
 };
 
@@ -196,7 +195,7 @@ void ShellBrowserMainParts::PreMainMessageLoopRun() {
   net_log_.reset(new ShellNetLog("content_shell"));
   InitializeBrowserContexts();
   device::GeolocationProvider::SetGeolocationDelegate(
-      new ShellGeolocationDelegate(browser_context()));
+      new ShellGeolocationDelegate());
   Shell::Initialize();
   net::NetModule::SetResourceProvider(PlatformResourceProvider);
   ShellDevToolsManagerDelegate::StartHttpHandler(browser_context_.get());

device/geolocation/geolocation_delegate.h

@@ -25,6 +25,7 @@ class DEVICE_GEOLOCATION_EXPORT GeolocationDelegate {
   virtual bool UseNetworkLocationProviders();
 
   // Creates a new AccessTokenStore for geolocation. May return nullptr.
+  // Won't be called unless UseNetworkLocationProviders() is true.
   virtual scoped_refptr<AccessTokenStore> CreateAccessTokenStore();
 
   // Allows an embedder to return its own LocationProvider implementation.

device/geolocation/location_arbitrator.cc

@@ -60,15 +60,17 @@ bool LocationArbitrator::StartProvider(bool enable_high_accuracy) {
   if (providers_.empty()) {
     RegisterSystemProvider();
 
-    const scoped_refptr<AccessTokenStore> access_token_store =
-        GetAccessTokenStore();
-    if (access_token_store && delegate_->UseNetworkLocationProviders()) {
-      DCHECK(DefaultNetworkProviderURL().is_valid());
-      token_store_callback_.Reset(
-          base::Bind(&LocationArbitrator::OnAccessTokenStoresLoaded,
-                     base::Unretained(this)));
-      access_token_store->LoadAccessTokens(token_store_callback_.callback());
-      return true;
+    if (delegate_->UseNetworkLocationProviders()) {
+      const scoped_refptr<AccessTokenStore> access_token_store =
+          GetAccessTokenStore();
+      if (access_token_store) {
+        DCHECK(DefaultNetworkProviderURL().is_valid());
+        token_store_callback_.Reset(
+            base::Bind(&LocationArbitrator::OnAccessTokenStoresLoaded,
+                       base::Unretained(this)));
+        access_token_store->LoadAccessTokens(token_store_callback_.callback());
+        return true;
+      }
     }
   }
   return DoStartProviders();