Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2122063002:
[OriginTrials] Don't attempt to initialize origin trials in isolated worlds. (Closed)
|
Created:
4 years, 5 months ago by iclelland Modified:
4 years, 5 months ago Reviewers:
haraken CC:
chromium-reviews, blink-reviews, chasej+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[OriginTrials] Don't attempt to initialize origin trials in isolated worlds.
This fixes an issue where calling ScriptState::forMainWorld within an
extension would sometimes interfere with registration of some private APIs.
BUG=619465
Committed: https://crrev.com/a9b70a69ed18c1ba22b9270741e5368db816bc7a
Cr-Commit-Position: refs/heads/master@{#404204}
Patch Set 1 #Patch Set 2 : Only install origin trials in main world #Messages
Total messages: 16 (6 generated)
iclelland@chromium.org changed reviewers: + haraken@chromium.org
+r haraken, can you PTAL? Thanks
Description was changed from ========== [OriginTrials] Don't attempt to initialize origin trials when there are no tokens. This fixes an issue where calling ScriptState::forMainWorld within an extension would someitmes interfere with registration of some private APIs, but also improves performance in the most common case. BUG=619465 ========== to ========== [OriginTrials] Don't attempt to initialize origin trials when there are no tokens. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs, but also improves performance in the most common case. BUG=619465 ==========
LGTM
On 2016/07/05 23:53:40, haraken wrote: > LGTM It looks like this patch stops us from being able to enable origin-trial-features through runtime flags :( I think I'm going to have to find another way to tell whether the OT initialization is going to interfere with the chrome://print context.
On 2016/07/05 23:53:40, haraken wrote: > LGTM It looks like this patch stops us from being able to enable origin-trial-features through runtime flags :( I think I'm going to have to find another way to tell whether the OT initialization is going to interfere with the chrome://print context.
Description was changed from ========== [OriginTrials] Don't attempt to initialize origin trials when there are no tokens. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs, but also improves performance in the most common case. BUG=619465 ========== to ========== [OriginTrials] Don't attempt to initialize origin trials in isolated worlds. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs. BUG=619465 ==========
haraken, can you PTAL again? I rewrote this to avoid even trying to install origin trials in isolated worlds, since the first approach didn't allow runtime flags to work anymore.
On 2016/07/07 13:31:54, iclelland wrote: > haraken, can you PTAL again? I rewrote this to avoid even trying to install > origin trials in isolated worlds, since the first approach didn't allow runtime > flags to work anymore. Just to confirm: You're intending to make OriginTrial not work on isolated worlds (i.e., disable all OriginTrial features on isolated worlds), right? If that's the case, LGTM.
On 2016/07/07 14:05:41, haraken wrote: > On 2016/07/07 13:31:54, iclelland wrote: > > haraken, can you PTAL again? I rewrote this to avoid even trying to install > > origin trials in isolated worlds, since the first approach didn't allow > runtime > > flags to work anymore. > > Just to confirm: You're intending to make OriginTrial not work on isolated > worlds (i.e., disable all OriginTrial features on isolated worlds), right? > > If that's the case, LGTM. Yes, that's correct. We are not supporting origin trials in isolated worlds at all. Thanks!
The CQ bit was checked by iclelland@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [OriginTrials] Don't attempt to initialize origin trials in isolated worlds. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs. BUG=619465 ========== to ========== [OriginTrials] Don't attempt to initialize origin trials in isolated worlds. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs. BUG=619465 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [OriginTrials] Don't attempt to initialize origin trials in isolated worlds. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs. BUG=619465 ========== to ========== [OriginTrials] Don't attempt to initialize origin trials in isolated worlds. This fixes an issue where calling ScriptState::forMainWorld within an extension would sometimes interfere with registration of some private APIs. BUG=619465 Committed: https://crrev.com/a9b70a69ed18c1ba22b9270741e5368db816bc7a Cr-Commit-Position: refs/heads/master@{#404204} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a9b70a69ed18c1ba22b9270741e5368db816bc7a Cr-Commit-Position: refs/heads/master@{#404204} |
