Bug 76735 – [chromium] Tiled layers that get skipped once never get to draw again

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Vangelis Kokkevis]

If m_skipsDraw is set on a tiled layer because it's been unable to reserve a texture, the flag will stick with it for as long as the layer is around.  The issue is that TiledLayerChromium::drawsContent() will return false and the layer will never end up in a RS's layer list.  As a result, prepareToUpdate() will never be called on it and m_skipsDraw won't be reset.

This is a big issue for root layers as they stick around even after reloading the page.

[Vangelis Kokkevis]

Created attachment 123372 [details]
Patch

[WebKit Review Bot]

Attachment 123372 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adrienne Walker]

Comment on attachment 123372 [details]
Patch

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

Nice catch! I guess this must have regressed when I changed which layers got added to render surface layer lists.

> Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp:416
> +    // Neet to set the max limit again as it gets ovewritten by updateLayers().

nit: typos.

Also, it's really too bad you have to do this.  Maybe when we separate out the texture manager's responsibilities more it'll be possible to have a test interact with it more reasonably.

[Vangelis Kokkevis]

Created attachment 123401 [details]
Patch

[Vangelis Kokkevis]

(In reply to comment #4)
> Created an attachment (id=123401) [details]
> Patch

Fixed typos (oops!) and order of include files.

[WebKit Review Bot]

Attachment 123401 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
 + baa86bc...bf9dada master     -> origin/master  (forced update)
	M	Source/WebKit/chromium/ChangeLog
	M	Source/WebKit/chromium/DEPS
	M	Source/WebKit/mac/Plugins/WebNetscapePluginView.mm
	M	Source/WebKit/mac/Plugins/Hosted/WebHostedNetscapePluginView.mm
	M	Source/WebKit/mac/Plugins/WebBasePluginPackage.mm
	M	Source/WebKit/mac/Plugins/WebBaseNetscapePluginView.mm
	M	Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm
	M	Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
	M	Source/WebKit/mac/ChangeLog
	M	Source/WebKit/mac/Misc/WebElementDictionary.mm
	M	Source/WebKit/mac/Misc/WebIconDatabase.mm
	M	Source/WebKit/mac/History/WebHistoryItem.mm
	M	Source/WebKit/mac/History/WebBackForwardList.mm
	M	Source/WebKit/mac/Carbon/CarbonWindowAdapter.mm
	M	Source/WebKit/mac/WebView/WebArchive.mm
	M	Source/WebKit/mac/WebView/WebViewData.mm
	M	Source/WebKit/mac/WebView/WebView.mm
	M	Source/WebKit/mac/WebView/WebResource.mm
	M	Source/WebKit/mac/WebView/WebDataSource.mm
	M	Source/WebKit/mac/WebView/WebTextIterator.mm
	M	Source/WebKit/mac/WebView/WebHTMLView.mm
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/page/DOMWindow.cpp
	M	Source/WebCore/platform/RunLoop.cpp
	M	Source/WebCore/platform/mac/RunLoopMac.mm
	M	Source/WebCore/platform/RunLoop.h
r105554 = 507f56cd2ce6237ed37916e4d44446183ee0a24d (refs/remotes/origin/master)
First, rewinding head to replay your work on top of it...
Applying: Make WebCore RunLoop work for WebKit1
Using index info to reconstruct a base tree...
<stdin>:41: trailing whitespace.
        
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Make WebCore RunLoop work for WebKit1

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 170.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Vangelis Kokkevis]

Created attachment 123422 [details]
Resync-ed and resolved conflicts

[James Robinson]

Comment on attachment 123422 [details]
Resync-ed and resolved conflicts

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

> Source/WebCore/ChangeLog:8
> +        with the layer for the remainign of its lifetime.

typo: remainign -> remainder

> Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp:99
> +class FakeCCLayerTreeHostClient : public CCLayerTreeHostClient {

this is very nearly the same thing as FakeCCLayerTreeHostClient in LayerChromiumTest.cpp. Could you unify them and put it in a shared location? Seems like a quite handy thing to have for unit tests.

[Vangelis Kokkevis]

Committed r105845: <http://trac.webkit.org/changeset/105845>