Bug 158593 - WorkerNavigator is missing some attributes
Summary: WorkerNavigator is missing some attributes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar, WebExposed
Depends on: 163124
Blocks:
  Show dependency treegraph
 
Reported: 2016-06-09 16:43 PDT by Chris Dumez
Modified: 2016-10-07 10:26 PDT (History)
8 users (show)

See Also:


Attachments
Patch (28.20 KB, patch)
2016-06-09 19:54 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.13 KB, patch)
2016-06-09 20:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (29.74 KB, patch)
2016-06-10 15:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (30.69 KB, patch)
2016-06-10 16:52 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (30.68 KB, patch)
2016-06-10 16:55 PDT, 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 2016-06-09 16:43:58 PDT
WorkerNavigator is missing some NavigatorID attributes:
https://html.spec.whatwg.org/multipage/workers.html#the-workernavigator-object
https://html.spec.whatwg.org/multipage/webappapis.html#navigatorid

We should use the same NavigatorID interface for Navigator and WorkerNavigator.
Comment 1 Radar WebKit Bug Importer 2016-06-09 16:45:21 PDT
<rdar://problem/26731334>
Comment 2 Chris Dumez 2016-06-09 19:54:34 PDT
Created attachment 280978 [details]
Patch
Comment 3 Chris Dumez 2016-06-09 20:21:49 PDT
Created attachment 280984 [details]
Patch
Comment 4 Darin Adler 2016-06-10 15:24:38 PDT
Comment on attachment 280984 [details]
Patch

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

> Source/WebCore/page/NavigatorBase.h:35
> +    String appName() const;

Should be a static member function. Annoying that it makes a new String every time and doesn’t use ASCIILiteral. Same for most of the other functions below.

> Source/WebCore/page/NavigatorBase.h:38
> +    String platform() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:40
> +    String appCodeName() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:41
> +    String product() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:42
> +    String productSub() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:43
> +    String vendor() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:44
> +    String vendorSub() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:46
> +    bool onLine() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:48
> +    String language() const;

Should be a static member function.

> Source/WebCore/page/NavigatorBase.h:51
> +    int hardwareConcurrency() const;

Should be a static member function.

> Source/WebCore/platform/Language.cpp:115
> +            Vector<String> overrideCopy;
> +            overrideCopy.reserveInitialCapacity(override.size());
> +            for (String& language : override)
> +                overrideCopy.uncheckedAppend(language.isolatedCopy());
> +            return overrideCopy;

This should be an isolatedCopy function that takes a Vector<String> and returns another Vector<String>, not written in line here like this.
Comment 5 Chris Dumez 2016-06-10 15:56:58 PDT
Created attachment 281054 [details]
Patch
Comment 6 Chris Dumez 2016-06-10 16:52:23 PDT
Created attachment 281060 [details]
Patch
Comment 7 Chris Dumez 2016-06-10 16:55:19 PDT
Created attachment 281061 [details]
Patch
Comment 8 WebKit Commit Bot 2016-06-11 10:42:48 PDT
Comment on attachment 281061 [details]
Patch

Clearing flags on attachment: 281061

Committed r201970: <http://trac.webkit.org/changeset/201970>
Comment 9 WebKit Commit Bot 2016-06-11 10:42:54 PDT
All reviewed patches have been landed.  Closing bug.