Bug 80994 - Web Inspector: add didCancelFrame timeline event
Summary: Web Inspector: add didCancelFrame timeline event
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:
Blocks:
 
Reported: 2012-03-13 08:53 PDT by Andrey Kosyakov
Modified: 2012-03-14 09:06 PDT (History)
16 users (show)

See Also:


Attachments
Patch (11.99 KB, patch)
2012-03-13 08:57 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (16.24 KB, patch)
2012-03-13 10:57 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (17.20 KB, patch)
2012-03-13 11:18 PDT, Andrey Kosyakov
no flags Details | Formatted Diff | Diff
Patch (17.95 KB, patch)
2012-03-14 02:25 PDT, Andrey Kosyakov
pfeldman: review+
gustavo: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Kosyakov 2012-03-13 08:53:28 PDT
This adds a an ability to cancel certain timeline events, unless they've been followed by other events.
The above is used to implement didCancelFrame() event, which is fired when a platform decides not to render frame after didBeginFrame() was called.
Comment 1 Andrey Kosyakov 2012-03-13 08:57:23 PDT
Created attachment 131628 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-13 08:59:44 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Pavel Feldman 2012-03-13 09:48:25 PDT
Comment on attachment 131628 [details]
Patch

Please provide a test.
Comment 4 Andrey Kosyakov 2012-03-13 10:57:49 PDT
Created attachment 131671 [details]
Patch
Comment 5 Andrey Kosyakov 2012-03-13 11:18:24 PDT
Created attachment 131676 [details]
Patch
Comment 6 Pavel Feldman 2012-03-13 11:30:04 PDT
Comment on attachment 131671 [details]
Patch

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

Overall looks good, a bunch of nits to be fixed prior to landing.

> Source/WebCore/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

Could you generate fresh ChangeLogs?

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:177
> +    pushCancellableRecord(InspectorObject::create(), TimelineRecordType::BeginFrame);

"Cancelable" ?

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:423
>          entry.record->setArray("children", entry.children);

You should move children to under the cancelable as well.

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:466
> +        didCompleteCurrentRecord(m_recordStack.last().type);

didCompleteCurrentRecord is not applicable to atomic events, you should do appendRecord instead. I am not sure you even need a stack for them.

> Source/WebCore/testing/Internals.cpp:676
> +void Internals::emitTimelineDidBeginFrame()

emitInspectorDidBeginFrame ?
Comment 7 Build Bot 2012-03-13 12:27:41 PDT
Comment on attachment 131671 [details]
Patch

Attachment 131671 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11942659
Comment 8 Gustavo Noronha (kov) 2012-03-13 15:00:15 PDT
Comment on attachment 131671 [details]
Patch

Attachment 131671 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11947774
Comment 9 Collabora GTK+ EWS bot 2012-03-13 17:23:36 PDT
Comment on attachment 131671 [details]
Patch

Attachment 131671 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11945747
Comment 10 Andrey Kosyakov 2012-03-14 02:25:15 PDT
Created attachment 131813 [details]
Patch
Comment 11 Pavel Feldman 2012-03-14 02:36:44 PDT
Comment on attachment 131813 [details]
Patch

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

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:459
> +    m_recordStack.append(TimelineRecordEntry(record.release(), data, InspectorArray::create(), type, true));

I'd be great if we could make this children array nullable.
Comment 12 Gustavo Noronha (kov) 2012-03-14 03:19:01 PDT
Comment on attachment 131813 [details]
Patch

Attachment 131813 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11949926
Comment 13 Build Bot 2012-03-14 03:43:11 PDT
Comment on attachment 131813 [details]
Patch

Attachment 131813 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11949938
Comment 14 Andrey Kosyakov 2012-03-14 09:06:06 PDT
Committed r110706: <http://trac.webkit.org/changeset/110706>