Handle invisible to visible change on Geolocation::PageVisibilityChanged

This CL is split from https://chromium-review.googlesource.com/c/chromium/src/+/3449477 This CL changes Geolocation::PageVisibilityChanged so that it would care about invisible->visible cases. Before this CL, the code path assumed visible->invisible case, where it only needed to shutdown the notifier updates, which didn't require UpdateGeolocationConnection calls with actual notifier pointers. This CL updates it to loop for all notifiers registered, so that it can setup the timers in invisible->visible case. Bug: 626703 Change-Id: I32aa48837157d181d3348528b853b4f7b6af3b99 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3470643 Reviewed-by: Kentaro Hara <[email protected]> Commit-Queue: Kouhei Ueno <[email protected]> Auto-Submit: Kouhei Ueno <[email protected]> Cr-Commit-Position: refs/heads/main@{#972926}
scheib · Feb 18, 2022 · 85f8752 · 85f8752
1 parent a664737
commit 85f8752
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 1 deletion.
diff --git a/third_party/blink/renderer/modules/geolocation/geolocation.cc b/third_party/blink/renderer/modules/geolocation/geolocation.cc
@@ -512,7 +512,13 @@ void Geolocation::OnPositionUpdated(
 }
 
 void Geolocation::PageVisibilityChanged() {
-  UpdateGeolocationConnection(nullptr);
+  for (auto& notifier : *one_shots_)
+    UpdateGeolocationConnection(notifier);
+
+  HeapVector<Member<GeoNotifier>> watchers;
+  watchers_->CopyNotifiersToVector(watchers);
+  for (auto& notifier : watchers)
+    UpdateGeolocationConnection(notifier);
 }
 
 bool Geolocation::HasPendingActivity() const {

diff --git a/...rty/blink/web_tests/wpt_internal/geolocation-api/getCurrentPosition_visibility.https.html b/...rty/blink/web_tests/wpt_internal/geolocation-api/getCurrentPosition_visibility.https.html
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<meta charset='utf-8'>
+<title>Geolocation Test: getCurrentPosition returns after visibilityState changes</title>
+<link rel='help' href='http://www.w3.org/TR/geolocation-API/#get-current-position'>
+<script src='/resources/testharness.js'></script>
+<script src='/resources/testharnessreport.js'></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+
+<p>Clear all Geolocation permissions before running this test. If prompted for permission, please allow.</p>
+<div id='log'></div>
+
+<script>
+setup(test_driver.set_permission({name: 'geolocation'}, 'granted', true));
+
+promise_test(async t => {
+  assert_equals(document.visibilityState, "visible");
+
+  const gcp = new Promise(resolve => navigator.geolocation.getCurrentPosition(
+    p => resolve(pos),
+    e => resolve(e),
+    {timeout: 100, maximumAge: 0}))
+  await gcp;
+
+  const changed = new Promise(resolve => document.addEventListener("visibilitychange", resolve, {once:true}));
+  testRunner.setMainWindowHidden(true);
+  await changed;
+  assert_equals(document.visibilityState, "hidden");
+
+  const gcp2 = new Promise(resolve => navigator.geolocation.getCurrentPosition(
+    p => resolve(pos),
+    e => resolve(e),
+    {timeout: 100, maximumAge: 0}));
+
+  const changed2 = new Promise(resolve => document.addEventListener("visibilitychange", resolve, {once:true}));
+  testRunner.setMainWindowHidden(false);
+  await changed2;
+  assert_equals(document.visibilityState, "visible");
+
+  await gcp2;
+}, "getCurrentPosition returns after visibilityState changes");
+</script>