35142 2010-02-19 02:56:01 -0800 [GTK] Some region checks in scroll are not required 2010-05-13 02:21:53 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED INVALID P2 Normal --- 1 alex alex commit-queue eric xan.lopez oldest_to_newest 191847 0 alex 2010-02-19 02:56:01 -0800 In the scroll method of ChromeClientGtk we are checking if the new region after scrolling the delta has parts that we already have in the window, in order to move them and invalidate the ones that are not already rendered. The gdk_window_move_region already does all that work so we are duplicating the effort. 191849 1 49062 alex 2010-02-19 03:00:25 -0800 Created attachment 49062 Proposed patch This patch removes the intersect check and the invalidate, leaving all the responsability to the gdk_window_move_region method. 191950 2 49062 xan.lopez 2010-02-19 09:36:53 -0800 Comment on attachment 49062 Proposed patch Makes sense to me. I asked Alex to verify this works well in pre-CSW GTK+, and he said it does. (BTW, typo in the ChangeLog, s/alredy/already/) 192100 3 eric 2010-02-19 16:23:19 -0800 Attachment 49062 was posted by a committer and has review+, assigning to Alejandro G. Castro for commit. 194173 4 alex 2010-02-26 11:01:04 -0800 This was committed in r55295, closing. 194209 5 alex 2010-02-26 12:21:58 -0800 Patch reverted (r55298), it was causing problems with the frames I have to review the patch. 197498 6 alex 2010-03-09 01:15:25 -0800 The refactoring has landed in the bug 34214, and we also have the tiled backing store 35146. Probably the best option is to implement the gtk+ support for the tiled backing store, I'm going to check it. 218309 7 54545 alex 2010-04-28 03:43:33 -0700 Created attachment 54545 Proposed patch This patch avoids the invalidate and the operations related to it when we are using the region move function, it works with the frames. 224521 8 54545 xan.lopez 2010-05-12 04:43:22 -0700 Comment on attachment 54545 Proposed patch After talking with Alex we are going to add a comment explaining why we cannot rely only on gdk_moving_region, since its semantics do not quite match what we need in some cases (eg, when there are frames in the page). 225149 9 54545 commit-queue 2010-05-12 23:15:19 -0700 Comment on attachment 54545 Proposed patch Rejecting patch 54545 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['../WebKit/WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 54545, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1 Last 500 characters of output: =54545&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=35142&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 54545 from bug 35142. NOBODY (OOPS!) found in /Users/eseidel/Projects/CommitQueue/WebKit/gtk/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /Users/eseidel/Projects/CommitQueue/WebKit/gtk/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). 225170 10 54545 xan.lopez 2010-05-13 00:05:52 -0700 Comment on attachment 54545 Proposed patch Second try. 225179 11 xan.lopez 2010-05-13 00:18:43 -0700 Any clue about these errors Eric? I seem to get them in all the commits I try to get in through the commit queue. 225180 12 xan.lopez 2010-05-13 00:19:30 -0700 (In reply to comment #11) > Any clue about these errors Eric? I seem to get them in all the commits I try to get in through the commit queue. Bah, nevermind, in this particular case it was a mistake, direct your attention to bug 31018. 225208 13 alex 2010-05-13 02:21:53 -0700 The last patch it is not correct in some situations, think about a scroll that causes a region move of a small part of the bottom to the top, leaving a region in the middle, that central part is not invalidated in the gdk_window_move_region function. Closing the bug as invalid, the initial small refactor makes no sense in this function. 49062 2010-02-19 03:00:25 -0800 2010-02-27 00:10:54 -0800 Proposed patch review_scroll_method.patch text/plain 2251 alex ZGlmZiAtLWdpdCBhL1dlYktpdC9ndGsvQ2hhbmdlTG9nIGIvV2ViS2l0L2d0ay9DaGFuZ2VMb2cK aW5kZXggNDNkMmQyMC4uZGY1OGQxZSAxMDA2NDQKLS0tIGEvV2ViS2l0L2d0ay9DaGFuZ2VMb2cK KysrIGIvV2ViS2l0L2d0ay9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMC0wMi0xOSAg QWxlamFuZHJvIEcuIENhc3RybyAgPGFsZXhAaWdhbGlhLmNvbT4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbR1RLXSBTb21lIHJlZ2lvbiBjaGVja3Mg aW4gc2Nyb2xsIGFyZSBub3QgcmVxdWlyZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTM1MTQyCisKKyAgICAgICAgUmVtb3ZlcyBzb21lIG9mIHRoZSBv cGVyYXRpb25zIGNoZWNraW5nIHRoZSBtb3ZlZCBhbmQgaW52YWxpZGF0ZWQKKyAgICAgICAgcmVn aW9ucyB3aGVuIHNjcm9sbGluZywgaXQgaXMgZG9uZSBhbHJlZHkgaW4KKyAgICAgICAgZ2RrX3dp bmRvd19tb3ZlX3JlZ2lvbi4KKworICAgICAgICAqIFdlYkNvcmVTdXBwb3J0L0Nocm9tZUNsaWVu dEd0ay5jcHA6CisKIDIwMTAtMDItMTggIERpZWdvIEVzY2FsYW50ZSBVcnJlbG8gIDxkZXNjYWxh bnRlQGlnYWxpYS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgR3VzdGF2byBOb3JvbmhhIFNp bHZhLgpkaWZmIC0tZ2l0IGEvV2ViS2l0L2d0ay9XZWJDb3JlU3VwcG9ydC9DaHJvbWVDbGllbnRH dGsuY3BwIGIvV2ViS2l0L2d0ay9XZWJDb3JlU3VwcG9ydC9DaHJvbWVDbGllbnRHdGsuY3BwCmlu ZGV4IDc0ZjVlMDcuLjllODE4ZGYgMTAwNjQ0Ci0tLSBhL1dlYktpdC9ndGsvV2ViQ29yZVN1cHBv cnQvQ2hyb21lQ2xpZW50R3RrLmNwcAorKysgYi9XZWJLaXQvZ3RrL1dlYkNvcmVTdXBwb3J0L0No cm9tZUNsaWVudEd0ay5jcHAKQEAgLTM2NCwyNSArMzY0LDEwIEBAIHZvaWQgQ2hyb21lQ2xpZW50 OjpzY3JvbGwoY29uc3QgSW50U2l6ZSYgZGVsdGEsIGNvbnN0IEludFJlY3QmIHJlY3RUb1Njcm9s bCwgY29uCiAgICAgLy8gV2UgY2Fubm90IHVzZSBnZGtfd2luZG93X3Njcm9sbCBoZXJlIGJlY2F1 c2UgaXQgaXMgb25seSBhYmxlIHRvCiAgICAgLy8gc2Nyb2xsIHRoZSB3aG9sZSB3aW5kb3cgYXQg b25jZSwgYW5kIHdlIG9mdGVuIG5lZWQgdG8gc2Nyb2xsCiAgICAgLy8gcG9ydGlvbnMgb2YgdGhl IHdpbmRvdyBvbmx5ICh0aGluayBmcmFtZXMpLgotICAgIEdka1JlY3RhbmdsZSBhcmVhID0gY2xp cFJlY3Q7Ci0gICAgR2RrUmVjdGFuZ2xlIG1vdmVSZWN0OwotCi0gICAgR2RrUmVjdGFuZ2xlIHNv dXJjZVJlY3QgPSBhcmVhOwotICAgIHNvdXJjZVJlY3QueCAtPSBkZWx0YS53aWR0aCgpOwotICAg IHNvdXJjZVJlY3QueSAtPSBkZWx0YS5oZWlnaHQoKTsKLQotICAgIEdka1JlZ2lvbiogaW52YWxp ZFJlZ2lvbiA9IGdka19yZWdpb25fcmVjdGFuZ2xlKCZhcmVhKTsKLQotICAgIGlmIChnZGtfcmVj dGFuZ2xlX2ludGVyc2VjdCgmYXJlYSwgJnNvdXJjZVJlY3QsICZtb3ZlUmVjdCkpIHsKLSAgICAg ICAgR2RrUmVnaW9uKiBtb3ZlUmVnaW9uID0gZ2RrX3JlZ2lvbl9yZWN0YW5nbGUoJm1vdmVSZWN0 KTsKLSAgICAgICAgZ2RrX3dpbmRvd19tb3ZlX3JlZ2lvbih3aW5kb3csIG1vdmVSZWdpb24sIGRl bHRhLndpZHRoKCksIGRlbHRhLmhlaWdodCgpKTsKLSAgICAgICAgZ2RrX3JlZ2lvbl9vZmZzZXQo bW92ZVJlZ2lvbiwgZGVsdGEud2lkdGgoKSwgZGVsdGEuaGVpZ2h0KCkpOwotICAgICAgICBnZGtf cmVnaW9uX3N1YnRyYWN0KGludmFsaWRSZWdpb24sIG1vdmVSZWdpb24pOwotICAgICAgICBnZGtf cmVnaW9uX2Rlc3Ryb3kobW92ZVJlZ2lvbik7Ci0gICAgfQotCi0gICAgZ2RrX3dpbmRvd19pbnZh bGlkYXRlX3JlZ2lvbih3aW5kb3csIGludmFsaWRSZWdpb24sIEZBTFNFKTsKLSAgICBnZGtfcmVn aW9uX2Rlc3Ryb3koaW52YWxpZFJlZ2lvbik7CisgICAgR2RrUmVjdGFuZ2xlIG1vdmVSZWN0ID0g Y2xpcFJlY3Q7CisgICAgR2RrUmVnaW9uKiBtb3ZlUmVnaW9uID0gZ2RrX3JlZ2lvbl9yZWN0YW5n bGUoJm1vdmVSZWN0KTsKKyAgICBnZGtfd2luZG93X21vdmVfcmVnaW9uKHdpbmRvdywgbW92ZVJl Z2lvbiwgZGVsdGEud2lkdGgoKSwgZGVsdGEuaGVpZ2h0KCkpOworICAgIGdka19yZWdpb25fZGVz dHJveShtb3ZlUmVnaW9uKTsKIH0KIAogLy8gRklYTUU6IHRoaXMgZG9lcyBub3QgdGFrZSBpbnRv IGFjY291bnQgdGhlIFdNIGRlY29yYXRpb25zCg== 54545 2010-04-28 03:43:33 -0700 2010-05-13 00:09:42 -0700 Proposed patch gtk_scrolling.patch text/plain 2601 alex Y29tbWl0IDNkMGMyNWQwN2E5YWQ2OGFlZDA0ODdhOWRiMjliNjY3NDFjZGFhNWIKQXV0aG9yOiBB bGVqYW5kcm8gRy4gQ2FzdHJvIDxhbGV4QGlnYWxpYS5jb20+CkRhdGU6ICAgV2VkIEFwciAyOCAx MjozMDoxMSAyMDEwICswMjAwCgogICAgMjAxMC0wNC0yOCAgQWxlamFuZHJvIEcuIENhc3RybyAg PGFsZXhAaWdhbGlhLmNvbT4KICAgIAogICAgICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KICAgIAogICAgICAgICAgICBbR1RLXSBTb21lIHJlZ2lvbiBjaGVja3MgaW4gc2Nyb2xs IGFyZSBub3QgcmVxdWlyZWQKICAgICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTM1MTQyCiAgICAKICAgICAgICAgICAgQXZvaWRzIHRoZSBpbnZhbGlkYXRp b24gb3BlcmF0aW9uIHdoZW4gbW92aW5nIGEgcmVnaW9uIGluIHRoZQogICAgICAgICAgICBzY3Jv bGwsIGluIHRoYXQgY2FzZSB0aGUgaW52YWxpZGF0aW9uIGlzIGRvbmUgYWxyZWFkeSBpbgogICAg ICAgICAgICBnZGtfd2luZG93X21vdmVfcmVnaW9uLgogICAgCiAgICAgICAgICAgICogV2ViQ29y ZVN1cHBvcnQvQ2hyb21lQ2xpZW50R3RrLmNwcDoKICAgICAgICAgICAgKFdlYktpdDo6Q2hyb21l Q2xpZW50OjpzY3JvbGwpOgoKZGlmZiAtLWdpdCBhL1dlYktpdC9ndGsvQ2hhbmdlTG9nIGIvV2Vi S2l0L2d0ay9DaGFuZ2VMb2cKaW5kZXggZmU5ZTA1Yi4uMjg1MzVjZCAxMDA2NDQKLS0tIGEvV2Vi S2l0L2d0ay9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2d0ay9DaGFuZ2VMb2cKQEAgLTEsMyArMSwx NyBAQAorMjAxMC0wNC0yOCAgQWxlamFuZHJvIEcuIENhc3RybyAgPGFsZXhAaWdhbGlhLmNvbT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbR1RLXSBT b21lIHJlZ2lvbiBjaGVja3MgaW4gc2Nyb2xsIGFyZSBub3QgcmVxdWlyZWQKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM1MTQyCisKKyAgICAgICAgQXZv aWRzIHRoZSBpbnZhbGlkYXRpb24gb3BlcmF0aW9uIHdoZW4gbW92aW5nIGEgcmVnaW9uIGluIHRo ZQorICAgICAgICBzY3JvbGwsIGluIHRoYXQgY2FzZSB0aGUgaW52YWxpZGF0aW9uIGlzIGRvbmUg YWxyZWFkeSBpbgorICAgICAgICBnZGtfd2luZG93X21vdmVfcmVnaW9uLgorCisgICAgICAgICog V2ViQ29yZVN1cHBvcnQvQ2hyb21lQ2xpZW50R3RrLmNwcDoKKyAgICAgICAgKFdlYktpdDo6Q2hy b21lQ2xpZW50OjpzY3JvbGwpOgorCiAyMDEwLTA0LTI1ICBTYW0gV2VpbmlnICA8c2FtQHdlYmtp dC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgTWFjaWVqIFN0YWNob3dpYWsuCmRpZmYgLS1n aXQgYS9XZWJLaXQvZ3RrL1dlYkNvcmVTdXBwb3J0L0Nocm9tZUNsaWVudEd0ay5jcHAgYi9XZWJL aXQvZ3RrL1dlYkNvcmVTdXBwb3J0L0Nocm9tZUNsaWVudEd0ay5jcHAKaW5kZXggOGExZDQ4Yi4u OTI5YzM4YiAxMDA2NDQKLS0tIGEvV2ViS2l0L2d0ay9XZWJDb3JlU3VwcG9ydC9DaHJvbWVDbGll bnRHdGsuY3BwCisrKyBiL1dlYktpdC9ndGsvV2ViQ29yZVN1cHBvcnQvQ2hyb21lQ2xpZW50R3Rr LmNwcApAQCAtMzY3LDE4ICszNjcsMTUgQEAgdm9pZCBDaHJvbWVDbGllbnQ6OnNjcm9sbChjb25z dCBJbnRTaXplJiBkZWx0YSwgY29uc3QgSW50UmVjdCYgcmVjdFRvU2Nyb2xsLCBjb24KICAgICBz b3VyY2VSZWN0LnggLT0gZGVsdGEud2lkdGgoKTsKICAgICBzb3VyY2VSZWN0LnkgLT0gZGVsdGEu aGVpZ2h0KCk7CiAKLSAgICBHZGtSZWdpb24qIGludmFsaWRSZWdpb24gPSBnZGtfcmVnaW9uX3Jl Y3RhbmdsZSgmYXJlYSk7Ci0KICAgICBpZiAoZ2RrX3JlY3RhbmdsZV9pbnRlcnNlY3QoJmFyZWEs ICZzb3VyY2VSZWN0LCAmbW92ZVJlY3QpKSB7CiAgICAgICAgIEdka1JlZ2lvbiogbW92ZVJlZ2lv biA9IGdka19yZWdpb25fcmVjdGFuZ2xlKCZtb3ZlUmVjdCk7CiAgICAgICAgIGdka193aW5kb3df bW92ZV9yZWdpb24od2luZG93LCBtb3ZlUmVnaW9uLCBkZWx0YS53aWR0aCgpLCBkZWx0YS5oZWln aHQoKSk7Ci0gICAgICAgIGdka19yZWdpb25fb2Zmc2V0KG1vdmVSZWdpb24sIGRlbHRhLndpZHRo KCksIGRlbHRhLmhlaWdodCgpKTsKLSAgICAgICAgZ2RrX3JlZ2lvbl9zdWJ0cmFjdChpbnZhbGlk UmVnaW9uLCBtb3ZlUmVnaW9uKTsKICAgICAgICAgZ2RrX3JlZ2lvbl9kZXN0cm95KG1vdmVSZWdp b24pOworICAgIH0gZWxzZSB7CisgICAgICAgIEdka1JlZ2lvbiogaW52YWxpZFJlZ2lvbiA9IGdk a19yZWdpb25fcmVjdGFuZ2xlKCZhcmVhKTsKKyAgICAgICAgZ2RrX3dpbmRvd19pbnZhbGlkYXRl X3JlZ2lvbih3aW5kb3csIGludmFsaWRSZWdpb24sIEZBTFNFKTsKKyAgICAgICAgZ2RrX3JlZ2lv bl9kZXN0cm95KGludmFsaWRSZWdpb24pOwogICAgIH0KLQotICAgIGdka193aW5kb3dfaW52YWxp ZGF0ZV9yZWdpb24od2luZG93LCBpbnZhbGlkUmVnaW9uLCBGQUxTRSk7Ci0gICAgZ2RrX3JlZ2lv bl9kZXN0cm95KGludmFsaWRSZWdpb24pOwogfQogCiAvLyBGSVhNRTogdGhpcyBkb2VzIG5vdCB0 YWtlIGludG8gYWNjb3VudCB0aGUgV00gZGVjb3JhdGlvbnMK