Summary: | Service Worker should use a correct user agent | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||
Component: | Service Workers | Assignee: | youenn fablet <youennf> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | achristensen, beidson, cdumez, commit-queue, dbates, ews-watchlist, ggaren, japhet, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
youenn fablet
2017-12-07 19:05:45 PST
Created attachment 328861 [details]
Patch
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.
Created attachment 328870 [details]
Patch
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.
Created attachment 328877 [details]
Patch
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.
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. Created attachment 328882 [details]
Patch
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.
Comment on attachment 328882 [details] Patch Clearing flags on attachment: 328882 Committed r225716: <https://trac.webkit.org/changeset/225716> All reviewed patches have been landed. Closing bug. Reopening to attach new patch. Created attachment 328901 [details]
Addendum
(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. Comment on attachment 328901 [details] Addendum Clearing flags on attachment: 328901 Committed r225720: <https://trac.webkit.org/changeset/225720> All reviewed patches have been landed. Closing bug. |