Bug 179192

Summary: Use a single identifier type to identify Service Workers
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, benjamin, buildbot, cmarcelo, dbates, ggaren, japhet, rniwa, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Chris Dumez
Reported 2017-11-02 12:41:19 PDT
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.
Attachments
Patch (59.21 KB, patch)
2017-11-02 12:46 PDT, Chris Dumez
buildbot: commit-queue-
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.63 MB, application/zip)
2017-11-02 13:46 PDT, Build Bot
no flags
Patch (87.70 KB, patch)
2017-11-02 21:42 PDT, Chris Dumez
no flags
Patch (85.62 KB, patch)
2017-11-02 21:43 PDT, Chris Dumez
no flags
Patch (86.13 KB, patch)
2017-11-02 21:51 PDT, Chris Dumez
no flags
Patch (91.68 KB, patch)
2017-11-02 21:54 PDT, Chris Dumez
no flags
Patch (92.30 KB, patch)
2017-11-02 22:02 PDT, Chris Dumez
no flags
Patch (92.69 KB, patch)
2017-11-02 22:09 PDT, Chris Dumez
no flags
Patch (92.70 KB, patch)
2017-11-02 22:14 PDT, Chris Dumez
no flags
Patch (92.73 KB, patch)
2017-11-02 22:22 PDT, Chris Dumez
no flags
Patch (92.73 KB, patch)
2017-11-02 22:40 PDT, Chris Dumez
no flags
Patch (92.76 KB, patch)
2017-11-03 09:25 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2017-11-02 12:46:47 PDT
Brady Eidson
Comment 2 2017-11-02 13:04:38 PDT
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 3 2017-11-02 13:46:39 PDT
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
Comment 4 2017-11-02 13:46:40 PDT
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
Comment 5 2017-11-02 21:42:28 PDT
Chris Dumez
Comment 6 2017-11-02 21:43:31 PDT
Chris Dumez
Comment 7 2017-11-02 21:44:15 PDT
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
Comment 8 2017-11-02 21:51:30 PDT
Build Bot
Comment 9 2017-11-02 21:53:11 PDT
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
Comment 10 2017-11-02 21:54:58 PDT
Build Bot
Comment 11 2017-11-02 21:56:15 PDT
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
Comment 12 2017-11-02 22:02:59 PDT
Build Bot
Comment 13 2017-11-02 22:05:19 PDT
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
Comment 14 2017-11-02 22:09:50 PDT
Build Bot
Comment 15 2017-11-02 22:12:32 PDT
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
Comment 16 2017-11-02 22:14:19 PDT
Build Bot
Comment 17 2017-11-02 22:15:24 PDT
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
Comment 18 2017-11-02 22:22:11 PDT
Build Bot
Comment 19 2017-11-02 22:24:21 PDT
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
Comment 20 2017-11-02 22:40:17 PDT
Build Bot
Comment 21 2017-11-02 22:42:37 PDT
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 22 2017-11-02 22:56:39 PDT
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 23 2017-11-03 09:21:06 PDT
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
Comment 24 2017-11-03 09:25:46 PDT
Build Bot
Comment 25 2017-11-03 09:27:23 PDT
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
Comment 26 2017-11-03 09:37:10 PDT
Radar WebKit Bug Importer
Comment 27 2017-11-15 12:22:40 PST
Note You need to log in before you can comment on or make changes to this bug.