Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(21)

Issue 2035973002: Don't allow deferred frames to create new windows. (Closed)

Created:
3 years, 5 months ago by Mariusz Mlynski
Modified:
3 years, 4 months ago
Reviewers:
kinuko, Nate Chapin
CC:
chromium-reviews, blink-reviews, kinuko+watch
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't allow deferred frames to create new windows. New pages never defer loads, which makes it difficult for ScopedPageLoadDeferrer to protect against synchronous loads. This patch adds a check in ChromeClientImpl::createWindow. BUG=616907 Committed: https://crrev.com/25a8ae78165cd8725465d26e02a342d7121c42f4 Cr-Commit-Position: refs/heads/master@{#403640}

Patch Set 1 #

Patch Set 2 : Added test. #

Patch Set 3 : WebView::create update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -0 lines) Patch
M third_party/WebKit/Source/web/ChromeClientImpl.cpp View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/tests/ChromeClientImplTest.cpp View 1 2 3 chunks +46 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (6 generated)
Nate Chapin
Cod change LGTM. Is your repro in bug 616907 viable as a layout test?
3 years, 5 months ago (2016-06-09 18:29:41 UTC) #2
Mariusz Mlynski
On 2016/06/09 18:29:41, Nate Chapin wrote: > Cod change LGTM. Is your repro in bug ...
3 years, 5 months ago (2016-06-09 18:54:11 UTC) #3
Nate Chapin
That's what I was afraid of. It might be worth taking a quick look as ...
3 years, 5 months ago (2016-06-09 19:05:46 UTC) #4
Mariusz Mlynski
On 2016/06/09 19:05:46, Nate Chapin wrote: > That's what I was afraid of. It might ...
3 years, 5 months ago (2016-06-09 19:54:35 UTC) #5
Nate Chapin
On 2016/06/09 19:54:35, Mariusz Mlynski wrote: > On 2016/06/09 19:05:46, Nate Chapin wrote: > > ...
3 years, 5 months ago (2016-06-10 21:28:01 UTC) #6
Mariusz Mlynski
Nate, could you PTAL at the test?
3 years, 5 months ago (2016-06-14 05:58:12 UTC) #7
Nate Chapin
LGTM, thanks!
3 years, 5 months ago (2016-06-14 21:42:13 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2035973002/20001
3 years, 4 months ago (2016-07-04 03:11:18 UTC) #10
kinuko
I think we want this patch in, let me try CQ-ing now (in case you've ...
3 years, 4 months ago (2016-07-04 03:13:27 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/187149) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
3 years, 4 months ago (2016-07-04 03:24:11 UTC) #13
Mariusz Mlynski
WebView::create was modified a few days ago, please try again.
3 years, 4 months ago (2016-07-04 04:21:30 UTC) #14
kinuko
On 2016/07/04 04:21:30, Mariusz Mlynski wrote: > WebView::create was modified a few days ago, please ...
3 years, 4 months ago (2016-07-04 04:58:49 UTC) #15
kinuko
Ah ok, you already updated.. thanks!
3 years, 4 months ago (2016-07-04 04:59:08 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2035973002/40001
3 years, 4 months ago (2016-07-04 04:59:26 UTC) #19
commit-bot: I haz the power
Committed patchset #3 (id:40001)
3 years, 4 months ago (2016-07-04 06:25:14 UTC) #20
commit-bot: I haz the power
3 years, 4 months ago (2016-07-04 06:28:25 UTC) #22
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/25a8ae78165cd8725465d26e02a342d7121c42f4
Cr-Commit-Position: refs/heads/master@{#403640}

Powered by Google App Engine
This is Rietveld 408576698