fix: opt into location services once device service has been started (#14253)

* fix: opt into location services once device service has been started

* refactor: provide fake location provider to mock geolocation reponses

* chore: add spec for navigator.geolocation api using fake location provider

BUILD.gn

@@ -32,6 +32,12 @@ declare_args() {
   enable_desktop_capturer = true
   enable_run_as_node = true
   enable_osr = false
+
+  # Provide a fake location provider for mocking
+  # the geolocation responses. Disable it if you
+  # need to test with chromium's location provider.
+  # Should not be enabled for release build.
+  enable_fake_location_provider = !is_official_build
 }
 
 if (is_mas_build) {
@@ -282,6 +288,13 @@ static_library("electron_lib") {
     ]
   }
 
+  if (enable_fake_location_provider) {
+    defines += [
+      "OVERRIDE_LOCATION_PROVIDER"
+    ]
+    sources += filenames_gypi.lib_sources_location_provider
+  }
+
   if (is_mac) {
     deps += [
       "//third_party/crashpad/crashpad/client",
@@ -377,7 +390,6 @@ static_library("electron_lib") {
   }
 
   if (enable_desktop_capturer) {
-    deps += [ "//third_party/webrtc/modules/desktop_capture" ]
     sources += [
       "atom/browser/api/atom_api_desktop_capturer.cc",
       "atom/browser/api/atom_api_desktop_capturer.h",

atom/browser/atom_browser_client.cc

@@ -44,6 +44,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/web_preferences.h"
+#include "device/geolocation/public/cpp/location_provider.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "ppapi/host/ppapi_host.h"
 #include "services/network/public/cpp/resource_request_body.h"
@@ -64,6 +65,10 @@
 #include "chrome/browser/renderer_host/pepper/chrome_browser_pepper_host_factory.h"
 #endif  // defined(ENABLE_PEPPER_FLASH)
 
+#if defined(OVERRIDE_LOCATION_PROVIDER)
+#include "atom/browser/fake_location_provider.h"
+#endif  // defined(OVERRIDE_LOCATION_PROVIDER)
+
 using content::BrowserThread;
 
 namespace atom {
@@ -495,6 +500,15 @@ std::unique_ptr<net::ClientCertStore> AtomBrowserClient::CreateClientCertStore(
 #endif
 }
 
+std::unique_ptr<device::LocationProvider>
+AtomBrowserClient::OverrideSystemLocationProvider() {
+#if defined(OVERRIDE_LOCATION_PROVIDER)
+  return std::make_unique<FakeLocationProvider>();
+#else
+  return nullptr;
+#endif
+}
+
 brightray::BrowserMainParts* AtomBrowserClient::OverrideCreateBrowserMainParts(
     const content::MainFunctionParams&) {
   v8::V8::Initialize();  // Init V8 before creating main parts.

atom/browser/atom_browser_client.h

@@ -106,6 +106,8 @@ class AtomBrowserClient : public brightray::BrowserClient,
   void SiteInstanceDeleting(content::SiteInstance* site_instance) override;
   std::unique_ptr<net::ClientCertStore> CreateClientCertStore(
       content::ResourceContext* resource_context) override;
+  std::unique_ptr<device::LocationProvider> OverrideSystemLocationProvider()
+      override;
 
   // brightray::BrowserClient:
   brightray::BrowserMainParts* OverrideCreateBrowserMainParts(

atom/browser/atom_browser_main_parts.cc

@@ -237,9 +237,6 @@ void AtomBrowserMainParts::PostMainMessageLoopStart() {
 #if defined(OS_POSIX)
   HandleShutdownSignals();
 #endif
-  // TODO(deepak1556): Enable this optionally based on response
-  // from AtomPermissionManager.
-  GetGeolocationControl()->UserDidOptIntoLocationServices();
 }
 
 void AtomBrowserMainParts::PostMainMessageLoopRun() {

atom/browser/atom_browser_main_parts.h

@@ -54,6 +54,10 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
   // Returns a closure that can be used to remove |callback| from the list.
   void RegisterDestructionCallback(base::OnceClosure callback);
 
+  // Returns the connection to GeolocationControl which can be
+  // used to enable the location services once per client.
+  device::mojom::GeolocationControl* GetGeolocationControl();
+
   Browser* browser() { return browser_.get(); }
 
  protected:
@@ -87,8 +91,6 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
   std::unique_ptr<brightray::ViewsDelegate> views_delegate_;
 #endif
 
-  device::mojom::GeolocationControl* GetGeolocationControl();
-
   // A fake BrowserProcess object that used to feed the source code from chrome.
   std::unique_ptr<BrowserProcess> fake_browser_process_;
 

atom/browser/atom_permission_manager.cc

@@ -7,6 +7,7 @@
 #include <vector>
 
 #include "atom/browser/atom_browser_client.h"
+#include "atom/browser/atom_browser_main_parts.h"
 #include "atom/browser/web_contents_preferences.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/permission_type.h"
@@ -43,6 +44,7 @@ class AtomPermissionManager::PendingRequest {
                  const StatusesCallback& callback)
       : render_process_id_(render_frame_host->GetProcess()->GetID()),
         callback_(callback),
+        permissions_(permissions),
         results_(permissions.size(), blink::mojom::PermissionStatus::DENIED),
         remaining_results_(permissions.size()) {}
 
@@ -50,6 +52,18 @@ class AtomPermissionManager::PendingRequest {
                            blink::mojom::PermissionStatus status) {
     DCHECK(!IsComplete());
 
+    if (status == blink::mojom::PermissionStatus::GRANTED) {
+      const auto permission = permissions_[permission_id];
+      if (permission == content::PermissionType::MIDI_SYSEX) {
+        content::ChildProcessSecurityPolicy::GetInstance()
+            ->GrantSendMidiSysExMessage(render_process_id_);
+      } else if (permission == content::PermissionType::GEOLOCATION) {
+        AtomBrowserMainParts::Get()
+            ->GetGeolocationControl()
+            ->UserDidOptIntoLocationServices();
+      }
+    }
+
     results_[permission_id] = status;
     --remaining_results_;
   }
@@ -63,6 +77,7 @@ class AtomPermissionManager::PendingRequest {
  private:
   int render_process_id_;
   const StatusesCallback callback_;
+  std::vector<content::PermissionType> permissions_;
   std::vector<blink::mojom::PermissionStatus> results_;
   size_t remaining_results_;
 };
@@ -139,6 +154,10 @@ int AtomPermissionManager::RequestPermissionsWithDetails(
         content::ChildProcessSecurityPolicy::GetInstance()
             ->GrantSendMidiSysExMessage(
                 render_frame_host->GetProcess()->GetID());
+      } else if (permission == content::PermissionType::GEOLOCATION) {
+        AtomBrowserMainParts::Get()
+            ->GetGeolocationControl()
+            ->UserDidOptIntoLocationServices();
       }
       statuses.push_back(blink::mojom::PermissionStatus::GRANTED);
     }
@@ -153,10 +172,6 @@ int AtomPermissionManager::RequestPermissionsWithDetails(
 
   for (size_t i = 0; i < permissions.size(); ++i) {
     auto permission = permissions[i];
-    if (permission == content::PermissionType::MIDI_SYSEX) {
-      content::ChildProcessSecurityPolicy::GetInstance()
-          ->GrantSendMidiSysExMessage(render_frame_host->GetProcess()->GetID());
-    }
     const auto callback =
         base::Bind(&AtomPermissionManager::OnPermissionResponse,
                    base::Unretained(this), request_id, i);
@@ -186,7 +201,6 @@ void AtomPermissionManager::OnPermissionResponse(
   }
 }
 
-
 void AtomPermissionManager::ResetPermission(content::PermissionType permission,
                                             const GURL& requesting_origin,
                                             const GURL& embedding_origin) {}

atom/browser/fake_location_provider.cc

@@ -0,0 +1,44 @@
+// Copyright (c) 2018 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/fake_location_provider.h"
+
+#include "base/callback.h"
+#include "base/time/time.h"
+
+namespace atom {
+
+FakeLocationProvider::FakeLocationProvider() {
+  position_.latitude = 10;
+  position_.longitude = -10;
+  position_.accuracy = 1;
+  position_.error_code =
+      device::mojom::Geoposition::ErrorCode::POSITION_UNAVAILABLE;
+}
+
+FakeLocationProvider::~FakeLocationProvider() = default;
+
+void FakeLocationProvider::SetUpdateCallback(
+    const LocationProviderUpdateCallback& callback) {
+  callback_ = callback;
+}
+
+void FakeLocationProvider::StartProvider(bool high_accuracy) {}
+
+void FakeLocationProvider::StopProvider() {}
+
+const device::mojom::Geoposition& FakeLocationProvider::GetPosition() {
+  return position_;
+}
+
+void FakeLocationProvider::OnPermissionGranted() {
+  if (!callback_.is_null()) {
+    // Check device::ValidateGeoPosition for range of values.
+    position_.error_code = device::mojom::Geoposition::ErrorCode::NONE;
+    position_.timestamp = base::Time::Now();
+    callback_.Run(this, position_);
+  }
+}
+
+}  // namespace atom

atom/browser/fake_location_provider.h

@@ -0,0 +1,36 @@
+// Copyright (c) 2018 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_BROWSER_FAKE_LOCATION_PROVIDER_H_
+#define ATOM_BROWSER_FAKE_LOCATION_PROVIDER_H_
+
+#include "base/macros.h"
+#include "device/geolocation/public/cpp/location_provider.h"
+#include "services/device/public/mojom/geoposition.mojom.h"
+
+namespace atom {
+
+class FakeLocationProvider : public device::LocationProvider {
+ public:
+  FakeLocationProvider();
+  ~FakeLocationProvider() override;
+
+  // LocationProvider Implementation:
+  void SetUpdateCallback(
+      const LocationProviderUpdateCallback& callback) override;
+  void StartProvider(bool high_accuracy) override;
+  void StopProvider() override;
+  const device::mojom::Geoposition& GetPosition() override;
+  void OnPermissionGranted() override;
+
+ private:
+  device::mojom::Geoposition position_;
+  LocationProviderUpdateCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeLocationProvider);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_BROWSER_FAKE_LOCATION_PROVIDER_H_

atom/common/api/features.cc

@@ -31,6 +31,14 @@ bool IsPDFViewerEnabled() {
 #endif
 }
 
+bool IsFakeLocationProviderEnabled() {
+#if defined(OVERRIDE_LOCATION_PROVIDER)
+  return true;
+#else
+  return false;
+#endif
+}
+
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
@@ -39,6 +47,8 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("isDesktopCapturerEnabled", &IsDesktopCapturerEnabled);
   dict.SetMethod("isOffscreenRenderingEnabled", &IsOffscreenRenderingEnabled);
   dict.SetMethod("isPDFViewerEnabled", &IsPDFViewerEnabled);
+  dict.SetMethod("isFakeLocationProviderEnabled",
+                 &IsFakeLocationProviderEnabled);
 }
 
 }  // namespace

electron.gyp

@@ -356,6 +356,16 @@
           'link_settings': {
             'libraries': [ '<@(libchromiumcontent_v8_libraries)' ],
           },
+          'sources': [
+            '<@(lib_sources_location_provider)',
+          ],
+          'defines': [
+            # Enable fake location provider to mock geolocation
+            # responses in component build. Should not be enabled
+            # for release build. If you need to test with chromium's
+            # location provider, remove this definition.
+            'OVERRIDE_LOCATION_PROVIDER',
+          ],
         }],
         ['OS=="win"', {
           'sources': [

filenames.gypi

@@ -680,6 +680,10 @@
       'chromium_src/chrome/utility/printing_handler_win.cc',
       'chromium_src/chrome/utility/printing_handler_win.h',
     ],
+    'lib_sources_location_provider': [
+      'atom/browser/fake_location_provider.cc',
+      'atom/browser/fake_location_provider.h',
+    ],
     'framework_sources': [
       'atom/app/atom_library_main.h',
       'atom/app/atom_library_main.mm',

spec/chromium-spec.js

@@ -226,6 +226,47 @@ describe('chromium feature', () => {
     })
   })
 
+  describe('navigator.geolocation', () => {
+    before(function () {
+      if (!features.isFakeLocationProviderEnabled()) {
+        return this.skip()
+      }
+    })
+
+    it('returns position when permission is granted', (done) => {
+      navigator.geolocation.getCurrentPosition((position) => {
+        assert(position.timestamp)
+        done()
+      }, (error) => {
+        done(error)
+      })
+    })
+
+    it('returns error when permission is denied', (done) => {
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          partition: 'geolocation-spec'
+        }
+      })
+      w.webContents.on('ipc-message', (event, args) => {
+        if (args[0] === 'success') {
+          done()
+        } else {
+          done('unexpected response from geolocation api')
+        }
+      })
+      w.webContents.session.setPermissionRequestHandler((wc, permission, callback) => {
+        if (permission === 'geolocation') {
+          callback(false)
+        } else {
+          callback(true)
+        }
+      })
+      w.loadURL(`file://${fixtures}/pages/geolocation/index.html`)
+    })
+  })
+
   describe('window.open', () => {
     it('returns a BrowserWindowProxy object', () => {
       const b = window.open('about:blank', '', 'show=no')

spec/fixtures/pages/geolocation/index.html

@@ -0,0 +1,12 @@
+<script>
+  const ipcRenderer = require('electron').ipcRenderer;
+  navigator.geolocation.getCurrentPosition((position) => {
+    ipcRenderer.send(position);
+  }, (error) => {
+    if (error) {
+      ipcRenderer.send('success')
+    } else {
+      ipcRenderer.send('unexpected error')
+    }
+  })
+</script>