Closed Bug 1468209 Opened 2 years ago Closed 2 years ago

Remove nsIHttpChannelInternal.responseSynthesized in favor of nsILoadInfo.serviceWorkerTaintingSynthesized

Categories

(Core :: DOM: Service Workers, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently we have two different booleans that are tracked on channels, but are always the same value:

  nsIHttpChannelInternal.responseSynthesized
  nsILoadInfo.serviceWorkerTaintingSynthesized

Lets get rid of the nsIHttpChannelInternal boolean and just use the nsILoadInfo value.

This will help green up the service worker tests with the pref flipped because its one less piece of information that needs to be propagated back to the child from the parent.
Comment on attachment 8984843 [details] [diff] [review]
Remove nsIHttpChannelInternal.responseSynthesized and use nsILoadInfo.serviceWorkerTaintingSynthesized instead. r=valentin

Valentin, this patch removes some duplicate information we are storing between the channel and the LoadInfo.  Way back when we first added nsIHttpChannelInternal.responseSynthesized to indicate if a channel was synthesized by a service worker.  We then later added nsILoadInfo.serviceWorkerTaintingSynthesized to reflect if the tainting value was set when the service worker synthesized a response.  These values should always be the same.

In order to make parent-side intercept work I need to propagate this information back to the child process.  I could propagate both values, but it would be better to remove the duplicate information and just copy one across.

So this patch removes the older nsIHttpChannelInternal.responseSynthesized in favor of the newer nsILoadInfo value.
Attachment #8984843 - Flags: review?(valentin.gosu)
Comment on attachment 8984843 [details] [diff] [review]
Remove nsIHttpChannelInternal.responseSynthesized and use nsILoadInfo.serviceWorkerTaintingSynthesized instead. r=valentin

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

::: netwerk/protocol/http/HttpBaseChannel.cpp
@@ +472,5 @@
>  NS_IMETHODIMP
>  HttpBaseChannel::SetLoadFlags(nsLoadFlags aLoadFlags)
>  {
> +  bool synthesized = mLoadInfo &&
> +                     mLoadInfo->GetServiceWorkerTaintingSynthesized();

Do we need to ensure that SetLoadInfo isn't called after SetLoadFlags?
Attachment #8984843 - Flags: review?(valentin.gosu) → review+
(In reply to Valentin Gosu [:valentin] from comment #4)
> ::: netwerk/protocol/http/HttpBaseChannel.cpp
> @@ +472,5 @@
> >  NS_IMETHODIMP
> >  HttpBaseChannel::SetLoadFlags(nsLoadFlags aLoadFlags)
> >  {
> > +  bool synthesized = mLoadInfo &&
> > +                     mLoadInfo->GetServiceWorkerTaintingSynthesized();
> 
> Do we need to ensure that SetLoadInfo isn't called after SetLoadFlags?

Actually, reading the comment it seems we may not need this at all any more.  Let me try removing this code.
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/973e2ce95b63
Remove nsIHttpChannelInternal.responseSynthesized and use nsILoadInfo.serviceWorkerTaintingSynthesized instead. r=valentin
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/973e2ce95b63
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.