Bug 72621 – Web Inspector: dispatch messages from the front-end to the backend asynchronously.

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

[Pavel Feldman]

We should align the way we dispatch messages from the front-end to backend across the environments:

- WebKit has it synchronoulsly
- Chromium has it asynchronously
- Remote debugging has it asynchronously

Making it asynchronous made a number of flaky Qt tests pass.

[Pavel Feldman]

Created attachment 115601 [details]
Patch

[Timothy Hatcher]

Comment on attachment 115601 [details]
Patch

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

> Source/WebCore/inspector/InspectorConsoleAgent.cpp:138
> +    m_inspectorState->setBoolean(ConsoleAgentState::consoleMessagesEnabled, false);

How is this related to async?

> LayoutTests/inspector/timeline/timeline-script-tag-1.html:40
> +            else if (record.type === WebInspector.TimelineAgent.RecordType.ParseHTML) {
> +                var children = [];
> +                for (var i = 0; i < record.children.length; ++i) {
> +                    if (record.children[i].type !== WebInspector.TimelineAgent.RecordType.RecalculateStyles)
> +                        children.push(record.children[i]);
> +                }
> +                record.children = children;

How is changing this related to async dispatch? This wasn't one of the skipped tests so why does it need to change?

[Pavel Feldman]

Comment on attachment 115601 [details]
Patch

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

>> Source/WebCore/inspector/InspectorConsoleAgent.cpp:138
>> +    m_inspectorState->setBoolean(ConsoleAgentState::consoleMessagesEnabled, false);
> 
> How is this related to async?

This was actually a bug that was uncovered while running Qt tests after migrating to the async messaging. There was a race condition making console messages reported twice upon re-connecting to the backend: first event was generated upon console call, while the second copy was sent in return to the ::enable command.

>> LayoutTests/inspector/timeline/timeline-script-tag-1.html:40
>> +                record.children = children;
> 
> How is changing this related to async dispatch? This wasn't one of the skipped tests so why does it need to change?

It is making this test pass. Now that the commands are asynchronous, extra style recalculation event is generated which this test is not testing.

[Timothy Hatcher]

Those are good comments for the ChangeLog.

[Pavel Feldman]

Created attachment 115786 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 115786 [details]
Patch

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

> Source/WebCore/inspector/InspectorConsoleAgent.cpp:138
> +    m_inspectorState->setBoolean(ConsoleAgentState::consoleMessagesEnabled, false);

It looks like we may clear inspector state once when front-end is disconnecting instead of spreading this logic over individual agents.

> Source/WebCore/inspector/InspectorFrontendClientLocal.cpp:83
> +            m_inspectorController->dispatchMessageFromFrontend(m_messages.takeFirst());

Please add a comment here that this task may be already destroyed after dispatchMessageFromFrontend call.

[Pavel Feldman]

Committed r100756: <http://trac.webkit.org/changeset/100756>