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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 184043002: Only Microtask::performCheckpoint() if recursionLevel is 0 (Closed)

Created:
6 years, 9 months ago by rafaelw
Modified:
6 years, 9 months ago
Reviewers:
adamk
CC:
blink-reviews, sof, eae+blinkwatch, dglazkov+blink, adamk+blink_chromium.org, Inactive, rwlbuis
Visibility:
Public.

Description

Only Microtask::performCheckpoint() if recursionLevel is 0 This maintains the invariant that microtask work (e.g. Mutation Observer callbacks) always occur on an empty page script stack. Previously, it was possible for a microtask to fire before the outer script had exited on document.write('<script>') or showModalDialog, etc.. R=adamk@chromium.org, adamk BUG=352181 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169770

Patch Set 1 #

Patch Set 2 : dont performCheckpoint unless recursionLevel is 0 #

Patch Set 3 : ws #

Total comments: 1

Patch Set 4 : sync #

Patch Set 5 : add test #

Patch Set 6 : return if recursionLevel > 0 #

Patch Set 7 : cleanup #

Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -0 lines) Patch
A LayoutTests/fast/dom/MutationObserver/document-write.html View 1 2 3 4 1 chunk +53 lines, -0 lines 0 comments Download
A + LayoutTests/fast/dom/MutationObserver/document-write-expected.txt View 1 2 3 4 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/dom/Microtask.cpp View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 33 (0 generated)
rafaelw
6 years, 9 months ago (2014-02-28 00:26:23 UTC) #1
adamk
lgtm
6 years, 9 months ago (2014-02-28 00:31:40 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/1
6 years, 9 months ago (2014-02-28 00:31:50 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/1
6 years, 9 months ago (2014-02-28 01:12:43 UTC) #4
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-02-28 04:53:07 UTC) #5
commit-bot: I haz the power
Retried try job too often on linux_blink for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink&number=14590
6 years, 9 months ago (2014-02-28 04:53:07 UTC) #6
dominicc (has gone to gerrit)
I'm pretty sure this will wreak havoc with a bunch of the fast/parser tests in ...
6 years, 9 months ago (2014-02-28 04:54:36 UTC) #7
adamk
On 2014/02/28 04:54:36, dominicc wrote: > I'm pretty sure this will wreak havoc with a ...
6 years, 9 months ago (2014-02-28 17:37:40 UTC) #8
rafaelw
The CQ bit was checked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-20 21:53:02 UTC) #9
rafaelw
The CQ bit was unchecked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-20 21:53:05 UTC) #10
rafaelw
6 years, 9 months ago (2014-03-20 21:54:13 UTC) #11
adamk
lgtm https://codereview.chromium.org/184043002/diff/40001/Source/core/html/parser/HTMLScriptRunner.cpp File Source/core/html/parser/HTMLScriptRunner.cpp (right): https://codereview.chromium.org/184043002/diff/40001/Source/core/html/parser/HTMLScriptRunner.cpp#newcode30 Source/core/html/parser/HTMLScriptRunner.cpp:30: #include "bindings/v8/V8PerIsolateData.h" This seems a bit wrong to ...
6 years, 9 months ago (2014-03-20 21:58:47 UTC) #12
rafaelw
The CQ bit was checked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-20 22:04:30 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/40001
6 years, 9 months ago (2014-03-20 22:04:36 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-20 22:07:36 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-20 22:07:37 UTC) #16
adamk
Can you add tests? Also, more CL description, please: under what conditions could we previously ...
6 years, 9 months ago (2014-03-20 22:35:01 UTC) #17
rafaelw
The CQ bit was checked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-21 03:36:17 UTC) #18
rafaelw
The CQ bit was unchecked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-21 03:36:20 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/60001
6 years, 9 months ago (2014-03-21 03:36:21 UTC) #20
rafaelw
The CQ bit was checked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-21 03:38:34 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/60001
6 years, 9 months ago (2014-03-21 03:38:42 UTC) #22
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 05:08:58 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_dbg
6 years, 9 months ago (2014-03-21 05:08:59 UTC) #24
rafaelw
The CQ bit was checked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-21 19:38:31 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/110001
6 years, 9 months ago (2014-03-21 19:38:38 UTC) #26
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 19:41:06 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_rel
6 years, 9 months ago (2014-03-21 19:41:07 UTC) #28
rafaelw
The CQ bit was checked by rafaelw@chromium.org
6 years, 9 months ago (2014-03-21 19:42:57 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rafaelw@chromium.org/184043002/110001
6 years, 9 months ago (2014-03-21 19:43:05 UTC) #30
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 19:45:32 UTC) #31
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.blink on linux_blink_rel
6 years, 9 months ago (2014-03-21 19:45:35 UTC) #32
rafaelw
6 years, 9 months ago (2014-03-21 21:07:35 UTC) #33
Message was sent while issue was closed. 
Committed patchset #7 manually as r169770 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698