Bug 80994 – Web Inspector: add didCancelFrame timeline event

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Andrey Kosyakov]

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.

[Andrey Kosyakov]

Created attachment 131628 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Pavel Feldman]

Comment on attachment 131628 [details]
Patch

Please provide a test.

[Andrey Kosyakov]

Created attachment 131671 [details]
Patch

[Andrey Kosyakov]

Created attachment 131676 [details]
Patch

[Pavel Feldman]

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 ?

[Build Bot]

Comment on attachment 131671 [details]
Patch

Attachment 131671 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11942659

[Gustavo Noronha (kov)]

Comment on attachment 131671 [details]
Patch

Attachment 131671 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11947774

[Collabora GTK+ EWS bot]

Comment on attachment 131671 [details]
Patch

Attachment 131671 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11945747

[Andrey Kosyakov]

Created attachment 131813 [details]
Patch

[Pavel Feldman]

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.

[Gustavo Noronha (kov)]

Comment on attachment 131813 [details]
Patch

Attachment 131813 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11949926

[Build Bot]

Comment on attachment 131813 [details]
Patch

Attachment 131813 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11949938

[Andrey Kosyakov]

Committed r110706: <http://trac.webkit.org/changeset/110706>