Bug 179192 – Use a single identifier type to identify Service Workers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Use a String identifier consistently to identify Service Workers, as per the specification. We currently have both a String identifier and a uint64_t identifier for each service worker.

[Chris Dumez]

Created attachment 325751 [details]
Patch

[Brady Eidson]

Comment on attachment 325751 [details]
Patch

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

> Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.h:46
> +    const String& identifier() const { return m_serviceWorkerThread->identifier(); }

Who calls this? If there's a way to get rid of callers to this, then we don't need to worry.

If we truly do need to maintain this accessor, then we have two options:
1 - verify it is only used on a given thread and ASSERT it's only called on that thread going forward
2 - make it return an isolated copy each time. :(

[Build Bot]

Comment on attachment 325751 [details]
Patch

Attachment 325751 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5079447

New failing tests:
svg/wicd/test-rightsizing-a.xhtml
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-canvas-tainting.https.html

[Build Bot]

Created attachment 325763 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Chris Dumez]

Created attachment 325841 [details]
Patch

[Chris Dumez]

Created attachment 325842 [details]
Patch

[Chris Dumez]

Comment on attachment 325842 [details]
Patch

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

> Source/WTF/wtf/ObjectIdentifier.h:34
> +    ObjectIdentifier() = default;

Unfortunately, the IPC code currently requires me to have a default constructor. See error at https://pastebin.com/vNGBKWVK.

[Chris Dumez]

Created attachment 325846 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 44 files


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

[Chris Dumez]

Created attachment 325847 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Created attachment 325849 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Created attachment 325850 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Created attachment 325852 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Created attachment 325853 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Created attachment 325856 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Comment on attachment 325856 [details]
Patch

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

> Source/WTF/wtf/ObjectIdentifier.h:34
> +    ObjectIdentifier() = default;

Unfortunately, the IPC code currently requires me to have a default constructor. See error at https://pastebin.com/vNGBKWVK.

[Brady Eidson]

Comment on attachment 325856 [details]
Patch

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

> Source/WebCore/workers/service/ServiceWorkerIdentifier.h:34
> +enum ServiceWorkerType { };

Maybe ServiceWorkerIdentifierType to reduce the possibility of collisions in the future?

[Chris Dumez]

Created attachment 325902 [details]
Patch

[Build Bot]

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


ERROR: Source/WTF/wtf/ObjectIdentifier.h:36:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:75:  Missing spaces around :  [whitespace/init] [4]
Total errors found: 2 in 47 files


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

[Chris Dumez]

Committed r224403: <https://trac.webkit.org/changeset/224403>

[Radar WebKit Bug Importer]

<rdar://problem/35567415>