Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2244003003:
Change which ExceptionEvents are triggered by Promises (Closed)
|
Created:
4 years, 4 months ago by Dan Ehrenberg Modified:
4 years, 4 months ago CC:
jgruber, v8-reviews_googlegroups.com, Yang Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionChange which ExceptionEvents are triggered by Promises
To make async/await catch prediction work well, this patch regularizes
the exception events sent to DevTools from various places in the Promise
lifecycle. The core is that there should be an exception event when the
rejection first starts, rather than when it is propagated.
- Several cases within Promise code which propagate errors are
modified to not trigger a new ExceptionEvent in that case, such
as .then on a rejected Promise and returning a rejected Promise
from .then, as well as Promise.race and Promise.all.
- Make Promise.reject() create an ExceptionEvent, subject to catch
prediction based on the Promise stack. This is important
so that, e.g., if "await Promise.reject()" will trigger a new
throw (rather than a silent rethrow of something that never
triggered an event in the first place).
BUG=v8:5167
Committed: https://crrev.com/013e49f73cab96c49df284312f8316e572df3a96
Cr-Commit-Position: refs/heads/master@{#38847}
Patch Set 1 #Patch Set 2 : Changed tests #Patch Set 3 : Update tests #Patch Set 4 : No redundant exception for async/await #Patch Set 5 : Test for redundant exceptions #Patch Set 6 : Fix comments for the tests, add another test and fix it #Patch Set 7 : Suppress some more events, which actually allows all of the original tests to pass with minimal mod… #Patch Set 8 : Fix expected #args #
Total comments: 6
Patch Set 9 : Changes based on code review, and suppress Promise.all/Promise.race-based redundant exceptions #Patch Set 10 : formatting #Patch Set 11 : formatting #Patch Set 12 : // namespace #
Total comments: 3
Patch Set 13 : Fixed typos #Dependent Patchsets: Messages
Total messages: 61 (44 generated)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)
Description was changed from
==========
Change which ExceptionEvents Promises trigger
- When a Promise is resolved with a rejected Promise, then no
additional ExceptionEvent is created, with the idea that an
ExceptionEvent was sent when the "inner" rejection was created.
- Make Promise.reject() create an ExceptionEvent. This is important
so that, e.g., if "await Promise.reject()" will trigger a new
throw (rather than a silent rethrow of something that never
triggered an event in the first place).
BUG=v8:5167
==========
to
==========
Change which ExceptionEvents are triggered by Promises
To make async/await catch prediction work well, this patch regularizes
the exception events sent to DevTools from various places in the Promise
lifecycle. The core is that there should be an exception event when the
rejection first starts, rather than when it is propagated.
- When a Promise is resolved with a rejected Promise, then no
additional ExceptionEvent is created, with the idea that an
ExceptionEvent was sent when the "inner" rejection was created.
- Make Promise.reject() create an ExceptionEvent. This is important
so that, e.g., if "await Promise.reject()" will trigger a new
throw (rather than a silent rethrow of something that never
triggered an event in the first place).
The patches together have the unfortunate effect of making certain
idioms which are caught be predicted as uncaught, as seen in the test
updates. A minimal case is:
Promise.resolve().then(() => Promise.reject()).catch(() => {})
Previously, this would predict as 'caught', as the catch block is visible
when propagating the reject, but it would now predict as uncaught, as it is
impossible to say what is happening when the Promise.reject() function
is in the middle of running. However, it seems to me like this is the
appropriate time to throw the event, with appropriately usable context.
BUG=v8:5167
==========
littledan@chromium.org changed reviewers: + adamk@chromium.org, caitp@igalia.com, gsathya@chromium.org, kozyatinskiy@chromium.org
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/10968) v8_linux_dbg_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng_triggered/b...)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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 littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
littledan@chromium.org changed reviewers: + neis@chromium.org
Does this CL have an effect, in itself, on how async/await and rejection interact? Or is there a follow-up which does that? It would help me analyze the effects of this patch to know how it affects async/await. Another high-level note: as you're changing what the tests do, please update the comments to match. Some of them now say the opposite of what the test does (that change would also make reviewing the tests easier; as-is, it's tricky as a reviewer to tell exactly why a test changed in a particular way).
On 2016/08/18 at 20:08:31, adamk wrote: > Does this CL have an effect, in itself, on how async/await and rejection interact? Or is there a follow-up which does that? It would help me analyze the effects of this patch to know how it affects async/await. Yes, the new test added in async-debug-caught-exception.js test for that. I ran them without this patch, and two exception events were triggered. Although the scenario was already possible, async/await should make it more common, which is why I see this as a core part of async await exception event fixes. > > Another high-level note: as you're changing what the tests do, please update the comments to match. Some of them now say the opposite of what the test does (that change would also make reviewing the tests easier; as-is, it's tricky as a reviewer to tell exactly why a test changed in a particular way). Will do.
The CQ bit was checked by littledan@chromium.org
The new version fixes comments in existing tests and also fixes a new test case, where there is an await before returning a rejected promise from an async function. PTAL.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-v8-committers". Note that this has nothing to do with OWNERS files.
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
This lgtm from the v8 code side, I'd like it to be signed-off from the devtools side (kozyatinskiy) before landing.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I play with patch and without it in DevTools. Here are some results: Current state: - Promise.reject() will never pause; - Promise.reject().catch(() => console.log(239)) will never pause; - Promise.resolve().then(() => Promise.reject()).catch(() => console.log(239)) will never pause; - var r; var p = new Promise((t, e) => r = e); r(); will always pause on r call; - var r; var p = new Promise((t, e) => r = e); p.catch(() => console.log(239)); r() will pause only if "Pause On Caught Exceptions" is set; - var r; var p = new Promise((t, e) => r = e); r(); p.catch(() => console.log(239)); will always pause; With patch: - Promise.reject() will always pause; - Promise.reject().catch(() => console.log(239)) will always pause on reject call; - Promise.resolve().then(() => Promise.reject()).catch(() => console.log(239)) will always pause; - var r; var p = new Promise((t, e) => r = e); r(); will always pause on r call; - var r; var p = new Promise((t, e) => r = e); p.catch(() => console.log(239)); r() will pause only if "Pause On Caught Exceptions" is set; - var r; var p = new Promise((t, e) => r = e); r(); p.catch(() => console.log(239)); will always pause; New behavior is definitely better then current. I think that second case isn't important and is the same with last one, we can't predict handlers that will be installed after promise fulfilled. But.. third one is important for DevTools users. I use this idiom in my code and I think a lot of other developers use it too. It looks for me that at moment of Promise.reject call we know that we're running microtask for promise that already has catch handler. To maintain hasCatchHandler flag we can store in each promise counter, on then call we will increase this counter in this promise, on catch call we will decrease this counter in this promise and in all promises in this chain. If counter is more then one then this rejection is uncaught otherwise it's caught. WDYT?
On 2016/08/22 at 22:04:50, kozyatinskiy wrote: > I play with patch and without it in DevTools. Here are some results: > > Current state: > - Promise.reject() will never pause; > - Promise.reject().catch(() => console.log(239)) will never pause; > - Promise.resolve().then(() => Promise.reject()).catch(() => console.log(239)) will never pause; > - var r; var p = new Promise((t, e) => r = e); r(); will always pause on r call; > - var r; var p = new Promise((t, e) => r = e); p.catch(() => console.log(239)); r() will pause only if "Pause On Caught Exceptions" is set; > - var r; var p = new Promise((t, e) => r = e); r(); p.catch(() => console.log(239)); will always pause; > > With patch: > - Promise.reject() will always pause; > - Promise.reject().catch(() => console.log(239)) will always pause on reject call; > - Promise.resolve().then(() => Promise.reject()).catch(() => console.log(239)) will always pause; > - var r; var p = new Promise((t, e) => r = e); r(); will always pause on r call; > - var r; var p = new Promise((t, e) => r = e); p.catch(() => console.log(239)); r() will pause only if "Pause On Caught Exceptions" is set; > - var r; var p = new Promise((t, e) => r = e); r(); p.catch(() => console.log(239)); will always pause; > > New behavior is definitely better then current. > I think that second case isn't important and is the same with last one, we can't predict handlers that will be installed after promise fulfilled. > > But.. third one is important for DevTools users. I use this idiom in my code and I think a lot of other developers use it too. It looks for me that at moment of Promise.reject call we know that we're running microtask for promise that already has catch handler. To maintain hasCatchHandler flag we can store in each promise counter, on then call we will increase this counter in this promise, on catch call we will decrease this counter in this promise and in all promises in this chain. If counter is more then one then this rejection is uncaught otherwise it's caught. WDYT? OK, sounds like the right thing is to invoke catch prediction as part of Promise.reject()'s call into the runtime. We already pass a boolean into %PromiseRejectEvent; we can make this a tri-state where the third option is "do catch prediction". I think that would get your third case and more.
On 2016/08/23 at 00:15:56, Dan Ehrenberg wrote: > On 2016/08/22 at 22:04:50, kozyatinskiy wrote: > > I play with patch and without it in DevTools. Here are some results: > > > > Current state: > > - Promise.reject() will never pause; > > - Promise.reject().catch(() => console.log(239)) will never pause; > > - Promise.resolve().then(() => Promise.reject()).catch(() => console.log(239)) will never pause; > > - var r; var p = new Promise((t, e) => r = e); r(); will always pause on r call; > > - var r; var p = new Promise((t, e) => r = e); p.catch(() => console.log(239)); r() will pause only if "Pause On Caught Exceptions" is set; > > - var r; var p = new Promise((t, e) => r = e); r(); p.catch(() => console.log(239)); will always pause; > > > > With patch: > > - Promise.reject() will always pause; > > - Promise.reject().catch(() => console.log(239)) will always pause on reject call; > > - Promise.resolve().then(() => Promise.reject()).catch(() => console.log(239)) will always pause; > > - var r; var p = new Promise((t, e) => r = e); r(); will always pause on r call; > > - var r; var p = new Promise((t, e) => r = e); p.catch(() => console.log(239)); r() will pause only if "Pause On Caught Exceptions" is set; > > - var r; var p = new Promise((t, e) => r = e); r(); p.catch(() => console.log(239)); will always pause; > > > > New behavior is definitely better then current. > > I think that second case isn't important and is the same with last one, we can't predict handlers that will be installed after promise fulfilled. > > > > But.. third one is important for DevTools users. I use this idiom in my code and I think a lot of other developers use it too. It looks for me that at moment of Promise.reject call we know that we're running microtask for promise that already has catch handler. To maintain hasCatchHandler flag we can store in each promise counter, on then call we will increase this counter in this promise, on catch call we will decrease this counter in this promise and in all promises in this chain. If counter is more then one then this rejection is uncaught otherwise it's caught. WDYT? > > OK, sounds like the right thing is to invoke catch prediction as part of Promise.reject()'s call into the runtime. We already pass a boolean into %PromiseRejectEvent; we can make this a tri-state where the third option is "do catch prediction". I think that would get your third case and more. The new version makes better use of catch prediction, which allows this patch to pass all of the original tests (with only small modifications to allow for a more useful stack frame). PTAL
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from
==========
Change which ExceptionEvents are triggered by Promises
To make async/await catch prediction work well, this patch regularizes
the exception events sent to DevTools from various places in the Promise
lifecycle. The core is that there should be an exception event when the
rejection first starts, rather than when it is propagated.
- When a Promise is resolved with a rejected Promise, then no
additional ExceptionEvent is created, with the idea that an
ExceptionEvent was sent when the "inner" rejection was created.
- Make Promise.reject() create an ExceptionEvent. This is important
so that, e.g., if "await Promise.reject()" will trigger a new
throw (rather than a silent rethrow of something that never
triggered an event in the first place).
The patches together have the unfortunate effect of making certain
idioms which are caught be predicted as uncaught, as seen in the test
updates. A minimal case is:
Promise.resolve().then(() => Promise.reject()).catch(() => {})
Previously, this would predict as 'caught', as the catch block is visible
when propagating the reject, but it would now predict as uncaught, as it is
impossible to say what is happening when the Promise.reject() function
is in the middle of running. However, it seems to me like this is the
appropriate time to throw the event, with appropriately usable context.
BUG=v8:5167
==========
to
==========
Change which ExceptionEvents are triggered by Promises
To make async/await catch prediction work well, this patch regularizes
the exception events sent to DevTools from various places in the Promise
lifecycle. The core is that there should be an exception event when the
rejection first starts, rather than when it is propagated.
- Several cases within Promise code which propagate errors are
modified to not trigger a new ExceptionEvent in that case, such
as .then on a rejected Promise and returning a rejected Promise
from .then.
- Make Promise.reject() create an ExceptionEvent, subject to catch
prediction based on the Promise stack. This is important
so that, e.g., if "await Promise.reject()" will trigger a new
throw (rather than a silent rethrow of something that never
triggered an event in the first place).
BUG=v8:5167
==========
https://codereview.chromium.org/2244003003/diff/140001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/2244003003/diff/140001/src/js/promise.js#newc... src/js/promise.js:308: function RejectPromise(promise, reason, suppressDebugEvent) { It's confusing to me that %PromiseRejectEvent takes a boolean that's inverted from this one. Can you change them to match? Either change this one to "emitDebugEvent" or similar, or change the runtime function to make its argument mean "suppress". If you do the latter, you could avoid passing the DEBUG_IS_ACTIVE bit, since I assume it's easy to get ahold of that from C++ anyway. https://codereview.chromium.org/2244003003/diff/140001/src/js/promise.js#newc... src/js/promise.js:375: %PromiseRejectEvent(promise, r, false, false); Passing two bools is not ideal, see suggestion in runtime-internal.cc. https://codereview.chromium.org/2244003003/diff/140001/src/runtime/runtime-in... File src/runtime/runtime-internal.cc (right): https://codereview.chromium.org/2244003003/diff/140001/src/runtime/runtime-in... src/runtime/runtime-internal.cc:282: CONVERT_BOOLEAN_ARG_CHECKED(from_promise_reject, 3); Rather than taking a second bool arg, you could split this runtime function into two and factor the logic out into a helper.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/08/23 21:22:24, Dan Ehrenberg wrote: > The new version makes better use of catch prediction, which allows this patch to > pass all of the original tests (with only small modifications to allow for a > more useful stack frame). PTAL Thanks! Now it works great in DevTools. lgtm for DevTools behavior.
Description was changed from ========== Change which ExceptionEvents are triggered by Promises To make async/await catch prediction work well, this patch regularizes the exception events sent to DevTools from various places in the Promise lifecycle. The core is that there should be an exception event when the rejection first starts, rather than when it is propagated. - Several cases within Promise code which propagate errors are modified to not trigger a new ExceptionEvent in that case, such as .then on a rejected Promise and returning a rejected Promise from .then. - Make Promise.reject() create an ExceptionEvent, subject to catch prediction based on the Promise stack. This is important so that, e.g., if "await Promise.reject()" will trigger a new throw (rather than a silent rethrow of something that never triggered an event in the first place). BUG=v8:5167 ========== to ========== Change which ExceptionEvents are triggered by Promises To make async/await catch prediction work well, this patch regularizes the exception events sent to DevTools from various places in the Promise lifecycle. The core is that there should be an exception event when the rejection first starts, rather than when it is propagated. - Several cases within Promise code which propagate errors are modified to not trigger a new ExceptionEvent in that case, such as .then on a rejected Promise and returning a rejected Promise from .then, as well as Promise.race and Promise.all. - Make Promise.reject() create an ExceptionEvent, subject to catch prediction based on the Promise stack. This is important so that, e.g., if "await Promise.reject()" will trigger a new throw (rather than a silent rethrow of something that never triggered an event in the first place). BUG=v8:5167 ==========
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Made cleanups and further improvements based on review; PTAL https://codereview.chromium.org/2244003003/diff/140001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/2244003003/diff/140001/src/js/promise.js#newc... src/js/promise.js:308: function RejectPromise(promise, reason, suppressDebugEvent) { On 2016/08/23 at 22:29:27, adamk wrote: > It's confusing to me that %PromiseRejectEvent takes a boolean that's inverted from this one. Can you change them to match? Either change this one to "emitDebugEvent" or similar, or change the runtime function to make its argument mean "suppress". If you do the latter, you could avoid passing the DEBUG_IS_ACTIVE bit, since I assume it's easy to get ahold of that from C++ anyway. Good point. Switched it to debugEvent as a positive reason everywhere. That change motivated an audit of the code base (since I'm not depending on a default argument anymore) to see whether events should be triggered, and I found that we probably shouldn't be triggering these events for the output of Promise.all and Promise.race, so this patch fixes that. https://codereview.chromium.org/2244003003/diff/140001/src/js/promise.js#newc... src/js/promise.js:375: %PromiseRejectEvent(promise, r, false, false); On 2016/08/23 at 22:29:27, adamk wrote: > Passing two bools is not ideal, see suggestion in runtime-internal.cc. Fixed, now separated out a different function. https://codereview.chromium.org/2244003003/diff/140001/src/runtime/runtime-in... File src/runtime/runtime-internal.cc (right): https://codereview.chromium.org/2244003003/diff/140001/src/runtime/runtime-in... src/runtime/runtime-internal.cc:282: CONVERT_BOOLEAN_ARG_CHECKED(from_promise_reject, 3); On 2016/08/23 at 22:29:27, adamk wrote: > Rather than taking a second bool arg, you could split this runtime function into two and factor the logic out into a helper. Very good point, done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/22433)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm besides a few style/typo nits Agreed on the intuition for race and all as well. https://codereview.chromium.org/2244003003/diff/220001/src/js/promise.js File src/js/promise.js (right): https://codereview.chromium.org/2244003003/diff/220001/src/js/promise.js#newc... src/js/promise.js:313: // RejectPromise ( promise, reason )o Stray edit? vi user? https://codereview.chromium.org/2244003003/diff/220001/src/js/promise.js#newc... src/js/promise.js:323: if ((debugEvent && DEBUG_IS_ACTIVE)|| Nit: please add a space between ")" and "||" https://codereview.chromium.org/2244003003/diff/220001/src/runtime/runtime-in... File src/runtime/runtime-internal.cc (right): https://codereview.chromium.org/2244003003/diff/220001/src/runtime/runtime-in... src/runtime/runtime-internal.cc:313: if (promise_on_stack->IsJSObject()) Style nit: please add curlies since this doesn't all fir on one line.
The CQ bit was checked by littledan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kozyatinskiy@chromium.org, adamk@chromium.org Link to the patchset: https://codereview.chromium.org/2244003003/#ps240001 (title: "Fixed typos")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #13 (id:240001)
Message was sent while issue was closed.
Description was changed from ========== Change which ExceptionEvents are triggered by Promises To make async/await catch prediction work well, this patch regularizes the exception events sent to DevTools from various places in the Promise lifecycle. The core is that there should be an exception event when the rejection first starts, rather than when it is propagated. - Several cases within Promise code which propagate errors are modified to not trigger a new ExceptionEvent in that case, such as .then on a rejected Promise and returning a rejected Promise from .then, as well as Promise.race and Promise.all. - Make Promise.reject() create an ExceptionEvent, subject to catch prediction based on the Promise stack. This is important so that, e.g., if "await Promise.reject()" will trigger a new throw (rather than a silent rethrow of something that never triggered an event in the first place). BUG=v8:5167 ========== to ========== Change which ExceptionEvents are triggered by Promises To make async/await catch prediction work well, this patch regularizes the exception events sent to DevTools from various places in the Promise lifecycle. The core is that there should be an exception event when the rejection first starts, rather than when it is propagated. - Several cases within Promise code which propagate errors are modified to not trigger a new ExceptionEvent in that case, such as .then on a rejected Promise and returning a rejected Promise from .then, as well as Promise.race and Promise.all. - Make Promise.reject() create an ExceptionEvent, subject to catch prediction based on the Promise stack. This is important so that, e.g., if "await Promise.reject()" will trigger a new throw (rather than a silent rethrow of something that never triggered an event in the first place). BUG=v8:5167 Committed: https://crrev.com/013e49f73cab96c49df284312f8316e572df3a96 Cr-Commit-Position: refs/heads/master@{#38847} ==========
Message was sent while issue was closed.
Patchset 13 (id:??) landed as https://crrev.com/013e49f73cab96c49df284312f8316e572df3a96 Cr-Commit-Position: refs/heads/master@{#38847} |
