90031 2012-06-26 17:56:16 -0700 [chromium] Should schedule a commit when dropping contents textures 2012-06-28 12:28:07 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jamesr jamesr cc-bugs danakj enne webkit.review.bot oldest_to_newest 658174 0 jamesr 2012-06-26 17:56:16 -0700 [chromium] Should schedule a commit when dropping contents textures 658177 1 149649 jamesr 2012-06-26 17:57:57 -0700 Created attachment 149649 Patch 659562 2 enne 2012-06-28 09:36:01 -0700 Can you explain more why this commit needs to be here and why the proxies behave differently? 659604 3 jamesr 2012-06-28 10:39:35 -0700 Without this commit we have a bit of stale state in CCLayerTreeHostImpl::canDraw(), which is used only by the thread scheduler. In single threaded mode, render_widget forces us to make a frame no matter what our state is so we do a commit regardless. 659606 4 danakj 2012-06-28 10:43:01 -0700 FWIW CCSingleThreadProxy::doComposite does use canDraw as well. 659607 5 jamesr 2012-06-28 10:43:32 -0700 Oh! So it does...interesting 659615 6 jamesr 2012-06-28 10:58:40 -0700 (In reply to comment #4) > FWIW CCSingleThreadProxy::doComposite does use canDraw as well. In CCSingleThreadProxy commitAndComposite() does an unconditional doCommit(), then calls doComposite() which checks canDraw. Actually though I was wrong that this is why the proxies behave differently for this patch. If we did a commit in threaded mode we would reset the canDraw state as well. In single threaded mode, we get a call to commitAndComposite() from render_widget whenever we become visible. In threaded mode, the scheduler won't schedule a commit unless something tells it to. That's what this patch does. 659646 7 enne 2012-06-28 11:27:22 -0700 (In reply to comment #6) > (In reply to comment #4) > > FWIW CCSingleThreadProxy::doComposite does use canDraw as well. > > In CCSingleThreadProxy commitAndComposite() does an unconditional doCommit(), then calls doComposite() which checks canDraw. > > Actually though I was wrong that this is why the proxies behave differently for this patch. If we did a commit in threaded mode we would reset the canDraw state as well. In single threaded mode, we get a call to commitAndComposite() from render_widget whenever we become visible. In threaded mode, the scheduler won't schedule a commit unless something tells it to. That's what this patch does. Should we do something parallel here, like calling setNeedsCommit() when we become visible? Maybe I just want more symmetry, and it seems a little weird to have this extra commit to notify the main thread that the contents textures earlier so we can clear a flag. 659650 8 jamesr 2012-06-28 11:29:16 -0700 It's not quite just that, we actually do need to do a commit and paint, upload, etc if we've dropped contents textures. I don't think we want to do setNeedsCommit() unconditionally when becoming visible - if we become visible but we haven't evicted anything (say you're switching back and forth rapidly between two tabs), then we should just put a frame up. There's no need for a commit in that case, unless something else has changed. 659678 9 enne 2012-06-28 12:12:26 -0700 (In reply to comment #8) > It's not quite just that, we actually do need to do a commit and paint, upload, etc if we've dropped contents textures. > > I don't think we want to do setNeedsCommit() unconditionally when becoming visible - if we become visible but we haven't evicted anything (say you're switching back and forth rapidly between two tabs), then we should just put a frame up. There's no need for a commit in that case, unless something else has changed. Ah, ok. The piece I was missing was that we also don't commit when not visible, so this commit gets saved for when we need it and come visible again. Sounds good. 659679 10 149649 enne 2012-06-28 12:12:39 -0700 Comment on attachment 149649 Patch R=me. 659684 11 149649 jamesr 2012-06-28 12:20:27 -0700 Comment on attachment 149649 Patch Thanks! 659693 12 149649 webkit.review.bot 2012-06-28 12:28:02 -0700 Comment on attachment 149649 Patch Clearing flags on attachment: 149649 Committed r121450: <http://trac.webkit.org/changeset/121450> 659694 13 webkit.review.bot 2012-06-28 12:28:07 -0700 All reviewed patches have been landed. Closing bug. 149649 2012-06-26 17:57:57 -0700 2012-06-28 12:28:02 -0700 Patch bug-90031-20120626175757.patch text/plain 5246 jamesr U3VidmVyc2lvbiBSZXZpc2lvbjogMTIxMjg3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTFjMGZiOTk4NDBiMmQ0 YWNlNGNmNmI3ZmU4YjEzN2NmMWU1OWRmNy4uODdlZjU4ZTExYmY3ZmFlMDU4ZWMwNzRhNTgyNTBi YzZhYTRmOWIxNyAxMDA3NTUKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA2LTI2ICBKYW1l cyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW2Nocm9taXVtXSBT aG91bGQgc2NoZWR1bGUgYSBjb21taXQgd2hlbiBkcm9wcGluZyBjb250ZW50cyB0ZXh0dXJlcwor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTAwMzEKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJZiB3ZSdyZSBk cm9wcGluZyBjb250ZW50cyB0ZXh0dXJlcyBvbiB0aGUgaW1wbCB0aHJlYWQsIHdlIG5lZWQgdG8g c2NoZWR1bGUgYSBjb21taXQgdG8gcGljayB1cCBuZXcgY29udGVudHMgYXQKKyAgICAgICAgdGhl IG5leHQgY29tbWl0IG9wcG9ydHVuaXR5LiBBbHNvIGFkZHMgc29tZSB0cmFjZXMgdG8gbWFrZSBk ZWJ1Z2dpbmcgaXNzdWVzIGxpa2UgdGhpcyBlYXNpZXIuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy9jaHJvbWl1bS9jYy9DQ0xheWVyVHJlZUhvc3RJbXBsLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OkNDTGF5ZXJUcmVlSG9zdEltcGw6OmNvbW1pdENvbXBsZXRlKToKKyAgICAgICAgKFdl YkNvcmU6OkNDTGF5ZXJUcmVlSG9zdEltcGw6OmNhbkRyYXcpOgorICAgICAgICAoV2ViQ29yZTo6 Q0NMYXllclRyZWVIb3N0SW1wbDo6cmVsZWFzZUNvbnRlbnRzVGV4dHVyZXMpOgorICAgICAgICAq IHBsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDU2NoZWR1bGVyLmNwcDoKKyAgICAgICAg KFdlYkNvcmU6OkNDU2NoZWR1bGVyOjpwcm9jZXNzU2NoZWR1bGVkQWN0aW9ucyk6CisKIDIwMTIt MDYtMjYgIERhdmUgVGhhcnAgIDxkdGhhcnBAY29kZWF1cm9yYS5vcmc+CiAKICAgICAgICAgaWV0 ZXN0Y2VudGVyL2NzczMvdmFsdWVzYW5kdW5pdHMvdW5pdHMtMDAwLmh0bSBhc3NlcnRzCmRpZmYg LS1naXQgYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv Y2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4IDY5OGFiYmVhNDRlYzRmNGNmNmYyZmU3ODdiZDNiYzRm MTA3ZjEwYmYuLmJjYjhjNDQ4Y2I5ODA4NzM3MzBkOGNmMGE5ZjBkZjAxOTE1ZTUyNTggMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJL aXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTItMDYtMjYgIEphbWVz IFJvYmluc29uICA8amFtZXNyQGNocm9taXVtLm9yZz4KKworICAgICAgICBbY2hyb21pdW1dIFNo b3VsZCBzY2hlZHVsZSBhIGNvbW1pdCB3aGVuIGRyb3BwaW5nIGNvbnRlbnRzIHRleHR1cmVzCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05MDAzMQorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFkZHMgYSBzb21l d2hhdCB2YWN1b3VzIHRlc3QgdW5pdCB0ZXN0IGZvciBjb21taXR0aW5nIHdoZW4gcmVsZWFzaW5n IHRleHR1cmVzLgorCisgICAgICAgICogdGVzdHMvQ0NMYXllclRyZWVIb3N0SW1wbFRlc3QuY3Bw OgorCiAyMDEyLTA2LTI1ICBKYW1lcyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5vcmc+CiAK ICAgICAgICAgW2Nocm9taXVtXSBBZGQgV2ViTGF5ZXIgQVBJIGZvciBzY3JvbGxpbmcKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDTGF5 ZXJUcmVlSG9zdEltcGwuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hy b21pdW0vY2MvQ0NMYXllclRyZWVIb3N0SW1wbC5jcHAKaW5kZXggMzAxMWI1ZWNjNjYzMDZjY2Q2 MGQ5OTIxNDFkMWY4NGZlZjdlZTlkZC4uMWUzMmVjNzMzODc0N2Q0ODg5N2RiM2IyZTcyOTY4YWIw YzNmNjYwOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hy b21pdW0vY2MvQ0NMYXllclRyZWVIb3N0SW1wbC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vY2MvQ0NMYXllclRyZWVIb3N0SW1wbC5jcHAKQEAgLTE1 NCw2ICsxNTQsNyBAQCB2b2lkIENDTGF5ZXJUcmVlSG9zdEltcGw6OmJlZ2luQ29tbWl0KCkKIAog dm9pZCBDQ0xheWVyVHJlZUhvc3RJbXBsOjpjb21taXRDb21wbGV0ZSgpCiB7CisgICAgVFJBQ0Vf RVZFTlQwKCJjYyIsICJDQ0xheWVyVHJlZUhvc3RJbXBsOjpjb21taXRDb21wbGV0ZSIpOwogICAg IC8vIFJlY29tcHV0ZSBtYXggc2Nyb2xsIHBvc2l0aW9uOyBtdXN0IGJlIGFmdGVyIGxheWVyIGNv bnRlbnQgYm91bmRzIGFyZQogICAgIC8vIHVwZGF0ZWQuCiAgICAgdXBkYXRlTWF4U2Nyb2xsUG9z aXRpb24oKTsKQEAgLTE2MiwxNCArMTYzLDIyIEBAIHZvaWQgQ0NMYXllclRyZWVIb3N0SW1wbDo6 Y29tbWl0Q29tcGxldGUoKQogCiBib29sIENDTGF5ZXJUcmVlSG9zdEltcGw6OmNhbkRyYXcoKQog ewotICAgIGlmICghbV9yb290TGF5ZXJJbXBsKQorICAgIGlmICghbV9yb290TGF5ZXJJbXBsKSB7 CisgICAgICAgIFRSQUNFX0VWRU5UX0lOU1RBTlQwKCJjYyIsICJDQ0xheWVyVHJlZUhvc3RJbXBs OjpjYW5EcmF3IG5vIHJvb3QgbGF5ZXIiKTsKICAgICAgICAgcmV0dXJuIGZhbHNlOwotICAgIGlm ICh2aWV3cG9ydFNpemUoKS5pc0VtcHR5KCkpCisgICAgfQorICAgIGlmICh2aWV3cG9ydFNpemUo KS5pc0VtcHR5KCkpIHsKKyAgICAgICAgVFJBQ0VfRVZFTlRfSU5TVEFOVDAoImNjIiwgIkNDTGF5 ZXJUcmVlSG9zdEltcGw6OmNhbkRyYXcgZW1wdHkgdmlld3BvcnQiKTsKICAgICAgICAgcmV0dXJu IGZhbHNlOwotICAgIGlmICghbV9sYXllclJlbmRlcmVyKQorICAgIH0KKyAgICBpZiAoIW1fbGF5 ZXJSZW5kZXJlcikgeworICAgICAgICBUUkFDRV9FVkVOVF9JTlNUQU5UMCgiY2MiLCAiQ0NMYXll clRyZWVIb3N0SW1wbDo6Y2FuRHJhdyBubyBsYXllclJlbmRlcmVyIik7CiAgICAgICAgIHJldHVy biBmYWxzZTsKLSAgICBpZiAobV9jb250ZW50c1RleHR1cmVzV2VyZVB1cmdlZFNpbmNlTGFzdENv bW1pdCkKKyAgICB9CisgICAgaWYgKG1fY29udGVudHNUZXh0dXJlc1dlcmVQdXJnZWRTaW5jZUxh c3RDb21taXQpIHsKKyAgICAgICAgVFJBQ0VfRVZFTlRfSU5TVEFOVDAoImNjIiwgIkNDTGF5ZXJU cmVlSG9zdEltcGw6OmNhbkRyYXcgY29udGVudHMgdGV4dHVyZXMgcHVyZ2VkIik7CiAgICAgICAg IHJldHVybiBmYWxzZTsKKyAgICB9CiAgICAgcmV0dXJuIHRydWU7CiB9CiAKQEAgLTQ3NCw2ICs0 ODMsNyBAQCB2b2lkIENDTGF5ZXJUcmVlSG9zdEltcGw6OnJlbGVhc2VDb250ZW50c1RleHR1cmVz KCkKIHsKICAgICBjb250ZW50c1RleHR1cmVBbGxvY2F0b3IoKS0+ZGVsZXRlQWxsVGV4dHVyZXMo KTsKICAgICBtX2NvbnRlbnRzVGV4dHVyZXNXZXJlUHVyZ2VkU2luY2VMYXN0Q29tbWl0ID0gdHJ1 ZTsKKyAgICBtX2NsaWVudC0+c2V0TmVlZHNDb21taXRPbkltcGxUaHJlYWQoKTsKIH0KIAogdm9p ZCBDQ0xheWVyVHJlZUhvc3RJbXBsOjpzZXRNZW1vcnlBbGxvY2F0aW9uTGltaXRCeXRlcyhzaXpl X3QgYnl0ZXMpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9j aHJvbWl1bS9jYy9DQ1NjaGVkdWxlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9jaHJvbWl1bS9jYy9DQ1NjaGVkdWxlci5jcHAKaW5kZXggZDgzNTViZDM2ZWExZDEwZGYy Mjg1YzRjMmFiODJjY2ZlOGVjMzY4MC4uNmY4NjVjOWNkNjA4YjEzY2I4MTM3NzkzNzgwMDllZjY3 MmUwMmZjNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hy b21pdW0vY2MvQ0NTY2hlZHVsZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL2Nocm9taXVtL2NjL0NDU2NoZWR1bGVyLmNwcApAQCAtMTU5LDYgKzE1OSw3IEBAIHZv aWQgQ0NTY2hlZHVsZXI6OnByb2Nlc3NTY2hlZHVsZWRBY3Rpb25zKCkKICAgICBkbyB7CiAgICAg ICAgIGFjdGlvbiA9IG5leHRBY3Rpb24oKTsKICAgICAgICAgbV9zdGF0ZU1hY2hpbmUudXBkYXRl U3RhdGUoYWN0aW9uKTsKKyAgICAgICAgVFJBQ0VfRVZFTlQxKCJjYyIsICJDQ1NjaGVkdWxlcjo6 cHJvY2Vzc1NjaGVkdWxlZEFjdGlvbnMoKSIsICJhY3Rpb24iLCBhY3Rpb24pOwogCiAgICAgICAg IHN3aXRjaCAoYWN0aW9uKSB7CiAgICAgICAgIGNhc2UgQ0NTY2hlZHVsZXJTdGF0ZU1hY2hpbmU6 OkFDVElPTl9OT05FOgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9jaHJvbWl1bS90ZXN0cy9D Q0xheWVyVHJlZUhvc3RJbXBsVGVzdC5jcHAgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3Rlc3Rz L0NDTGF5ZXJUcmVlSG9zdEltcGxUZXN0LmNwcAppbmRleCAzNDg0NmI3YzIxOGU2YTQ1OGVjYTU3 ZjUxODQ5YjVhZWNmZTkyNjRkLi5iMGI0ZmQ0ZTk4M2MxZmU4ZTM4MDQzODU2MmYwN2YwNDQ4NzYz YTlhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3Rlc3RzL0NDTGF5ZXJUcmVl SG9zdEltcGxUZXN0LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3Rlc3RzL0NDTGF5 ZXJUcmVlSG9zdEltcGxUZXN0LmNwcApAQCAtMjg1NSw2ICsyODU1LDEyIEBAIFRFU1RfRihDQ0xh eWVyVHJlZUhvc3RJbXBsVGVzdCwgc3VyZmFjZVRleHR1cmVDYWNoaW5nTWVtb3J5TGltaXQpCiAg ICAgfQogfQogCitURVNUX0YoQ0NMYXllclRyZWVIb3N0SW1wbFRlc3QsIHJlbGVhc2VDb250ZW50 c1RleHR1cmVTaG91bGRUcmlnZ2VyQ29tbWl0KQoreworICAgIG1faG9zdEltcGwtPnJlbGVhc2VD b250ZW50c1RleHR1cmVzKCk7CisgICAgRVhQRUNUX1RSVUUobV9kaWRSZXF1ZXN0Q29tbWl0KTsK K30KKwogc3RydWN0IFJlbmRlclBhc3NDYWNoZUVudHJ5IHsKICAgICBtdXRhYmxlIE93blB0cjxD Q1JlbmRlclBhc3M+IHJlbmRlclBhc3NQdHI7CiAgICAgQ0NSZW5kZXJQYXNzKiByZW5kZXJQYXNz Owo=