Bug 112172

Summary: Web Inspector: do not aggregate non-main thread timeline events, handle them a generic fashion
Product: WebKit Reporter: Andrey Kosyakov <caseq>
Component: Web Inspector (Deprecated)Assignee: Andrey Kosyakov <caseq>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, dglazkov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 112168    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch vsevik: review+

Description Andrey Kosyakov 2013-03-12 11:24:22 PDT 
This removes the logic to aggregate rasterize events in timeline agent and instead sends them, along with potential nested events, to front-end, similar to how main thread events are handled. These are later supposed to be collated with front-end event collation logic from bug 112168.
Comment 1 Andrey Kosyakov 2013-03-12 11:34:14 PDT 
Created attachment 192777 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-12 12:05:54 PDT 
Comment on attachment 192777 [details]
Patch

Attachment 192777 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17173227
Comment 3 Andrey Kosyakov 2013-03-13 01:21:38 PDT 
Created attachment 192879 [details]
Patch
Comment 4 Vsevolod Vlasov 2013-03-13 08:26:07 PDT 
Comment on attachment 192879 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192879&action=review

> Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:142
> +    return m_stack.isEmpty() || (m_stack.last().record->getString("type", &lastRecordType) && type == lastRecordType);

return !m_stack.isEmpty() && (m_stack.last().record->getString("type", &lastRecordType) && type == lastRecordType);

> Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:245
> +    ASSERT(!state.rasterizeStartStackLevel);

Looks like rasterizeStartStackLevel could be a boolean (isInRasterize).
Comment 5 Andrey Kosyakov 2013-03-13 10:01:13 PDT 
Created attachment 192941 [details]
Patch
Comment 6 Andrey Kosyakov 2013-03-13 10:05:10 PDT 
(In reply to comment #4)
> (From update of attachment 192879 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192879&action=review
> 
> > Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:142
> > +    return m_stack.isEmpty() || (m_stack.last().record->getString("type", &lastRecordType) && type == lastRecordType);
> 
> return !m_stack.isEmpty() && (m_stack.last().record->getString("type", &lastRecordType) && type == lastRecordType);

This is written as intended (though may be the method name is unfortunate), not having the top event is considered ok for the purpose of validation -- we hit this case if we started recording in the middle of the event.

> > Source/WebCore/inspector/TimelineTraceEventProcessor.cpp:245
> > +    ASSERT(!state.rasterizeStartStackLevel);
> 
> Looks like rasterizeStartStackLevel could be a boolean (isInRasterize).

Fair, this is atavism of an earlier implementation. Fixed.
Comment 7 Andrey Kosyakov 2013-03-14 07:07:27 PDT 
Committed r145809: <http://trac.webkit.org/changeset/145809>