Bug 55816 – Web Inspector: fix layout tests flakiness.

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

[Pavel Feldman]

A number of fixes were applied, there were multiple sources of flakiness.
- Order of issuing of evaluateForTestInFrontend was not guaranteed on the backend side (InspectorAgent side)
- Order of dispatching using timeouts was guaranteed via queueing. Source of all kinds of pains on SnowLeopard Release (inspector.js)
- Timeline now uses instrumentation calls, not UI representation while collecting events for tests
- No reloads in debugger tests that don't need them
- Forcing layout in timeline via calculating offsetHeight

[Pavel Feldman]

Created attachment 84846 [details]
Patch

[Pavel Feldman]

Created attachment 84849 [details]
[PATCH] Same with chromium baselines.

[Yury Semikhatsky]

Comment on attachment 84846 [details]
Patch

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

> LayoutTests/http/tests/inspector/inspector-test.js:331
> +        var intermediate = document.createElement("div");

Why do you need this?

> LayoutTests/inspector/console/console-uncaught-exception-expected.txt:6
> +console-uncaught-exception.html:40Error: Exception in inline script.

Chromium has stack traces for uncaught exceptions when front-end is open, this is why we need to reload inspected page. I don't see changes to the chromium-specific expectations in this patch. r- for this.

[Yury Semikhatsky]

Comment on attachment 84849 [details]
[PATCH] Same with chromium baselines.

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

> LayoutTests/platform/chromium/inspector/console/console-uncaught-exception-expected.txt:6
> +console-uncaught-exception.html:40Uncaught Error: Exception in inline script.aconsole-uncaught-exception.html:40bconsole-uncaught-exception.html:45(anonymous function)console-uncaught-exception.html:48

How can you be sure that frontend is already loaded when the exceptions are thrown?

> Source/WebCore/inspector/InspectorAgent.cpp:399
> +    m_pendingEvaluateTestCommands.clear();

Why do we need this magic?

[Pavel Feldman]

(In reply to comment #3)
> (From update of attachment 84846 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84846&action=review
> 
> > LayoutTests/http/tests/inspector/inspector-test.js:331
> > +        var intermediate = document.createElement("div");
> 
> Why do you need this?
> 

So that output was not the top-level element and was not generating DOM events upon every addResult invocation.

> > LayoutTests/inspector/console/console-uncaught-exception-expected.txt:6
> > +console-uncaught-exception.html:40Error: Exception in inline script.
> 
> Chromium has stack traces for uncaught exceptions when front-end is open, this is why we need to reload inspected page. I don't see changes to the chromium-specific expectations in this patch. r- for this.

Uploading patch with proper expectations and flow.


> How can you be sure that frontend is already loaded when the exceptions are thrown?
> 

See above.

> > Source/WebCore/inspector/InspectorAgent.cpp:399
> > +    m_pendingEvaluateTestCommands.clear();
> 
> Why do we need this magic?

When single inspector agent is reused, we'd like to clear potential pending messages so that they don't get fired with the next front-end. This is a paranoid check.

[Pavel Feldman]

Created attachment 84852 [details]
[PATCH] Same with everything passing in debug.

[Yury Semikhatsky]

(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 84846 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=84846&action=review
> > 
> > > LayoutTests/http/tests/inspector/inspector-test.js:331
> > > +        var intermediate = document.createElement("div");
> > 
> > Why do you need this?
> > 
> 
> So that output was not the top-level element and was not generating DOM events upon every addResult invocation.

A comment in the code would be very helpful for people who will read this code later.

[Pavel Feldman]

Created attachment 84853 [details]
Patch

[Pavel Feldman]

Committed r80416: <http://trac.webkit.org/changeset/80416>