DevTools: use async operation id instead of AsyncCallChain

DevTools: use async operation id instead of AsyncCallChain

Debugger agent API now allows to assign asinc operation a unique identifier and use it to trace that operation. Async operation id returned by traceAsyncOperationStarting is always valid and greater than zero (HashMap would crash on store/lookup of special values like -1 and 0 in it, to avoid that we just make sure that the key is always >0) hence there is no need to check if its valid or not anymore.

If the client needs to report that a callback is starting executing but its operation id is unknown (e.g. because the tracking was off when the operation started) it just passes InspectorDebuggerAgent::unknownAsyncOperationId as it id.

BUG=439376
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188733

[yurys, 2015-01-19 15:46:06]

yurys@chromium.org changed reviewers:
+ aandrey@chromium.org

[aandrey, 2015-01-20 14:33:36]

looks good

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
File Source/core/inspector/AsyncCallTracker.cpp (right):

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
Source/core/inspector/AsyncCallTracker.cpp:56: const int noOperationId = 0;
noAsyncOperationId or nullAsyncOperationId?
make a public const?

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
Source/core/inspector/AsyncCallTracker.cpp:129: AsyncCallChainMap<int>
m_asyncOperationCallChains;
remove?

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
Source/core/inspector/AsyncCallTracker.cpp:401: int id =
data->nextAsyncOperationUniqueId();
remove, use a Set of operationId's

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1098: int
InspectorDebuggerAgent::createAsyncCallChain(PassRefPtrWillBeRawPtr<AsyncCallStack>
callStack)
inline this method?

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1103: int id =
m_nextAsyncOperationId++;
should be circular, never negative

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1109: void
InspectorDebuggerAgent::didCreateAsyncCallChain(AsyncCallChain* chain)
this can be inlined too

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1139: AsyncCallChain* chain =
m_asyncOperations.get(operationId);
ASSERT(operationId > 0);

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1162:
RefPtrWillBeRawPtr<AsyncCallChain> chain = m_asyncOperations.take(operationId);
ditto

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1165:
m_asyncOperationsForStepInto.remove(chain);
can it be HashSet<int> now?

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/V8Asyn...
File Source/core/inspector/V8AsyncCallTracker.cpp (right):

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/V8Asyn...
Source/core/inspector/V8AsyncCallTracker.cpp:135:
m_debuggerAgent->traceAsyncCallbackStarting(state->isolate(), 0);
0 -> public const

[yurys, 2015-01-20 17:15:39]

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
File Source/core/inspector/AsyncCallTracker.cpp (right):

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
Source/core/inspector/AsyncCallTracker.cpp:56: const int noOperationId = 0;
On 2015/01/20 14:33:35, aandrey wrote:
> noAsyncOperationId or nullAsyncOperationId?
> make a public const?

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
Source/core/inspector/AsyncCallTracker.cpp:129: AsyncCallChainMap<int>
m_asyncOperationCallChains;
On 2015/01/20 14:33:36, aandrey wrote:
> remove?

Removed nextAsyncOperationUniqueId() instead and now reuse async operation id
generated by the debugger agent. Is used to track operations going on inside
this particular context.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/AsyncC...
Source/core/inspector/AsyncCallTracker.cpp:401: int id =
data->nextAsyncOperationUniqueId();
On 2015/01/20 14:33:35, aandrey wrote:
> remove, use a Set of operationId's

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1098: int
InspectorDebuggerAgent::createAsyncCallChain(PassRefPtrWillBeRawPtr<AsyncCallStack>
callStack)
On 2015/01/20 14:33:36, aandrey wrote:
> inline this method?

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1103: int id =
m_nextAsyncOperationId++;
On 2015/01/20 14:33:36, aandrey wrote:
> should be circular, never negative

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1109: void
InspectorDebuggerAgent::didCreateAsyncCallChain(AsyncCallChain* chain)
On 2015/01/20 14:33:36, aandrey wrote:
> this can be inlined too

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1139: AsyncCallChain* chain =
m_asyncOperations.get(operationId);
On 2015/01/20 14:33:36, aandrey wrote:
> ASSERT(operationId > 0);

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1162:
RefPtrWillBeRawPtr<AsyncCallChain> chain = m_asyncOperations.take(operationId);
On 2015/01/20 14:33:36, aandrey wrote:
> ditto

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/Inspec...
Source/core/inspector/InspectorDebuggerAgent.cpp:1165:
m_asyncOperationsForStepInto.remove(chain);
On 2015/01/20 14:33:36, aandrey wrote:
> can it be HashSet<int> now?

