Bug 180566 – Service Worker should use a correct user agent

• Component: Service Workers
• Status: RESOLVED FIXED

[youenn fablet]

Currently, it is the empty string

[Radar WebKit Bug Importer]

<rdar://problem/35926295>

[youenn fablet]

Created attachment 328861 [details]
Patch

[EWS Watchlist]

Attachment 328861 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h:91:  The parameter name "userAgent" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:74:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 328870 [details]
Patch

[EWS Watchlist]

Attachment 328870 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h:90:  The parameter name "userAgent" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:74:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 328877 [details]
Patch

[EWS Watchlist]

Attachment 328877 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h:90:  The parameter name "userAgent" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:74:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 328870 [details]
Patch

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:2355
> +    process().processPool().updateServiceWorkerUserAgent(userAgent());

Seems like it would be more robust if this were in WebPageProxy::setUserAgent(), no?
Also, this looks hackish so I think it deserves a FIXME comment.

> Source/WebKit/UIProcess/WebProcessPool.cpp:1030
> +    updateServiceWorkerUserAgent(pageProxy->userAgent());

Won't this temporarily reset the SW user agent to some default one until WebPageProxy::setUserAgent() gets called with the proper one?

Given that the SW process is shared between all pages, I slightly worry about frequent changes to the SW process user agent (especially with sometimes a wrong user agent). A ServiceWorker may get spawned at any time, and one it is spawned, its user agent persists. Therefore, if it ended up using a bad user agent (bad luck), it is stuck with this bad user agent.

[youenn fablet]

Created attachment 328882 [details]
Patch

[EWS Watchlist]

Attachment 328882 [details] did not pass style-queue:


ERROR: Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h:90:  The parameter name "userAgent" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:74:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 328882 [details]
Patch

Clearing flags on attachment: 328882

Committed r225716: <https://trac.webkit.org/changeset/225716>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[youenn fablet]

Reopening to attach new patch.

[youenn fablet]

Created attachment 328901 [details]
Addendum

[youenn fablet]

(In reply to youenn fablet from comment #14)
> Created attachment 328901 [details]
> Addendum

We actually need to set the user agent when starting the SW for Safari.

[WebKit Commit Bot]

Comment on attachment 328901 [details]
Addendum

Clearing flags on attachment: 328901

Committed r225720: <https://trac.webkit.org/changeset/225720>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.