[webkit-reviews] review granted: [Bug 212288] Make sure bundle identifier testing override is set in the network process : [Attachment 400288] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted katherine_cheney at apple.com's request
for review:
Bug 212288: Make sure bundle identifier testing override is set in the network
process
https://bugs.webkit.org/show_bug.cgi?id=212288

Attachment 400288: Patch

https://bugs.webkit.org/attachment.cgi?id=400288&action=review




--- Comment #11 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 400288
  --> https://bugs.webkit.org/attachment.cgi?id=400288
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(WTFM
ove(bundleIdentifier), [callbackAggregator = callbackAggregator.copyRef()] {
});

use-after-move of bundleIdentifier if there is more than one loop iteration.
Just use bundleIdentifier without moving.