perform service worker interception in child or parent process based on a pref

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P2
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

(Blocks 1 bug)

unspecified
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(5 attachments, 8 obsolete attachments)

7.46 KB, patch
valentin
: review+
Details | Diff | Splinter Review
13.01 KB, patch
bkelly
: review+
Details | Diff | Splinter Review
3.33 KB, patch
asuth
: review+
Details | Diff | Splinter Review
2.04 KB, patch
asuth
: review+
Details | Diff | Splinter Review
8.95 KB, patch
asuth
: review+
Details | Diff | Splinter Review
As a step towards full parent-side interception (bug 1231222) we are going to add a preference that will toggle either child-or-parent interception.  At first this will be switch to "child" since parent interception will not be fully correct.

This bug requires the patches in bug 1231211 to separate the service worker interception from the docshell innards.
Priority: -- → P2
Posted patch wip (obsolete) — Splinter Review
This work-in-progress does parent-side intercept and works for me in light browsing locally.  Issue to solve in later bugs:

1. It executes the service worker in the parent process itself right now. (bug 1231213)
2. Console messages from the service worker thread do not appears in the window's devtools toolbox. (bug 1429805)
3. Network monitor doesn't see requests from the service worker thread.  (bug 1432311)
Posted patch wip (obsolete) — Splinter Review
Blocks: 1432640
This try build is checking client-side intercept.  Tests will fail miserably with parent-side intercept right now.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfe893a6d475a05b0da22c504c3ac2ccae1da7c2
Comment on attachment 8944867 [details] [diff] [review]
P1 Send mCorsMode, mRedirectMode, and mFetchCacheMode to HttpChannelParent. r=valentin

Valentin, this patch serializes a few more values we need to create the FetchEvent.  I forgot them in the last bug, sorry.
Attachment #8944867 - Flags: review?(valentin.gosu)
Attachment #8944867 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8944868 [details] [diff] [review]
P2 Surface the ApplyConversion state from HttpChannelParent to HttpChannelChild. r=valentin

Service worker interception provides decoded data when it synthesizes a response on the channel.  We handle this by calling SetApplyConversion(false) when we synthesize.  If we are doing the synthesis in the parent we need to propagate the ApplyConversion flag back to the child process.

Note, I also removed the QI to nsIEncodingChannel.  Its not necessary as we already early exit above if we cannot do_QueryObject() to HttpBaseChannel above:

https://searchfox.org/mozilla-central/rev/e3cba77cee3ff1be38313abe9c804d13c51bd95b/netwerk/protocol/http/HttpChannelParent.cpp#1427

We can just work with the HttpBaseChannel directly instead.
Attachment #8944868 - Flags: review?(valentin.gosu)
Attachment #8944868 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8944868 [details] [diff] [review]
P2 Surface the ApplyConversion state from HttpChannelParent to HttpChannelChild. r=valentin

Review of attachment 8944868 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/HttpChannelParent.cpp
@@ +1490,5 @@
>      }
>    }
>  
> +  // Propagate whether or not conversion should occur from the parent-side
> +  // channel to the child-side channel.  The disable the parent-side

nit: Then disable...
Andrew, this patch adds a new service worker utility method to determine if parent-side intercept is enabled.  Currently I only opportunistically create the pref observer, so it depends on the main thread.  If we need OMT access in the future we could have ServiceWorkerRegistrar::Initialize() set it up and then use an atomic bool.  I chose not to do that for now, though, because that required the pref to be configured as an "early pref".

Note, I'm specifically *not* adding this to all.js for now.  I don't really want to advertise the existence of this preference since its not safe to use for normal browsing.  Not only is a bunch of stuff broken, but it runs content scripts in the parent process.  Lets keep it somewhat hidden for now.
Attachment #8944889 - Attachment is obsolete: true
Attachment #8945103 - Flags: review?(bugmail)
This patch loosens some restrictions on load group when dispatching a FetchEvent.  This code was there to help enforce b2g security checks that no longer exist.  Bug 1125961 has been marked WONTFIX.

I still leave the load group overriding stuff in place for now, though.  We just do it opportunistically.  I filed bug 1432184 to clean up the load group stuff later.
Attachment #8944896 - Attachment is obsolete: true
Comment on attachment 8945104 [details] [diff] [review]
P4 Don't require a load group when spawning the service worker thread for a FetchEvent. r=asuth

See comment 18.
Attachment #8945104 - Flags: review?(bugmail)
This patch makes us create the ServiceWorkerInterceptController in HttpChannelParentListener instead of the docshell when parent intercept is enabled.

Note, HttpChannelParent was also setting a LOAD_BYPASS_SERVICEWORKER flag to avoid any spurious interceptions for child-side intercept.  We also have to disable that behavior since we do actually want the parent-side intercept now.
Attachment #8944901 - Attachment is obsolete: true
Attachment #8945108 - Flags: review?(bugmail)
Attachment #8945103 - Flags: review?(bugmail) → review+
Attachment #8945104 - Flags: review?(bugmail) → review+
Attachment #8945108 - Flags: review?(bugmail) → review+

Comment 22

Last year
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/678870d85430
P1 Send mCorsMode, mRedirectMode, and mFetchCacheMode to HttpChannelParent. r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/9274ff8934e0
P2 Surface the ApplyConversion state from HttpChannelParent to HttpChannelChild. r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/c540f53d730b
P3 Add ServiceWorkerParentInterceptEnabled() method and backing pref. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4f3e57a1de6
P4 Don't require a load group when spawning the service worker thread for a FetchEvent. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/27dc825e05aa
P5 Perform service worker interception in the parent if ServiceWorkerParentInterceptEnable() returns true. r=asuth
Depends on: 1433641
No longer depends on: 1433641
Depends on: 1439879
Blocks: 1456995
You need to log in before you can comment on or make changes to this bug.