Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Single Promise, options object, revamped processing model #70

Merged
merged 14 commits into from
Mar 6, 2020
Merged

Single Promise, options object, revamped processing model #70

Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a426c13
Modify API shape to a single promise & options object. Revamp process…
yoavweiss Mar 2, 2020
2cb13e0
Fix up IDL
yoavweiss Mar 2, 2020
aaa06d6
Responded to review comments
yoavweiss Mar 3, 2020
1d11895
Fix bikeshed error
yoavweiss Mar 3, 2020
29e16ea
Switch to accept a sequence instead of an options object
yoavweiss Mar 3, 2020
81478c4
Fix IDL
yoavweiss Mar 3, 2020
b1e5ad5
Tidy up IDL
yoavweiss Mar 5, 2020
dfe9870
s/let/set/
yoavweiss Mar 5, 2020
c9d6223
Revamp the frozen array creation
yoavweiss Mar 5, 2020
997ee19
Place variable in the method call
yoavweiss Mar 5, 2020
f23bfb8
Attach UA list creation to UA
yoavweiss Mar 6, 2020
02ba201
Merge branch 'revamp_promises' of github.com:yoavweiss/ua-client-hint…
yoavweiss Mar 6, 2020
9777b49
add user agent to UA list
yoavweiss Mar 6, 2020
e244cac
[=user agent=]
yoavweiss Mar 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 60 additions & 26 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ urlPrefix: https://tools.ietf.org/html/draft-ietf-httpbis-header-structure; spec
text: list; url: #section-3.1
type: abstract-op
text: serialize Structured Header; url: #section-4.1
spec:infra; type:dfn; for:/; text:list
</pre>
<pre class="biblio">
{
Expand Down Expand Up @@ -316,56 +317,90 @@ Interface {#interface}
=================

<pre class="idl">
dictionary NavigatorUABrandVersionDict {
required DOMString brand;
dictionary NavigatorUABrandVersion {
DOMString brand;
DOMString version;
yoavweiss marked this conversation as resolved.
Show resolved Hide resolved
};

dictionary UADataOptions {
yoavweiss marked this conversation as resolved.
Show resolved Hide resolved
boolean platform = false;
boolean platformVersion = false;
boolean architecture = false;
boolean model = false;
};

dictionary UADataValues {
DOMString platform = "";
yoavweiss marked this conversation as resolved.
Show resolved Hide resolved
DOMString platformVersion = "";
DOMString architecture = "";
DOMString model = "";
};

[Exposed=Window]
interface NavigatorUAData {
readonly attribute FrozenArray&lt;NavigatorUABrandVersionDict&gt; brand;
readonly attribute FrozenArray&lt;NavigatorUABrandVersion&gt; uaList;
readonly attribute boolean mobile;
Promise&lt;DOMString&gt; getPlatform();
Promise&lt;SOMString&gt; getPlatformVersion();
Promise&lt;DOMString&gt; getArchitecture();
Promise&lt;DOMString&gt; getModel();
Promise&lt;UADataValues&gt; getHighEntropyValues(optional UADataOptions options);
yoavweiss marked this conversation as resolved.
Show resolved Hide resolved
};

interface mixin NavigatorUA {
[SecureContext] NavigatorUAData getUserAgent();
[SecureContext] readonly attribute NavigatorUAData userAgentData;
};
Navigator includes NavigatorUA;

</pre>

Processing model {#processing}
--------------
<dfn method for="NavigatorUA"><code>getUserAgent()</code></dfn> method MUST run these steps:

On getting, the {{NavigatorUAData/uaList}} attribute MUST run these steps:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, this will return a new frozen array each time. That's not great. In particular navigator.userAgentData.uaList !== navigator.userAgentData.uaList.

The way to do this is to have an associated <dfn for="NavigatorUAData">UA list</dfn> and say that it must be initialized following this algorithm, plus at the end, a call to https://heycam.github.io/webidl/#dfn-create-frozen-array. Then, the getting algorithm for {{NavigatorUAData/uaList}} returns this's UA list.

You can see an example of this in https://wicg.github.io/origin-policy/#monkeypatch-html-windoworworkerglobalscope. Here you can get away without an explicit initialization point (i.e. no need to monkeypatch navigation or similar to set the UA list) since the algorithm is the same for all NavigatorUAData instances.

Oh, that raises a new point. Do you want to allow the answer to vary between NavigatorUAData instances, or do you need to maintain consistency across some scope? E.g. across the UA, or across Windows that can reach each other synchronously, or...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Did the above, and did patch WindowOrWorkerGlobalScope, to make sure consistency is maintained across them.

I don't have a strong opinion about consistency across the UA or across Windows - it seems important that the algorithm would end up with the same values for them (especially with headers, to reduce caching variance), but not super important (to me) that objects would ===.
At the same time, if there's an easy way to make that happen in spec and implementation, I'm happy to do that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I don't think the objects should be ===; that'd be a bit strange. I was more concerned about whether the values should be consistent. I'll re-review from that perspective.


1. Let |list| be a [=/list=].

2. Collect pairs of [=user agent/brand=] and [=user agent/significant version=] which represent the user agent,
its equivalence class and/or its rendering engine.

3. For each pair:

1. Let |dict| be a new {{NavigatorUABrandVersion}} dictionary,
with [=user agent/brand=] as {{NavigatorUABrandVersion/brand}} and [=user agent/significant version=] as {{NavigatorUABrandVersion/version}}.

2. Append |dict| to |list|.

4. The user agent MAY execute the following steps:

1. [=list/Append=] additional items to |list| containing {{NavigatorUABrandVersion}} objects,
initialized with arbitrary {{NavigatorUABrandVersion/brand}} and {{NavigatorUABrandVersion/version}} combinations.

2. Randomize the order of the items in |list|.

Note: See [[#grease]] for more details on why these steps might be appropriate.

5. Return |list|.

On getting, the {{NavigatorUAData/mobile}} attribute must return the user agent's [=user agent/mobileness=].

The <dfn method for="NavigatorUA"><code>getHighEntropyValues()</code></dfn> method, called with {{UADataOptions}} |options|, MUST run these steps:

1. Let |p| be a [=a new promise=].

2. Run the following steps [=in parallel=]:

1. Let |UAData| be a new {{NavigatorUAData}} object whose values are initialized as follows:
1. Let |uaData| be a new {{UADataValues}}.

: {{NavigatorUAData/brand}}
:: The user agent's [=user agent/brand=].
: {{NavigatorUAData/getPlatform}}
:: The user agent's [=user agent/platform brand=].
: {{NavigatorUAData/getPlatformVersion}}
:: The user agent's [=user agent/platform version=].
: {{NavigatorUAData/getArchitecture}}
:: The user agent's [=user agent/platform architecture=].
: {{NavigatorUAData/getModel}}
:: The user agent's [=user agent/model=].
: {{NavigatorUAData/mobile}}
:: The user agent's [=user agent/mobileness=].
2. If |options|["{{UADataOptions/platform}}"] is true, let |uaData|["{{UADataValues/platform}}"] be the the user agent's [=user agent/platform brand=].

2. [=Resolve=] |p| with |UAData|.
3. If |options|["{{UADataOptions/platformVersion}}"] is true, let |uaData|["{{UADataValues/platformVersion}}"] be the the user agent's [=user agent/platform version=].

3. Return |p|.
4. If |options|["{{UADataOptions/architecture}}"] is true, let |uaData|["{{UADataValues/architecture}}"] be the the user agent's [=user agent/platform architecture=].

5. If |options|["{{UADataOptions/model}}"] is true, let |uaData|["{{UADataValues/model}}"] be the the user agent's [=user agent/model=].

6. [=Queue a task=] to [=resolve=] |p| with |uaData|.

ISSUE: Provide a method to only access the UA's significant version.
ISSUE: Add a specific task source this is queued on.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on the task queue for this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you plan to implement in Chrome?

There's always the option of giving maximum flexibility by creating an entirely new task source.

The closest counterpart I can think of off the top of my head is createImageBitmap. Unfortunately that doesn't do this right either :(. whatwg/html#5329


3. Return |p|.

Security and Privacy Considerations {#security-privacy}
===================================
Expand Down Expand Up @@ -590,4 +625,3 @@ Author/Change controller:

Specification document:
: this specification ([[#user-agent]]), and Section 5.5.3 of [[RFC7231]]