Bug 62703 – Web Inspector: [Qt] almost all styles tests are flaky

• Component: Web Inspector (Deprecated)
• Status: RESOLVED INVALID

[Ilya Tikhonovsky]

Many styles tests assign a test function as a sniffer for WebInspector.StylesSidebarPane.prototype._rebuildUpdate function
but sometimes not all the information is received/processed properly when test function is called.
It'd be nice to run the next step a little bit later.

[Ilya Tikhonovsky]

Created attachment 97257 [details]
[patch] initial version

[Pavel Feldman]

Comment on attachment 97257 [details]
[patch] initial version

We should instead add a sniffer prior to selecting node + make selectNode force synchronous styles update.

[Ilya Tikhonovsky]

Created attachment 97618 [details]
[patch] second version

[Pavel Feldman]

Comment on attachment 97618 [details]
[patch] second version

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

> LayoutTests/inspector/styles/styles-add-invalid-property.html:13
> +        InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", step0);

I don't see why this dispatch is necessary. _rebuildUpdate sniffer should take care of things. I thought we agreed to force synchronous styles recalc. You could make a utility showElementsPanelAndUpdateStyles method for that.

> LayoutTests/inspector/styles/styles-update-from-js.html:45
> +console.trace();

This is probably not intentional

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:241
> +    _reloadAllStylesDelayed: function()

_reloadAllStyles

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:254
> +    _reloadAllStyles: function()

_scheduleReloadAllStyles

[Ilya Tikhonovsky]

Comment on attachment 97618 [details]
[patch] second version

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

>> LayoutTests/inspector/styles/styles-add-invalid-property.html:13
>> +        InspectorTest.addSniffer(WebInspector.StylesSidebarPane.prototype, "_rebuildUpdate", step0);
> 
> I don't see why this dispatch is necessary. _rebuildUpdate sniffer should take care of things. I thought we agreed to force synchronous styles recalc. You could make a utility showElementsPanelAndUpdateStyles method for that.

There are two different things here. 
1) we want to switch the active panel
_rebuildUpdate sniffer can't be used as the initiator of the first and next steps directly because it is called first time as a result of showPanel and second time as result of selectNodeWithId (asynchronously).
We need to count such calls or assign this sniffer only when we have a guarantee that it will not be called second time for some reason. I decided to use second approach because it is more stable. BTW as I know you r- the patch with counter prepared by apavlov.

2) we want to make styles recalculation synchronous
Unfortunately we have at least two async call to backend and at least one setTimeout call between the moment when we modify styles at frontend and the moment when new styles are received and applied to the UI. 
I removed setTimeout call for the tests and found that some tests got broken because it's test steps have no direct correlation with _rebuildUpdate.

[Ilya Tikhonovsky]

Created attachment 97779 [details]
[patch] third version