Bug 147785 - Page cache doesn't work for pages actively using Geolocation
Summary: Page cache doesn't work for pages actively using Geolocation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-08-07 10:12 PDT by Chris Dumez
Modified: 2015-08-09 21:14 PDT (History)
4 users (show)

See Also:


Attachments
Patch (12.06 KB, patch)
2015-08-07 11:23 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (11.18 KB, patch)
2015-08-07 20:47 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-08-07 10:12:48 PDT
Page cache doesn't work for pages actively using Geolocation.
Comment 1 Chris Dumez 2015-08-07 10:12:57 PDT
rdar://problem/11147901
Comment 2 Chris Dumez 2015-08-07 11:08:13 PDT
rdar://problem/11147901
Comment 3 Chris Dumez 2015-08-07 11:23:20 PDT
Created attachment 258515 [details]
Patch
Comment 4 Chris Dumez 2015-08-07 20:47:19 PDT
Created attachment 258557 [details]
Patch
Comment 5 Chris Dumez 2015-08-07 20:51:48 PDT
I also tested manually with Google Maps on iOS and:
1. The page goes into the PageCache
2. When navigating back, no permission request is done, the location on the map keeps updating
3. If the location permissions are reset in General Settings while the page is into the PageCache, we correctly ask again for permission when navigating back to Google Maps.
Comment 6 Darin Adler 2015-08-09 18:11:56 PDT
Comment on attachment 258557 [details]
Patch

No rationale in this patch. Why did we think this wasn’t OK before? Why do we know it’s OK now? Is there nothing else we need to do to make everything work correctly?
Comment 7 Chris Dumez 2015-08-09 18:25:11 PDT
(In reply to comment #6)
> Comment on attachment 258557 [details]
> Patch
> 
> No rationale in this patch. Why did we think this wasn’t OK before? Why do
> we know it’s OK now? Is there nothing else we need to do to make everything
> work correctly?

I asked Benjamin and he did not remember why he restricted this in the first place. Benjamin thought the issue may have been related to revoking location permissions on iOS but I have manually tested that this is working as expected.

The PageCaching code seems to be doing all the right things and testing (manual and layout tests) seem to work as expected.

It is possible we simply played it safe due to timing restrictions.
Comment 8 WebKit Commit Bot 2015-08-09 21:14:22 PDT
Comment on attachment 258557 [details]
Patch

Clearing flags on attachment: 258557

Committed r188204: <http://trac.webkit.org/changeset/188204>
Comment 9 WebKit Commit Bot 2015-08-09 21:14:27 PDT
All reviewed patches have been landed.  Closing bug.