Opened 6 months ago

Closed 6 months ago

Last modified 4 months ago

#6595 closed defect (fixed)

Page/Frame structure isn't updated when navigating to document served by Service Worker

Reported by: sebastian Assignee: sebastian
Priority: P3 Milestone: Adblock-Plus-3.2-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29755575

Description

How to reproduce

  1. Go to https://regex101.com/ (or any other website that caches the top-level document through Service Workers) once, to make sure the Service Worker is registered and the cache populated.
  2. Go to https://www.nytimes.com/ (or any other website that doesn't cache its document through a Service Worker; either in the same or a new tab).
  3. Open the developer tools and go to the "Adblock Plus" panel.
  4. Go to https://regex101.com/ (or whichever website you used in step 1) again.

Observed behaviour

All requests on regex101.com are matched with the document domain of nytimes.com (as indicated in the devtools panel). That is because none of the code paths where we update the page/frame structure is reached, when the request loading the document is handled by a Service Worker. In this case the onHeadersReceived listener isn't called, and in the onBeforeNavigate listener we ignore HTTP(S) URLs.

Expected behaviour

The page/frame structure must be updated when navigating to a document served by a ServiceWorker, so that requests are matched against the correct document domain.

Attachments (1)

6595-devtools.jpg (82.4 KB) - added by Ross 4 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 6 months ago by sebastian

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 6 months ago by abpbot

A commit referencing this issue has landed:
Issue 6595 - Make sure frame structure gets always updated

comment:3 Changed 6 months ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:4 Changed 4 months ago by Ross

This seemed fixed, but then I noticed that going back and forth between https://regex101.com/ and https://www.nytimes.com/ (both waiting and not waiting for page loads) eventually causes the devtools to show a request for regex101.com from the domain nytimes.com (see attached image). Is that still this bug?

Changed 4 months ago by Ross

comment:5 Changed 4 months ago by sebastian

The problem appears to be that we update the document structure on onCommitted here, which is triggered when the browser starts showing the new document, but some requests might have already been sent at this point. For documents that are served right from the web we work around that issue by updating the document structure already on onHeadersReceived, however this event isn't triggered for documents served by Service Workers. Otherwise, the change here wouldn't have been necessary in the first place. It seems this edge case is a limitation we cannot work around.

Last edited 4 months ago by sebastian (previous) (diff)

comment:6 Changed 4 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Thanks. This seems to be working as expected then.

ABP 3.1.0.2069
Chrome 67 / 64 / 49 / Windows 7
Firefox 60 / 55 / 51 / Windows 7
Opera 52 / 45 / 38 / Windows 7

Note: See TracTickets for help on using tickets.