third_party/WebKit/Source/modules/serviceworkers/NavigationPreloadManager.idl - Issue 2780093003: V8 bindings gen: Error if OriginTrialEnabled and SecureContext together.

Side by Side Diff: third_party/WebKit/Source/modules/serviceworkers/NavigationPreloadManager.idl

Issue 2780093003: V8 bindings gen: Error if OriginTrialEnabled and SecureContext together. (Closed)

Patch Set: NavigationPreloadManager: Avoid hitting the error. Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« no previous file with comments | « third_party/WebKit/Source/bindings/scripts/v8_utilities.py ('k') | no next file » | no next file with comments »

OLD	NEW
1 // Copyright 2016 The Chromium Authors. All rights reserved.	1 // Copyright 2016 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 // TODO(falken): Revise link when this lands in the spec:	5 // TODO(falken): Revise link when this lands in the spec:

6 // https://github.com/w3c/ServiceWorker/issues/920	6 // https://github.com/w3c/ServiceWorker/issues/920

7 [	7 [

8 OriginTrialEnabled=ServiceWorkerNavigationPreload,	8 OriginTrialEnabled=ServiceWorkerNavigationPreload,

9 SecureContext,

10 Exposed=(Window,Worker),	9 Exposed=(Window,Worker),

11 ] interface NavigationPreloadManager {	10 ] interface NavigationPreloadManager {

12 [CallWith=ScriptState] Promise<void> enable();	11 [SecureContext, CallWith=ScriptState] Promise<void> enable();
	Matt Giuca 2017/03/29 06:24:37 Oh no this is awful. It turns out that Navigation Oh no this is awful. It turns out that NavigationPreloadManager is hitting the new error because it uses OTE and SC at the same time. But this isn't actually a problem because the SC is being automatically applied to all the members of NavigationPreloadManager (not the class itself), and the class itself is being registered manually. If I move the SecureContext down here to all the methods, it doesn't hit the check and seems to generate the same code. Is this acceptable? Yuki 2017/03/29 06:38:06 Just a question, why NavigationPreloadManager is O Show quoted text On 2017/03/29 06:24:37, Matt Giuca wrote: > Oh no this is awful. > > It turns out that NavigationPreloadManager is hitting the new error because it > uses OTE and SC at the same time. But this isn't actually a problem because the > SC is being automatically applied to all the members of NavigationPreloadManager > (not the class itself), and the class itself is being registered manually. > > If I move the SecureContext down here to all the methods, it doesn't hit the > check and seems to generate the same code. > > Is this acceptable? Just a question, why NavigationPreloadManager is OriginTrialEnabled? If it's because it's not yet stable and we'd like to expose the APIs to limited users and we're planning to remove [OriginTrialEnabled], it makes a sense. However, with this change, we're going to expose NavigationPreloadManager interface itself as long as the origin trial is specified even when it's not a secure context. Should we expose NavigationPreloadManager only when the origin trial is specified and it's a secure context? https://w3c.github.io/ServiceWorker/#navigation-preload-manager The spec seems saying we shouldn't expose the interface if it's not a secure context. falken 2017/03/29 06:43:45 Yes, NavigationPreloadManager is an experimental f Show quoted text On 2017/03/29 06:38:06, Yuki wrote: > On 2017/03/29 06:24:37, Matt Giuca wrote: > > Oh no this is awful. > > > > It turns out that NavigationPreloadManager is hitting the new error because it > > uses OTE and SC at the same time. But this isn't actually a problem because > the > > SC is being automatically applied to all the members of > NavigationPreloadManager > > (not the class itself), and the class itself is being registered manually. > > > > If I move the SecureContext down here to all the methods, it doesn't hit the > > check and seems to generate the same code. > > > > Is this acceptable? > > Just a question, why NavigationPreloadManager is OriginTrialEnabled? > If it's because it's not yet stable and we'd like to expose the APIs to limited > users and we're planning to remove [OriginTrialEnabled], it makes a sense. > > However, with this change, we're going to expose NavigationPreloadManager > interface itself as long as the origin trial is specified even when it's not a > secure context. > > Should we expose NavigationPreloadManager only when the origin trial is > specified and it's a secure context? > > https://w3c.github.io/ServiceWorker/#navigation-preload-manager > The spec seems saying we shouldn't expose the interface if it's not a secure > context. Yes, NavigationPreloadManager is an experimental feature that is in an origin trial. The feature should only be exposed in secure contexts. Matt Giuca 2017/03/30 02:57:28 I would be changing the IDL away from the spec, bu Show quoted text On 2017/03/29 06:43:45, falken wrote: > On 2017/03/29 06:38:06, Yuki wrote: > > On 2017/03/29 06:24:37, Matt Giuca wrote: > > > Oh no this is awful. > > > > > > It turns out that NavigationPreloadManager is hitting the new error because > it > > > uses OTE and SC at the same time. But this isn't actually a problem because > > the > > > SC is being automatically applied to all the members of > > NavigationPreloadManager > > > (not the class itself), and the class itself is being registered manually. > > > > > > If I move the SecureContext down here to all the methods, it doesn't hit the > > > check and seems to generate the same code. > > > > > > Is this acceptable? > > > > Just a question, why NavigationPreloadManager is OriginTrialEnabled? > > If it's because it's not yet stable and we'd like to expose the APIs to > limited > > users and we're planning to remove [OriginTrialEnabled], it makes a sense. > > > > However, with this change, we're going to expose NavigationPreloadManager > > interface itself as long as the origin trial is specified even when it's not a > > secure context. > > > > Should we expose NavigationPreloadManager only when the origin trial is > > specified and it's a secure context? > > > > https://w3c.github.io/ServiceWorker/#navigation-preload-manager > > The spec seems saying we shouldn't expose the interface if it's not a secure > > context. > > Yes, NavigationPreloadManager is an experimental feature that is in an origin > trial. The feature should only be exposed in secure contexts. I would be changing the IDL away from the spec, but it has the same effect in Chrome (the NavigationPreloadManager object itself is still available in a non-secure context, just not the methods). I can add a comment saying this is a work-around this bug.
13 [CallWith=ScriptState] Promise<void> disable();	12 [SecureContext, CallWith=ScriptState] Promise<void> disable();

14 [CallWith=ScriptState] Promise<void> setHeaderValue(ByteString value);	13 [SecureContext, CallWith=ScriptState] Promise<void> setHeaderValue(ByteStrin g value);

15 [CallWith=ScriptState] Promise<NavigationPreloadState> getState();	14 [SecureContext, CallWith=ScriptState] Promise<NavigationPreloadState> getSta te();

16 };	15 };

OLD	NEW