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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1095943002: DevTools: [console] Logged promise rejections do not change state once handled (Closed)

Created:
5 years, 8 months ago by pfeldman
Modified:
5 years, 8 months ago
Reviewers:
kozy, dgozman, sergeyv
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, vivekg, vivekg_samsung, yurys+blink_chromium.org, lushnikov+blink_chromium.org, blink-reviews-bindings_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, arv+blink, sergeyv+blink_chromium.org, aandrey+blink_chromium.org, kozyatinskiy+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

DevTools: [console] Logged promise rejections do not change state once handled BUG=477557 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194136

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : w/ test #

Patch Set 4 : for bots #

Patch Set 5 : #

Patch Set 6 : images optimized, worker tests added #

Patch Set 7 : thread-local sequence numbers #

Total comments: 13

Patch Set 8 : tests updated #

Patch Set 9 : tests unflaked #

Patch Set 10 : comments addressed #

Total comments: 13

Patch Set 11 : review comments addressed. #

Patch Set 12 : #

Patch Set 13 : for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+463 lines, -80 lines) Patch
M LayoutTests/http/tests/inspector/console-test.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +7 lines, -0 lines 0 comments Download
A LayoutTests/inspector/console/console-revoke-error.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +65 lines, -0 lines 0 comments Download
A LayoutTests/inspector/console/console-revoke-error-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +21 lines, -0 lines 0 comments Download
A LayoutTests/inspector/console/console-revoke-error-in-worker.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +57 lines, -0 lines 0 comments Download
A LayoutTests/inspector/console/console-revoke-error-in-worker-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +15 lines, -0 lines 0 comments Download
A LayoutTests/inspector/console/resources/worker-with-defer-handled-promise.js View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M LayoutTests/inspector/sources/debugger/promise-pane-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M Source/bindings/core/v8/RejectedPromises.h View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M Source/bindings/core/v8/RejectedPromises.cpp View 1 2 3 4 5 6 7 8 9 10 6 chunks +50 lines, -1 line 0 comments Download
M Source/bindings/core/v8/V8Initializer.cpp View 1 2 4 chunks +15 lines, -6 lines 0 comments Download
M Source/core/frame/ConsoleTypes.h View 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/inspector/ConsoleMessage.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +6 lines, -0 lines 0 comments Download
M Source/core/inspector/ConsoleMessage.cpp View 1 2 3 4 5 6 7 8 9 10 3 chunks +20 lines, -0 lines 0 comments Download
M Source/core/inspector/InspectorConsoleAgent.cpp View 1 2 3 4 5 6 7 8 9 10 2 chunks +5 lines, -0 lines 0 comments Download
M Source/devtools/front_end/Images/src/optimize_png.hashes View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -2 lines 0 comments Download
M Source/devtools/front_end/Images/src/svg2png.hashes View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -2 lines 0 comments Download
M Source/devtools/front_end/Images/src/toolbarButtonGlyphs.svg View 1 2 3 4 5 6 7 8 9 10 3 chunks +35 lines, -6 lines 0 comments Download
M Source/devtools/front_end/Images/toolbarButtonGlyphs.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M Source/devtools/front_end/Images/toolbarButtonGlyphs_2x.png View 1 2 3 4 5 6 7 8 9 10 Binary file 0 comments Download
M Source/devtools/front_end/bindings/PresentationConsoleMessageHelper.js View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M Source/devtools/front_end/console/ConsoleView.js View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +22 lines, -5 lines 0 comments Download
M Source/devtools/front_end/console/ConsoleViewMessage.js View 1 2 3 4 5 6 7 8 9 10 7 chunks +22 lines, -13 lines 0 comments Download
M Source/devtools/front_end/console/consoleView.css View 6 chunks +9 lines, -2 lines 0 comments Download
M Source/devtools/front_end/main/Main.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +9 lines, -5 lines 0 comments Download
M Source/devtools/front_end/sdk/ConsoleModel.js View 1 2 3 4 5 6 7 8 9 10 13 chunks +76 lines, -34 lines 0 comments Download
M Source/devtools/front_end/ui/smallIcon.css View 3 chunks +6 lines, -0 lines 0 comments Download
M Source/devtools/protocol.json View 1 2 3 4 5 6 7 8 9 10 2 chunks +4 lines, -2 lines 0 comments Download
M public/web/WebConsoleMessage.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 15 (3 generated)
pfeldman
I'll need to optimize pngs.
5 years, 8 months ago (2015-04-18 20:38:01 UTC) #2
pfeldman
(i'm working on the tests for it, but it can already be looked at)
5 years, 8 months ago (2015-04-18 20:42:38 UTC) #3
kozy
https://codereview.chromium.org/1095943002/diff/120001/LayoutTests/inspector/console/console-revoke-error-expected.txt File LayoutTests/inspector/console/console-revoke-error-expected.txt (right): https://codereview.chromium.org/1095943002/diff/120001/LayoutTests/inspector/console/console-revoke-error-expected.txt#newcode11 LayoutTests/inspector/console/console-revoke-error-expected.txt:11: at createPromise (file:///Users/pfeldman/code/chromium/src/third_party/WebKit/LayoutTests/inspector/console/console-revoke-error.html:11:24) I heard that such tests can ...
5 years, 8 months ago (2015-04-20 13:35:07 UTC) #4
pfeldman
https://codereview.chromium.org/1095943002/diff/120001/LayoutTests/inspector/console/console-revoke-error-expected.txt File LayoutTests/inspector/console/console-revoke-error-expected.txt (right): https://codereview.chromium.org/1095943002/diff/120001/LayoutTests/inspector/console/console-revoke-error-expected.txt#newcode11 LayoutTests/inspector/console/console-revoke-error-expected.txt:11: at createPromise (file:///Users/pfeldman/code/chromium/src/third_party/WebKit/LayoutTests/inspector/console/console-revoke-error.html:11:24) On 2015/04/20 13:35:06, kozyatinskiy wrote: > ...
5 years, 8 months ago (2015-04-20 17:05:58 UTC) #5
kozy
https://codereview.chromium.org/1095943002/diff/180001/Source/core/inspector/ConsoleMessage.cpp File Source/core/inspector/ConsoleMessage.cpp (right): https://codereview.chromium.org/1095943002/diff/180001/Source/core/inspector/ConsoleMessage.cpp#newcode23 Source/core/inspector/ConsoleMessage.cpp:23: AtomicallyInitializedStaticReference(WTF::ThreadSpecific<SequenceNumber>, sequenceNumber, new WTF::ThreadSpecific<SequenceNumber>); AtomicallyInitializedStaticReference(WTF::ThreadSpecific<int>, sequenceNumber, 0); ++(*sequenceNumber);
5 years, 8 months ago (2015-04-20 17:17:34 UTC) #6
kozy
lgtm
5 years, 8 months ago (2015-04-20 17:21:29 UTC) #7
dgozman
https://codereview.chromium.org/1095943002/diff/180001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/1095943002/diff/180001/Source/devtools/front_end/console/ConsoleView.js#newcode564 Source/devtools/front_end/console/ConsoleView.js:564: relatedMessage.level = WebInspector.ConsoleMessage.MessageLevel.RevokedError; Why does view mutate model objects? ...
5 years, 8 months ago (2015-04-21 09:16:36 UTC) #8
pfeldman
https://codereview.chromium.org/1095943002/diff/180001/Source/devtools/front_end/console/ConsoleView.js File Source/devtools/front_end/console/ConsoleView.js (right): https://codereview.chromium.org/1095943002/diff/180001/Source/devtools/front_end/console/ConsoleView.js#newcode564 Source/devtools/front_end/console/ConsoleView.js:564: relatedMessage.level = WebInspector.ConsoleMessage.MessageLevel.RevokedError; On 2015/04/21 09:16:35, dgozman wrote: > ...
5 years, 8 months ago (2015-04-21 12:15:27 UTC) #9
dgozman
lgtm modulo re-filtering on updateMessage
5 years, 8 months ago (2015-04-21 13:26:01 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1095943002/240001
5 years, 8 months ago (2015-04-21 13:34:52 UTC) #13
commit-bot: I haz the power
Committed patchset #13 (id:240001) as https://src.chromium.org/viewvc/blink?view=rev&revision=194136
5 years, 8 months ago (2015-04-21 15:10:16 UTC) #14
alancutter (OOO until 2018)
5 years, 8 months ago (2015-04-23 05:08:47 UTC) #15
Message was sent while issue was closed. 
A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/1090583005/ by alancutter@chromium.org.

The reason for reverting is: The following tests were made leaky due to this
change:
virtual/slimmingpaint/fast/css/fontfaceset-add-remove-while-loading.html
inspector/console/console-revoke-error.html
inspector/console/console-uncaught-promise.html
inspector/sources/debugger/debugger-uncaught-promise-on-pause.html
inspector/sources/debugger/debugger-pause-on-promise-rejection.html
fast/css/fontfaceset-add-remove-while-loading.html
http/tests/serviceworker/fetch-request-resources.html
http/tests/fetch/window/fetch-base-https-other-https.html
http/tests/fetch/window/fetch.html
inspector-enabled/console/console-uncaught-promise-no-inspector.html
web-animations-api/player-finished-promise.html
web-animations-api/player-ready-promise.html

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/bu...

Confirmed with manual bisect..

Powered by Google App Engine
This is Rietveld 408576698