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

Issue 1308723003: DOMWindow::navigator should return navigator w/o frame when detached. (Closed)

Created:
5 years, 3 months ago by yhirano
Modified:
5 years, 3 months ago
Reviewers:
haraken, kinuko
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

DOMWindow::navigator should return a navigator w/o frame when detached. Currently DOMWindow::navigator returns a navigator associated with a frame even when the window is detached from the frame and another window is attached to it. That means calling frame()->document() may return an incorrect document, for example. This CL makes LocalDOMWindow::navigator return a navigator with a null frame when the window is not associated to the frame. BUG=522791 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201055

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 2

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Total comments: 2

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -0 lines) Patch
A LayoutTests/http/tests/serviceworker/chromium/frame-detached-by-navigation.html View 1 2 3 4 5 1 chunk +28 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/serviceworker/chromium/frame-removed.html View 1 2 3 4 5 1 chunk +24 lines, -0 lines 0 comments Download
M Source/core/frame/LocalDOMWindow.cpp View 1 2 3 4 5 6 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 21 (7 generated)
yhirano
PS1 is crashy: gamepad/gamepad-detached-no-crash.html vibration/vibration-detached-no-crash.html fast/dom/Window/property-access-on-cached-window-after-frame-removed.html fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced.html http/tests/navigation/beacon-detached-no-crash.html
5 years, 3 months ago (2015-08-21 14:05:11 UTC) #1
yhirano
Tests pass with PS2 on my environment (three tests fail, but they fail without any ...
5 years, 3 months ago (2015-08-21 14:33:29 UTC) #2
yhirano
5 years, 3 months ago (2015-08-24 02:14:33 UTC) #4
yhirano
haraken@, kinuko@, can you take a look?
5 years, 3 months ago (2015-08-24 02:14:53 UTC) #5
haraken
LGTM with a comment. https://codereview.chromium.org/1308723003/diff/40001/Source/core/frame/LocalDOMWindow.cpp File Source/core/frame/LocalDOMWindow.cpp (right): https://codereview.chromium.org/1308723003/diff/40001/Source/core/frame/LocalDOMWindow.cpp#newcode664 Source/core/frame/LocalDOMWindow.cpp:664: if (frame() && frame()->domWindow() != ...
5 years, 3 months ago (2015-08-24 02:22:16 UTC) #6
kinuko
lgtm/2
5 years, 3 months ago (2015-08-24 02:49:53 UTC) #7
yhirano
https://codereview.chromium.org/1308723003/diff/40001/Source/core/frame/LocalDOMWindow.cpp File Source/core/frame/LocalDOMWindow.cpp (right): https://codereview.chromium.org/1308723003/diff/40001/Source/core/frame/LocalDOMWindow.cpp#newcode664 Source/core/frame/LocalDOMWindow.cpp:664: if (frame() && frame()->domWindow() != this) { On 2015/08/24 ...
5 years, 3 months ago (2015-08-24 03:27:11 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308723003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308723003/80001
5 years, 3 months ago (2015-08-24 03:27:31 UTC) #12
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/97343)
5 years, 3 months ago (2015-08-24 04:29:50 UTC) #14
yhirano
After talking with haraken@, I checked firefox's behavior and it returns a non-null navigator when ...
5 years, 3 months ago (2015-08-24 06:24:29 UTC) #15
haraken
LGTM https://codereview.chromium.org/1308723003/diff/120001/Source/core/frame/LocalDOMWindow.cpp File Source/core/frame/LocalDOMWindow.cpp (right): https://codereview.chromium.org/1308723003/diff/120001/Source/core/frame/LocalDOMWindow.cpp#newcode665 Source/core/frame/LocalDOMWindow.cpp:665: m_navigator = Navigator::create(isCurrentlyDisplayedInFrame() ? frame() : nullptr); Can ...
5 years, 3 months ago (2015-08-24 06:28:21 UTC) #16
yhirano
https://codereview.chromium.org/1308723003/diff/120001/Source/core/frame/LocalDOMWindow.cpp File Source/core/frame/LocalDOMWindow.cpp (right): https://codereview.chromium.org/1308723003/diff/120001/Source/core/frame/LocalDOMWindow.cpp#newcode665 Source/core/frame/LocalDOMWindow.cpp:665: m_navigator = Navigator::create(isCurrentlyDisplayedInFrame() ? frame() : nullptr); On 2015/08/24 ...
5 years, 3 months ago (2015-08-24 06:52:35 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308723003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308723003/140001
5 years, 3 months ago (2015-08-24 08:06:36 UTC) #20
commit-bot: I haz the power
5 years, 3 months ago (2015-08-24 10:02:34 UTC) #21
Message was sent while issue was closed.
Committed patchset #7 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201055

Powered by Google App Engine
This is Rietveld 408576698