Bug 112172 - Web Inspector: do not aggregate non-main thread timeline events, handle them a generic fashion
Summary: Web Inspector: do not aggregate non-main thread timeline events, handle them ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andrey Kosyakov
URL:
Keywords:
Depends on: 112168
Blocks:
  Show dependency tree / graph
 
Reported: 2013-03-12 11:24 PDT by Andrey Kosyakov
Modified: 2013-03-14 07:07 PDT (History)
10 users (show)

See Also:

Attachments
Patch (24.43 KB, patch)
2013-03-12 11:34 PDT, Andrey Kosyakov 		no flags Details | Formatted Diff | Diff
Patch (24.45 KB, patch)
2013-03-13 01:21 PDT, Andrey Kosyakov 		no flags Details | Formatted Diff | Diff
Patch (24.45 KB, patch)
2013-03-13 10:01 PDT, Andrey Kosyakov 		vsevik: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>