Done.

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/V8Asyn...
File Source/core/inspector/V8AsyncCallTracker.cpp (right):

https://codereview.chromium.org/855383002/diff/1/Source/core/inspector/V8Asyn...
Source/core/inspector/V8AsyncCallTracker.cpp:135:
m_debuggerAgent->traceAsyncCallbackStarting(state->isolate(), 0);
On 2015/01/20 14:33:36, aandrey wrote:
> 0 -> public const

Done.

[yurys, 2015-01-21 06:43:34]

The CQ bit was checked by yurys@chromium.org

[yurys, 2015-01-21 06:43:42]

The CQ bit was unchecked by yurys@chromium.org

[yurys, 2015-01-21 06:48:09]

PTAL

[aandrey, 2015-01-21 08:39:45]

lgtm

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
Source/core/inspector/InspectorDebuggerAgent.cpp:135: ,
m_lastAsyncOperationId(1)
1 -> 0

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
Source/core/inspector/InspectorDebuggerAgent.cpp:1070:
RefPtrWillBeRawPtr<AsyncCallChain> chain;
= nullptr (otherwise Oilpan will complain)

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
Source/core/inspector/InspectorDebuggerAgent.cpp:1077: while (true) {
nit: maybe do-while loop as it was in AsyncCallTracker?

[yurys, 2015-01-21 08:50:05]

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
Source/core/inspector/InspectorDebuggerAgent.cpp:135: ,
m_lastAsyncOperationId(1)
On 2015/01/21 08:39:45, aandrey wrote:
> 1 -> 0

Done.

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
Source/core/inspector/InspectorDebuggerAgent.cpp:1070:
RefPtrWillBeRawPtr<AsyncCallChain> chain;
On 2015/01/21 08:39:45, aandrey wrote:
> = nullptr (otherwise Oilpan will complain)

Done.

https://codereview.chromium.org/855383002/diff/80001/Source/core/inspector/In...
Source/core/inspector/InspectorDebuggerAgent.cpp:1077: while (true) {
On 2015/01/21 08:39:45, aandrey wrote:
> nit: maybe do-while loop as it was in AsyncCallTracker?

Done.

[yurys, 2015-01-21 08:50:08]

The CQ bit was checked by yurys@chromium.org

[commit-bot: I haz the power, 2015-01-21 08:50:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/855383002/100001

[commit-bot: I haz the power, 2015-01-21 11:43:54]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188733

[aandrey, 2015-01-21 12:51:27]

https://codereview.chromium.org/855383002/diff/100001/Source/core/inspector/I...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/855383002/diff/100001/Source/core/inspector/I...
Source/core/inspector/InspectorDebuggerAgent.cpp:1082:
m_asyncOperations.set(m_lastAsyncOperationId, chain);
there should be corresponding m_asyncOperations.remove() somewhere

[yurys, 2015-01-21 13:05:22]

https://codereview.chromium.org/855383002/diff/100001/Source/core/inspector/I...
File Source/core/inspector/InspectorDebuggerAgent.cpp (right):

https://codereview.chromium.org/855383002/diff/100001/Source/core/inspector/I...
Source/core/inspector/InspectorDebuggerAgent.cpp:1082:
m_asyncOperations.set(m_lastAsyncOperationId, chain);
On 2015/01/21 12:51:27, aandrey wrote:
> there should be corresponding m_asyncOperations.remove() somewhere

Good catch, thank you! Please take a look at the fix
https://codereview.chromium.org/849333003/