Bug 196621

Summary: Add a test to check for the service worker process name
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2019-04-04 13:04:26 PDT
Add a test to check for the service worker process name
Comment 1 youenn fablet 2019-04-04 13:06:29 PDT
Created attachment 366744 [details]
Patch
Comment 2 youenn fablet 2019-04-04 13:32:24 PDT
Created attachment 366747 [details]
Patch
Comment 3 Chris Dumez 2019-04-08 13:09:56 PDT
Comment on attachment 366747 [details]
Patch

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

> Source/WebCore/testing/ServiceWorkerInternals.mm:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

2019

> Source/WebCore/testing/ServiceWorkerInternals.mm:40
> +    return "name: " + name;

Why prepend "name: " ? I think the test would look much better without.
Comment 4 youenn fablet 2019-04-08 13:14:48 PDT
(In reply to Chris Dumez from comment #3)
> Comment on attachment 366747 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366747&action=review
> 
> > Source/WebCore/testing/ServiceWorkerInternals.mm:2
> > + * Copyright (C) 2018 Apple Inc. All rights reserved.
> 
> 2019

OK

> > Source/WebCore/testing/ServiceWorkerInternals.mm:40
> > +    return "name: " + name;
> 
> Why prepend "name: " ? I think the test would look much better without.

The default implementation of processName() returns an empty string.
If the call to _LSCopyApplicationInformationItem is returning an empty string, we will fail the test as it would be "name:" and not "".
Comment 5 Chris Dumez 2019-04-08 13:30:56 PDT
Comment on attachment 366747 [details]
Patch

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

>>> Source/WebCore/testing/ServiceWorkerInternals.mm:40
>>> +    return "name: " + name;
>> 
>> Why prepend "name: " ? I think the test would look much better without.
> 
> The default implementation of processName() returns an empty string.
> If the call to _LSCopyApplicationInformationItem is returning an empty string, we will fail the test as it would be "name:" and not "".

You would fail the test either way since your layout test expects: "WebKitTestRunner Service Worker (localhost)". As it stands, your getter is lying, it does not return only the name.
Comment 6 youenn fablet 2019-04-08 14:46:54 PDT
> You would fail the test either way since your layout test expects:
> "WebKitTestRunner Service Worker (localhost)".

No, since we check for name.length, which allows to pass for iOS/GTK.
I'll change the default implementation to return "n/a" and check for this string instead.
Comment 7 youenn fablet 2019-04-08 14:51:09 PDT
Created attachment 366987 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2019-04-08 15:29:53 PDT
Comment on attachment 366987 [details]
Patch for landing

Clearing flags on attachment: 366987

Committed r244054: <https://trac.webkit.org/changeset/244054>
Comment 9 WebKit Commit Bot 2019-04-08 15:29:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-04-08 15:30:19 PDT
<rdar://problem/49714318>