[webkit-reviews] review granted: [Bug 181749] Service Workers restored from persistent storage have 'redundant' state : [Attachment 331519] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 181749: Service Workers restored from persistent storage have 'redundant'
state
https://bugs.webkit.org/show_bug.cgi?id=181749

Attachment 331519: Patch

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




--- Comment #4 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 331519
  --> https://bugs.webkit.org/attachment.cgi?id=331519
Patch

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

> Source/WebCore/workers/service/server/SWServer.cpp:493
> +	   worker->setState(ServiceWorkerState::Activated);

We could assert in updateRegistrationState that the worker state is
Activated/Activating if setState was called before updateRegistrationState.

> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:216
> +	       return;

If we move this closer to
WebProcess::singleton().webLoaderStrategy().scheduleLoadFromNetworkProcess, we
would have twice the same 3 lines of code.
We could then wrap them in a single function.

> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:230
>      LOG(NetworkScheduling, "(WebProcess) WebLoaderStrategy::scheduleLoad,
url '%s' will be scheduled through ServiceWorker handle fetch algorithm",
resourceLoader.url().string().latin1().data());

This is preexisting but maybe we could make the message clearer.
Current message can be read like the load will be done through Service Worker
while it will actually go to the NetworkProcess.