Bug 192951

Summary: navigator.userAgent in service workers does not reflect customUserAgent set by client
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Service WorkersAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, commit-queue, dbates, esprehn+autocc, ews-watchlist, ggaren, japhet, kangil.han, megan_gardner, thorton, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 192952    
Bug Blocks:    
Attachments:
Description Flags
API test
none
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2018-12-20 13:09:19 PST
navigator.userAgent in service workers does not reflect customUserAgent set by client.
Attachments
API test (3.87 KB, patch)
2018-12-20 13:25 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (38.36 KB, patch)
2018-12-21 13:09 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (34.23 KB, patch)
2018-12-21 15:54 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (34.47 KB, patch)
2018-12-21 19:06 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2018-12-20 13:25:04 PST
Created attachment 357857 [details] API test
Tim Horton
Comment 2 2018-12-20 17:49:23 PST
If you fix this, can you fix navigator.platform simultaneously?
Chris Dumez
Comment 3 2018-12-21 13:09:09 PST
Created attachment 357971 [details] Patch
youenn fablet
Comment 4 2018-12-21 14:14:18 PST
I have some worries about trying to reuse a service worker with different user agents. Here are some suggestions if we want to go down that path. Instead of tieing the user agent with fetch, I would pass it as part of registering a service worker client (see DocumentLoader::registerTemporaryServiceWorkerClient). Whenever a new service worker client is registered, it would update the user agent of the corresponding SWServerWorker. That should work well if the corresponding service worker is not running. For an existing running service worker, I am not sure what is the best option. A message to the service worker instance could be sent to notify the user agent has changed and update its value. Or we could notify the given client to bypass the service worker. I wonder whether adding some logging might help future debugging in breakage caused by user agent changes. To properly detect all these cases, we would need to persist the user agent in the database, probably overkill.
Chris Dumez
Comment 5 2018-12-21 15:54:10 PST
Created attachment 357994 [details] Patch
youenn fablet
Comment 6 2018-12-21 16:46:36 PST
Comment on attachment 357994 [details] Patch I wonder whether we should release_log the case of a client registered to a running service worker with a different user agent. Maybe we could store the user agent in the SWServerWorker and when registering a client, check the user agent of the active worker if any? View in context: https://bugs.webkit.org/attachment.cgi?id=357994&action=review > Source/WebCore/workers/service/server/SWServer.cpp:743 > +void SWServer::registerServiceWorkerClient(ClientOrigin&& clientOrigin, ServiceWorkerClientData&& data, const Optional<ServiceWorkerRegistrationIdentifier>& controllingServiceWorkerRegistrationIdentifier, const String& userAgent) String&& > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:138 > +void WebSWContextManagerConnection::installServiceWorker(const ServiceWorkerContextData& data, SessionID sessionID, const String& userAgent) String&&
Chris Dumez
Comment 7 2018-12-21 19:06:12 PST
Created attachment 358012 [details] Patch
WebKit Commit Bot
Comment 8 2018-12-21 19:23:18 PST
Comment on attachment 358012 [details] Patch Clearing flags on attachment: 358012 Committed r239534: <https://trac.webkit.org/changeset/239534>
WebKit Commit Bot
Comment 9 2018-12-21 19:23:19 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2018-12-21 19:24:27 PST
<rdar://problem/46914755>
Note You need to log in before you can comment on or make changes to this bug.