1148064 - Enable interception of pings through service workers

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

This is similar to bug 1147695 but for pings.

Comment 1

[(no longer active)]

Attachment: Enable interception of pings through service workers

Currently when sending a beacon, HttpBaseChannel::ShouldIntercept tries
to get access to the nsINetworkInterceptController interface through the
channel's notification callbacks, but in this case the notification
callback is the nsPingListener object.

This patch extends nsPingListener to make it aware of
nsINetworkInterceptController, and have it route the request for
nsINetworkInterceptController correctly to the docshell without the need
to mess with the notification callbacks.

This will be tested in bug 1147699.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8584017 [details] [diff] [review]
Enable interception of pings through service workers

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

Please pass this by someone who knows docshell once.

::: docshell/base/nsDocShell.cpp
@@ +531,5 @@
> +  if (aIID.Equals(NS_GET_IID(nsINetworkInterceptController)) &&
> +      mInterceptController) {
> +    nsCOMPtr<nsINetworkInterceptController> copy(mInterceptController);
> +    *aResult = copy.forget().take();
> +    return NS_OK;

Why not use the same pattern as for channel event sink?

Comment 3

[(no longer active)]

(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #2)
> ::: docshell/base/nsDocShell.cpp
> @@ +531,5 @@
> > +  if (aIID.Equals(NS_GET_IID(nsINetworkInterceptController)) &&
> > +      mInterceptController) {
> > +    nsCOMPtr<nsINetworkInterceptController> copy(mInterceptController);
> > +    *aResult = copy.forget().take();
> > +    return NS_OK;
> 
> Why not use the same pattern as for channel event sink?

Because I'd prefer us to move away from calling AddRef and Release manually...  But you have a good point on the inconsistency, so I'll change the other pattern.  :-)

Comment 4

[(no longer active)]

Attachment: Enable interception of pings through service workers

Currently when sending a beacon, HttpBaseChannel::ShouldIntercept tries
to get access to the nsINetworkInterceptController interface through the
channel's notification callbacks, but in this case the notification
callback is the nsPingListener object.

This patch extends nsPingListener to make it aware of
nsINetworkInterceptController, and have it route the request for
nsINetworkInterceptController correctly to the docshell without the need
to mess with the notification callbacks.

This will be tested in bug 1147699.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8585103 [details] [diff] [review]
Enable interception of pings through service workers

Could we mark SendPingInfo to be stack class?

Comment 6

[(no longer active)]

Of course!

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aca9840cf06f