Bug 57400 – Fullscreen code assumes all layers use GPU compositing when USE(ACCELERATED_COMPOSITING)

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[David Dorwin]

Document::webkitWillEnterFullScreenForElement, Document::webkitDidEnterFullScreenForElement, and Document::webkitWillExitFullScreenForElement all assert that m_fullScreenRenderer->layer()->backing() is valid inside the #if USE(ACCELERATED_COMPOSITING) blocks. However, accelerated compositing may be compiled in but disabled at runtime or for some elements. This is the case for Chromium, which supports a command line switch and does not currently accelerate all types of elements. In addition, the layout tests do not run with GPU acceleration enabled by default.

These asserts and subsequent use of a NULL backing cause the fullscreen tests to crash in the non-GPU-enabled runs of DumpRenderTree and when Chrome is run without GPU acceleration enabled.

I have a fix.

[David Dorwin]

Created attachment 87432 [details]
Check isComposited() before using the backing and remove the asserts.

[Jer Noble]

Looks good to me.

[David Levin]

Comment on attachment 87432 [details]
Check isComposited() before using the backing and remove the asserts.

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

> Source/WebCore/ChangeLog:9
> +        Accelerated compositing may be compiled in but disabled at runtime or for so

This sentence looks incomplete.

> Source/WebCore/dom/Document.cpp:4924
> +          page()->chrome()->client()->setRootFullScreenLayer(m_fullScreenRenderer->layer()->backing()->graphicsLayer());

Your indent is incorrect. (4 space indent in WK unlike chromium with a 2 space indent).

> Source/WebCore/dom/Document.cpp:4949
> +          page()->chrome()->client()->setRootFullScreenLayer(m_fullScreenRenderer->layer()->backing()->graphicsLayer());

Ditto.

[David Dorwin]

Created attachment 88134 [details]
Fixed indent and ChangeLog.

[WebKit Commit Bot]

Comment on attachment 88134 [details]
Fixed indent and ChangeLog.

Rejecting attachment 88134 [details] from commit-queue.

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

Last 500 characters of output:
mmit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Parsed 2 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/dom/Document.cpp
Hunk #1 succeeded at 4849 (offset -71 lines).
Hunk #2 FAILED at 4861.
Hunk #3 FAILED at 4874.
2 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/dom/Document.cpp.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

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

[David Dorwin]

Created attachment 88254 [details]
Merged with head

[David Dorwin]

Created attachment 88257 [details]
Merged with head

[WebKit Review Bot]

Comment on attachment 88257 [details]
Merged with head

Rejecting attachment 88257 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Commit Bot]

Comment on attachment 88257 [details]
Merged with head

Clearing flags on attachment: 88257

Committed r82956: <http://trac.webkit.org/changeset/82956>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.