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

Issue 383123009: DevTools: Support async call stacks for FileSystem API (part 1). (Closed)

Created:
6 years, 5 months ago by aandrey
Modified:
6 years, 5 months ago
Reviewers:
kinuko, nhiroki, yurys
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, tzik, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, Inactive, devtools-reviews_chromium.org, arv+blink, sergeyv+blink_chromium.org, aandrey+blink_chromium.org, kinuko+fileapi
Project:
blink
Visibility:
Public.

Description

DevTools: Support async call stacks for FileSystem API (part 1). This patch does not yet tracks async stacks for FileWriter and FileReader. BUG=272416, 390863 R=kinuko@chromium.org, nhiroki@chromium.org, yurys Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178149

Patch Set 1 : #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+374 lines, -16 lines) Patch
A LayoutTests/http/tests/inspector/filesystem/async-callstack-filesystem.html View 1 chunk +111 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/inspector/filesystem/async-callstack-filesystem-expected.txt View 1 chunk +60 lines, -0 lines 0 comments Download
M Source/core/inspector/AsyncCallStackTracker.h View 3 chunks +10 lines, -0 lines 0 comments Download
M Source/core/inspector/AsyncCallStackTracker.cpp View 8 chunks +60 lines, -2 lines 1 comment Download
M Source/core/inspector/CodeGeneratorInstrumentation.py View 4 chunks +7 lines, -0 lines 0 comments Download
M Source/core/inspector/InspectorDebuggerAgent.h View 2 chunks +5 lines, -0 lines 0 comments Download
M Source/core/inspector/InspectorDebuggerAgent.cpp View 1 chunk +30 lines, -0 lines 0 comments Download
M Source/core/inspector/InspectorInstrumentation.idl View 3 chunks +16 lines, -1 line 0 comments Download
M Source/modules/filesystem/DOMFileSystem.h View 4 chunks +20 lines, -4 lines 1 comment Download
M Source/modules/filesystem/FileSystemCallbacks.cpp View 7 chunks +55 lines, -9 lines 1 comment Download

Messages

Total messages: 14 (0 generated)
aandrey
6 years, 5 months ago (2014-07-11 18:34:25 UTC) #1
aandrey
Extracted the SnapshotFileCallback move into FileSystemCallbacks.h into the patch https://codereview.chromium.org/388923004/
6 years, 5 months ago (2014-07-11 19:08:30 UTC) #2
nhiroki
LGTM for Source/modules/filesystem/
6 years, 5 months ago (2014-07-14 07:39:45 UTC) #3
yurys
https://codereview.chromium.org/383123009/diff/60001/Source/core/inspector/AsyncCallStackTracker.cpp File Source/core/inspector/AsyncCallStackTracker.cpp (right): https://codereview.chromium.org/383123009/diff/60001/Source/core/inspector/AsyncCallStackTracker.cpp#newcode357 Source/core/inspector/AsyncCallStackTracker.cpp:357: setCurrentAsyncCallChain(context, data->m_fileSystemCallChains.get(callback)); style: consider extracting setCurrentAsyncCallChain out of the ...
6 years, 5 months ago (2014-07-14 14:55:55 UTC) #4
yurys
lgtm
6 years, 5 months ago (2014-07-14 15:14:10 UTC) #5
aandrey
The CQ bit was checked by aandrey@chromium.org
6 years, 5 months ago (2014-07-14 15:21:36 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aandrey@chromium.org/383123009/60001
6 years, 5 months ago (2014-07-14 15:21:58 UTC) #7
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: mac_blink_rel on tryserver.blink ...
6 years, 5 months ago (2014-07-14 16:36:00 UTC) #8
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 5 months ago (2014-07-14 17:42:42 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/16783)
6 years, 5 months ago (2014-07-14 17:42:43 UTC) #10
aandrey
The CQ bit was checked by aandrey@chromium.org
6 years, 5 months ago (2014-07-15 07:44:55 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aandrey@chromium.org/383123009/60001
6 years, 5 months ago (2014-07-15 07:45:29 UTC) #12
commit-bot: I haz the power
Change committed as 178149
6 years, 5 months ago (2014-07-15 08:18:06 UTC) #13
pfeldman
6 years, 5 months ago (2014-07-16 12:09:59 UTC) #14
Message was sent while issue was closed.
A revert of this CL has been created in
https://codereview.chromium.org/397843009/ by pfeldman@chromium.org.

The reason for reverting is: This seems to add a lot of complexity into async
code + violates the modularity: core/inspector should not be aware of the
filesystems..

Powered by Google App Engine
This is Rietveld 408576698