Bug 56148 – [chromium] Graphics Context is not properly recovered if the GPU process dies.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Alexey Marinichev]

Because of the race in the lost context detection, recreated context will be thrown away when GPU process restarts.

[Alexey Marinichev]

Created attachment 85397 [details]
Patch

[Kenneth Russell]

Comment on attachment 85397 [details]
Patch

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

Good work fixing these deep issues; r- for a couple of relatively minor concerns.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h:90
> +    PassRefPtr<LayerChromium> rootLayer() { return m_rootLayer.release(); }

I've already given you this feedback offline, but in this form this method should be named something like "releaseRootLayer".

> Source/WebKit/chromium/src/WebViewImpl.cpp:2436
> +            getCompositorContextAttributes(), m_page->chrome(), GraphicsContext3D::RenderDirectlyToHostWindow);

Are any similar improvements needed in WebGLRenderingContext or WebGLLayer?

> Source/WebKit/chromium/src/WebViewImpl.cpp:2443
> +        // TODO(amarinichev): In MacOS newContext->reshape method needs to be

WebKit uses FIXME without an owner, not TODO.

[Alexey Marinichev]

Created attachment 85418 [details]
Patch

[Kenneth Russell]

Comment on attachment 85418 [details]
Patch

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

> Source/WebKit/chromium/src/WebViewImpl.cpp:1087
> +    if (m_recreatingGraphicsContext) {
> +        reallocateRenderer();
> +        m_recreatingGraphicsContext = false;
> +        return;

It looks like we are going to lose the first composite operation right after recovery of the graphics context. Is this the case? Do we need another call to setRootLayerNeedsDisplay() or similar before returning here? And is it possible that doing so will avoid the need for the reshape hack in reallocateRenderer()?

[Alexey Marinichev]

reallocateRenderer invalidates the root layer which will trigger repaint.  I don't think we're losing anything here.  Perhaps I should add a comment about that.

[Kenneth Russell]

(In reply to comment #5)
> reallocateRenderer invalidates the root layer which will trigger repaint.  I don't think we're losing anything here.  Perhaps I should add a comment about that.

I think that would be helpful if you wouldn't mind uploading another patch for it.

[Kenneth Russell]

(The patch looks good otherwise and I'm happy to r+ it.)

[Alexey Marinichev]

Created attachment 85552 [details]
Patch

[Kenneth Russell]

Comment on attachment 85552 [details]
Patch

Looks good to me.

[WebKit Commit Bot]

Comment on attachment 85552 [details]
Patch

Rejecting attachment 85552 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2

Last 500 characters of output:
src/WebViewImpl.cpp
Hunk #1 succeeded at 1082 (offset 1 line).
Hunk #2 succeeded at 1099 (offset 1 line).
Hunk #3 succeeded at 2414 (offset 14 lines).
Hunk #4 FAILED at 2448.
1 out of 4 hunks FAILED -- saving rejects to file Source/WebKit/chromium/src/WebViewImpl.cpp.rej
patching file Source/WebKit/chromium/src/WebViewImpl.h
Hunk #1 succeeded at 532 (offset 1 line).

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Russell', u'--..." exit_code: 1

Full output: http://queues.webkit.org/results/8141474

[Alexey Marinichev]

Created attachment 85557 [details]
Rebased to top of tree.

[Kenneth Russell]

Comment on attachment 85557 [details]
Rebased to top of tree.

Let's try again.

[WebKit Commit Bot]

Comment on attachment 85557 [details]
Rebased to top of tree.

Rejecting attachment 85557 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'build-..." exit_code: 2

Last 500 characters of output:
......................................
tables/mozilla_expected_failures/other ..
transforms ....
transforms/2d ............
transforms/3d/general .....
transforms/3d/hit-testing ....
transforms/3d/point-mapping ........
transitions .................
transitions/interrupted-accelerated-transition.html -> failed

Exiting early after 1 failures. 21675 tests run.
516.00s total testing time

21674 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
13 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/8141544

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 85557 [details]:

transitions/interrupted-accelerated-transition.html bug 56242 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[Kenneth Russell]

Comment on attachment 85557 [details]
Rebased to top of tree.

I think that failure is unrelated to this patch. cq+'ing again.

[WebKit Commit Bot]

Comment on attachment 85557 [details]
Rebased to top of tree.

Clearing flags on attachment: 85557

Committed r81113: <http://trac.webkit.org/changeset/81113>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.