Bug 192951 - navigator.userAgent in service workers does not reflect customUserAgent set by client
Summary: navigator.userAgent in service workers does not reflect customUserAgent set b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Service Workers (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 192952
Blocks:
  Show dependency treegraph
 
Reported: 2018-12-20 13:09 PST by Chris Dumez
Modified: 2018-12-21 19:24 PST (History)
13 users (show)

See Also:


Attachments
API test (3.87 KB, patch)
2018-12-20 13:25 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (38.36 KB, patch)
2018-12-21 13:09 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (34.23 KB, patch)
2018-12-21 15:54 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (34.47 KB, patch)
2018-12-21 19:06 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2018-12-20 13:09:19 PST
navigator.userAgent in service workers does not reflect customUserAgent set by client.
Comment 1 Chris Dumez 2018-12-20 13:25:04 PST
Created attachment 357857 [details]
API test
Comment 2 Tim Horton 2018-12-20 17:49:23 PST
If you fix this, can you fix navigator.platform simultaneously?
Comment 3 Chris Dumez 2018-12-21 13:09:09 PST
Created attachment 357971 [details]
Patch
Comment 4 youenn fablet 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.
Comment 5 Chris Dumez 2018-12-21 15:54:10 PST
Created attachment 357994 [details]
Patch
Comment 6 youenn fablet 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&&
Comment 7 Chris Dumez 2018-12-21 19:06:12 PST
Created attachment 358012 [details]
Patch
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2018-12-21 19:23:19 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-12-21 19:24:27 PST
<rdar://problem/46914755>