Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(137)

Issue 1816983002: Add a setting to allow window object reuse across a top-level initial navigation. (Closed)

Created:
4 years, 9 months ago by dcheng
Modified:
4 years, 8 months ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a setting to allow window object reuse across a top-level initial navigation. r378508 fixed Blink's plumbing to remove the concept failed security context initialization. This breaks Android WebView though: previously a main frame with no opener would "fail to initialize", and the window object would be reused in the initial navigation. Now, a main frame with no opener is just treated as unique origin: this means it always fails the shouldReuseDefaultView() check and the window object is not reused across the initial navigation. Unfortunately, a number of Android apps depend on injecting script into the main frame and having it persist on the first navigation. The new setting is essentially an escape hatch for Android WebView: a main frame that is the only browsing context in its unit of related browsing contexts can only have script injected by the embedder. In this case, it's OK to allow reuse (aka universal access) on the initial navigation, since no web content could have injected script. BUG=593984 Committed: https://crrev.com/73f32aae5298f6a3779385fc4b6bdb08f6e070a5 Cr-Commit-Position: refs/heads/master@{#385263}

Patch Set 1 #

Patch Set 2 : Minor fixes, see if it passes tests. #

Patch Set 3 : . #

Patch Set 4 : . #

Patch Set 5 : Move out unrelated refactoring #

Patch Set 6 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -0 lines) Patch
M third_party/WebKit/Source/core/frame/Settings.in View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebSettingsImpl.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebSettingsImpl.cpp View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebFrameTest.cpp View 1 2 3 1 chunk +77 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebSettings.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12 (6 generated)
dcheng
Meh.
4 years, 8 months ago (2016-04-02 00:52:48 UTC) #5
jochen (gone - plz use gerrit)
lgtm
4 years, 8 months ago (2016-04-04 12:08:59 UTC) #6
Nate Chapin
On 2016/04/04 12:08:59, jochen wrote: > lgtm lgtm. lbtm, but lgtm.
4 years, 8 months ago (2016-04-05 18:35:54 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1816983002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1816983002/100001
4 years, 8 months ago (2016-04-05 18:39:24 UTC) #9
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 8 months ago (2016-04-05 19:47:47 UTC) #10
commit-bot: I haz the power
4 years, 8 months ago (2016-04-05 19:49:46 UTC) #12
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/73f32aae5298f6a3779385fc4b6bdb08f6e070a5
Cr-Commit-Position: refs/heads/master@{#385263}

Powered by Google App Engine
This is Rietveld 408576698