Bug 102247 – webgl-background-color.html started failing

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dimitri Glazkov (Google)]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=platform%2Fchromium%2Fvirtual%2Fthreaded%2Fcompositing%2Fwebgl%2Fwebgl-background-color.html

It's hard to say why.

[James Robinson]

Created attachment 174223 [details]
Patch

[James Robinson]

The issue here was actually a timeout due to chromium http://src.chromium.org/viewvc/chrome?view=rev&revision=167537.  WebViewHost's d'tor navigates the page (synchronously) to about:blank:

http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Tools/DumpRenderTree/chromium/WebViewHost.cpp&exact_package=chromium&q=~WebViewHost&type=cs&l=1446

one consequence of which is disabling compositing mode, which triggers this finishAllRendering() call:

http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp&exact_package=chromium&q=WebViewImpl.cpp&type=cs&l=3946

which attempts to make a synchronous call to the compositor thread.  Since the compositor thread is destroyed first, this call went nowhere and the main thread simply sat blocked until it was killed by the harness.

I think the reason http://src.chromium.org/viewvc/chrome?view=rev&revision=167537 tickled this was because of some subtleties in chromium's thread shutdown where the thread was living for longer, but I'm still tracking down exactly how this happened.

[James Robinson]

This patch also makes TestShell's behavior more similar to chrome's, where the compositor thread always outlives WebView instances.

[Adrienne Walker]

Comment on attachment 174223 [details]
Patch

R=me.  This really needs a better comment.

[Dimitri Glazkov (Google)]

(In reply to comment #4)
> (From update of attachment 174223 [details])
> R=me.  This really needs a better comment.

I think this change needs a meme.

[James Robinson]

Created attachment 174246 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 174246 [details]
Patch for landing

Clearing flags on attachment: 174246

Committed r134667: <http://trac.webkit.org/changeset/134667>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ojan Vafai]

Is there a way to add asserts instead of just a comment?

[James Robinson]

They aren't passing yet, although I think r134667 was a solid bug fix and should cut down on flaky crashes.  There's one more race that https://codereview.chromium.org/11363254 should fix.  Leaving open until it goes green...

[James Robinson]

Fixed by http://src.chromium.org/viewvc/chrome?view=rev&revision=168037, removed failing expectation in http://trac.webkit.org/changeset/134857