Bug 238800 - In case of COOP-based process swap, we need to make sure the document gets controlled by its matching service worker registration
Summary: In case of COOP-based process swap, we need to make sure the document gets co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Service Workers (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on: 238738
Blocks:
  Show dependency tree / graph
 
Reported: 2022-04-05 05:41 PDT by youenn fablet
Modified: 2022-05-26 14:59 PDT (History)
4 users (show)

See Also:

Attachments
Patch (12.76 KB, patch)
2022-04-05 09:06 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2022-04-05 05:41:36 PDT 
In case of COOP-based process swap, we need to make sure the document gets controlled by its matching service worker registration
Comment 1 Radar WebKit Bug Importer 2022-04-05 05:41:56 PDT 
<rdar://problem/91288849>
Comment 2 youenn fablet 2022-04-05 09:06:25 PDT 
Created attachment 456705 [details]
Patch
Comment 3 Chris Dumez 2022-04-06 07:19:40 PDT 
Comment on attachment 456705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456705&action=review

r=me

> Source/WebKit/NetworkProcess/NetworkLoadParameters.h:47
> +    mutable WebPageProxyIdentifier webPageProxyID;

These mutable seem a little odd. Maybe we should imply not mark the NetworkResourceLoader data member as const?
Comment 4 youenn fablet 2022-04-06 08:09:48 PDT 
(In reply to Chris Dumez from comment #3)
> Comment on attachment 456705 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456705&action=review
> 
> r=me
> 
> > Source/WebKit/NetworkProcess/NetworkLoadParameters.h:47
> > +    mutable WebPageProxyIdentifier webPageProxyID;
> 
> These mutable seem a little odd. Maybe we should imply not mark the
> NetworkResourceLoader data member as const?

There used to be no mutable, I guess we should remove them, and const_cast m_parameters when needed instead. Or make the member no longer const.
Comment 5 youenn fablet 2022-04-06 08:11:02 PDT 
> There used to be no mutable, I guess we should remove them, and const_cast
> m_parameters when needed instead. Or make the member no longer const.

I'll look into cleaning all these mutables in a follow-up.
Comment 6 Chris Dumez 2022-04-06 08:13:42 PDT 
(In reply to youenn fablet from comment #4)
> (In reply to Chris Dumez from comment #3)
> > Comment on attachment 456705 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=456705&action=review
> > 
> > r=me
> > 
> > > Source/WebKit/NetworkProcess/NetworkLoadParameters.h:47
> > > +    mutable WebPageProxyIdentifier webPageProxyID;
> > 
> > These mutable seem a little odd. Maybe we should imply not mark the
> > NetworkResourceLoader data member as const?
> 
> There used to be no mutable, I guess we should remove them, and const_cast
> m_parameters when needed instead. Or make the member no longer const.

Right, I think keeping the m_parameters const at this point no longer makes sense since we're updating its data member. Mutable has some use cases (e.g. lazy init) but IMO this isn't a good use case for it.
Comment 7 EWS 2022-04-06 08:21:59 PDT 
Committed r292468 (249319@main): <https://commits.webkit.org/249319@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456705 [details].
Comment 8 Brent Fulgham 2022-05-26 14:59:30 PDT 
This fix shipped with Safari 15.5 (all platforms).