The bundle identifier gets passed to the network process when it is initialized. If a test changes the bundle identifier for some purpose, it should make sure it initializes a new network process.
<rdar://problem/63539061>
Created attachment 400086 [details] Patch
Comment on attachment 400086 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400086&action=review > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:276 > + terminateNetworkProcess(); Why cannot you update the application bundle identifier in the existing Network Process? The current _setApplicationBundleIdentifier SPI in on the view. How can you guarantee that the network process has not launched yet when you call _setApplicationBundleIdentifier ? This seems like a super fragile SPI. I don't think that terminating a network process is a nice way to deal with this.
(In reply to Chris Dumez from comment #3) > Comment on attachment 400086 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=400086&action=review > > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:276 > > + terminateNetworkProcess(); > > Why cannot you update the application bundle identifier in the existing > Network Process? The current _setApplicationBundleIdentifier SPI in on the > view. How can you guarantee that the network process has not launched yet > when you call _setApplicationBundleIdentifier ? This seems like a super > fragile SPI. > > I don't think that terminating a network process is a nice way to deal with > this. Do you mean send IPC to the Network Process to update the bundleID there when _setApplicationBundleIdentifier is called?
(In reply to katherine_cheney from comment #4) > (In reply to Chris Dumez from comment #3) > > Comment on attachment 400086 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=400086&action=review > > > > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:276 > > > + terminateNetworkProcess(); > > > > Why cannot you update the application bundle identifier in the existing > > Network Process? The current _setApplicationBundleIdentifier SPI in on the > > view. How can you guarantee that the network process has not launched yet > > when you call _setApplicationBundleIdentifier ? This seems like a super > > fragile SPI. > > > > I don't think that terminating a network process is a nice way to deal with > > this. > > Do you mean send IPC to the Network Process to update the bundleID there > when _setApplicationBundleIdentifier is called? Yes.
(In reply to Chris Dumez from comment #5) > (In reply to katherine_cheney from comment #4) > > (In reply to Chris Dumez from comment #3) > > > Comment on attachment 400086 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=400086&action=review > > > > > > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:276 > > > > + terminateNetworkProcess(); > > > > > > Why cannot you update the application bundle identifier in the existing > > > Network Process? The current _setApplicationBundleIdentifier SPI in on the > > > view. How can you guarantee that the network process has not launched yet > > > when you call _setApplicationBundleIdentifier ? This seems like a super > > > fragile SPI. > > > > > > I don't think that terminating a network process is a nice way to deal with > > > this. > > > > Do you mean send IPC to the Network Process to update the bundleID there > > when _setApplicationBundleIdentifier is called? > > Yes. Seems wrong for a method on the view to behave differently whether the network process is already launched or not, even if it is SPI.
(In reply to Chris Dumez from comment #6) > (In reply to Chris Dumez from comment #5) > > (In reply to katherine_cheney from comment #4) > > > (In reply to Chris Dumez from comment #3) > > > > Comment on attachment 400086 [details] > > > > Patch > > > > > > > > View in context: > > > > https://bugs.webkit.org/attachment.cgi?id=400086&action=review > > > > > > > > > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:276 > > > > > + terminateNetworkProcess(); > > > > > > > > Why cannot you update the application bundle identifier in the existing > > > > Network Process? The current _setApplicationBundleIdentifier SPI in on the > > > > view. How can you guarantee that the network process has not launched yet > > > > when you call _setApplicationBundleIdentifier ? This seems like a super > > > > fragile SPI. > > > > > > > > I don't think that terminating a network process is a nice way to deal with > > > > this. > > > > > > Do you mean send IPC to the Network Process to update the bundleID there > > > when _setApplicationBundleIdentifier is called? > > > > Yes. > > Seems wrong for a method on the view to behave differently whether the > network process is already launched or not, even if it is SPI. Agreed, I didn't think about that. I'll fix it.
Created attachment 400278 [details] Patch
Created attachment 400282 [details] Patch
Created attachment 400288 [details] Patch
Comment on attachment 400288 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400288&action=review r=me with fixes. > Source/WebKit/ChangeLog:9 > + No new tests, will fix http/tests/in-app-browser-privacy/ tests. The part about tests usually comes after the description, not before. > Source/WebKit/NetworkProcess/NetworkProcess.cpp:2757 > +void NetworkProcess::updateBundleIdentifierInNetworkProcess(const String&& bundleIdentifier, CompletionHandler<void()>&& completionHandler) 'String&&' or 'const String&', but not 'const String&&'. Here I would just use String&&. > Source/WebKit/NetworkProcess/NetworkProcess.messages.in:187 > + UpdateBundleIdentifierInNetworkProcess(String bundleIdentifier) -> () Async Please strip 'InNetworkProcess'. This is an IPC to the network process so it is redundant. > Source/WebKit/NetworkProcess/NetworkProcess.messages.in:188 > + ClearBundleIdentifierInNetworkProcess() -> () Async ditto. > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1540 > +void NetworkProcessProxy::updateBundleIdentifierInNetworkProcess(const String&& bundleIdentifier, CompletionHandler<void()>&& completionHandler) const String&, not const String&&. > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:2488 > +void WebsiteDataStore::updateBundleIdentifierInNetworkProcess(String bundleIdentifier, CompletionHandler<void()>&& completionHandler) String -> const String& > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:2493 > + processPool->ensureNetworkProcess().updateBundleIdentifierInNetworkProcess(WTFMove(bundleIdentifier), [callbackAggregator = callbackAggregator.copyRef()] { }); use-after-move of bundleIdentifier if there is more than one loop iteration. Just use bundleIdentifier without moving.
Comment on attachment 400288 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400288&action=review Thanks! I'll make these changes before landing. >> Source/WebKit/ChangeLog:9 >> + No new tests, will fix http/tests/in-app-browser-privacy/ tests. > > The part about tests usually comes after the description, not before. Will fix. >> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:2493 >> + processPool->ensureNetworkProcess().updateBundleIdentifierInNetworkProcess(WTFMove(bundleIdentifier), [callbackAggregator = callbackAggregator.copyRef()] { }); > > use-after-move of bundleIdentifier if there is more than one loop iteration. Just use bundleIdentifier without moving. Oops, good catch.
Created attachment 400369 [details] Patch for landing
Committed r262212: <https://trac.webkit.org/changeset/262212> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400369 [details].