Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1864623004:
messageHandlerInMainThread should return immediately if there is no entered window (Closed)
DescriptionmessageHandlerInMainThread should return immediately if there is no entered window
messageHandlerInMainThread should return immediately if there is no entered window.
To that end, messageHandlerInMainThread already has an 'if (!enteredWindow || ...) return' branch,
but the branch never hits because enteredDOMWindow() crashes if there is no entered or current window.
This CL stops calling enteredDOMWindow() so that messageHandlerInMainThread can
return immediately if there is no entered window.
BUG=599115
Committed: https://crrev.com/b17f26e5db7fcb32d8a771048ca994f4bd4ce77e
Cr-Commit-Position: refs/heads/master@{#385524}
Patch Set 1 #
Total comments: 2
Patch Set 2 : #
Messages
Total messages: 25 (11 generated)
haraken@chromium.org changed reviewers: + csharrison@chromium.org
PTAL
haraken@chromium.org changed reviewers: + yukishiino@chromium.org
yukishiino: PTAL
LGTM. https://codereview.chromium.org/1864623004/diff/1/third_party/WebKit/Source/b... File third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp (right): https://codereview.chromium.org/1864623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:140: LocalDOMWindow* enteredWindow = toLocalDOMWindow(toDOMWindow(isolate->GetEnteredContext())); Maybe better to add a TODO. I think we should have toEnteredDOMWindowOrNull() or something like that. We don't want all call sites to write toLocalDOMWindow(toDOMWindow(isolate->GetEnteredContext())).
https://codereview.chromium.org/1864623004/diff/1/third_party/WebKit/Source/b... File third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp (right): https://codereview.chromium.org/1864623004/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp:140: LocalDOMWindow* enteredWindow = toLocalDOMWindow(toDOMWindow(isolate->GetEnteredContext())); On 2016/04/06 10:30:14, Yuki wrote: > Maybe better to add a TODO. > I think we should have toEnteredDOMWindowOrNull() or something like that. We > don't want all call sites to write > toLocalDOMWindow(toDOMWindow(isolate->GetEnteredContext())). Done. Will look into this after landing this CL.
The CQ bit was checked by haraken@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yukishiino@chromium.org Link to the patchset: https://codereview.chromium.org/1864623004/#ps20001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864623004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864623004/20001
Thanks for this change!
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
The CQ bit was checked by csharrison@chromium.org
lgtm Failures look like potential flakes. Retrying.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864623004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864623004/20001
The CQ bit was unchecked by csharrison@chromium.org
The CQ bit was checked by csharrison@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864623004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864623004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by csharrison@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1864623004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1864623004/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== messageHandlerInMainThread should return immediately if there is no entered window messageHandlerInMainThread should return immediately if there is no entered window. To that end, messageHandlerInMainThread already has an 'if (!enteredWindow || ...) return' branch, but the branch never hits because enteredDOMWindow() crashes if there is no entered or current window. This CL stops calling enteredDOMWindow() so that messageHandlerInMainThread can return immediately if there is no entered window. BUG=599115 ========== to ========== messageHandlerInMainThread should return immediately if there is no entered window messageHandlerInMainThread should return immediately if there is no entered window. To that end, messageHandlerInMainThread already has an 'if (!enteredWindow || ...) return' branch, but the branch never hits because enteredDOMWindow() crashes if there is no entered or current window. This CL stops calling enteredDOMWindow() so that messageHandlerInMainThread can return immediately if there is no entered window. BUG=599115 Committed: https://crrev.com/b17f26e5db7fcb32d8a771048ca994f4bd4ce77e Cr-Commit-Position: refs/heads/master@{#385524} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b17f26e5db7fcb32d8a771048ca994f4bd4ce77e Cr-Commit-Position: refs/heads/master@{#385524} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
