[webkit-reviews] review granted: [Bug 223200] Service worker soft-update loads not being marked app-bound : [Attachment 425310] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted katherine_cheney at apple.com's
request for review:
Bug 223200: Service worker soft-update loads not being marked app-bound
https://bugs.webkit.org/show_bug.cgi?id=223200

Attachment 425310: Patch

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




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

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

> Source/WebCore/workers/service/server/SWServerRegistration.h:111
> +    void setIsAppBound(bool isAppBound) { m_isAppBound = isAppBound; }

This setter does not seem needed given we set m_isAppBound in
scheduleSoftUpdate.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:1764
> +	   { "application/javascript", "" },

As I see it, the service worker script is empty so the second web page load
will go to the network process.
I think it would be better if we were using the following js:
"self.addEventListener('fetch', (event) => { event.respondWith(new Response(new
Blob(['<script>alert(\"synthetic response\")</script>'], {type:
'text/html'}))); })"

For the second load page, we would have something like:
EXPECT_WK_STREQ([webView _test_waitForAlert], "synthetic response");

> Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:1794
> +    [webView _appBoundNavigationData: ^(struct
WKAppBoundNavigationTestingData data) {

This might be racy once only the soft update load happens given there is a
timer of 1 second between the scheduling of the soft update and the actual soft
update.