100736 2012-10-30 01:59:26 -0700 [EFL][WK2][AC] Avoid storing dirty rects in a Vector inside EwkViewImpl 2012-10-31 11:02:26 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cdumez cdumez gyuyoung.kim kenneth laszlo.gombos lucas.de.marchi ostap73 rakuco webkit.review.bot yael oldest_to_newest 754080 0 cdumez 2012-10-30 01:59:26 -0700 http://trac.webkit.org/changeset/132483 is storing dirty rectangles in a Vector data member inside EwkView and then creates a Region from them in EwkViewImpl::displayTimerFired(). It would therefore be more optimal to keep the dirty rects as a Region instead of Vector to avoid iterating over the Vector to construct a Region from it. 754098 1 171402 cdumez 2012-10-30 02:08:54 -0700 Created attachment 171402 Patch 754101 2 cdumez 2012-10-30 02:10:50 -0700 I have tried running in MiniBrowser: http://trac.webkit.org/export/132877/trunk/Websites/webkit.org/blog-files/3d-transforms/poster-circle.html I could not reproduce infinite loop problem and the animation seems to work properly. I enabled AC using: --- a/Tools/Scripts/webkitperl/FeatureList.pm +++ b/Tools/Scripts/webkitperl/FeatureList.pm @@ -392,7 +392,7 @@ my @features = ( define => "ENABLE_TEXT_AUTOSIZING", default => 0, value => \$textAutosizingSupport }, { option => "tiled-backing-store", desc => "Toggle Tiled Backing Store support", - define => "WTF_USE_TILED_BACKING_STORE", default => isQt(), value => \$tiledBackingStoreSupport }, + define => "WTF_USE_TILED_BACKING_STORE", default => (isQt() || isEfl()), value => \$tiledBackingStoreSupport }, { option => "touch-events", desc => "Toggle Touch Events support", define => "ENABLE_TOUCH_EVENTS", default => (isQt() || isBlackBerry() || isEfl()), value => \$touchEventsSupport }, 754105 3 171402 kenneth 2012-10-30 02:14:04 -0700 Comment on attachment 171402 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171402&action=review > Source/WebKit2/UIProcess/API/efl/EwkViewImpl.h:275 > + OwnPtr<WebCore::Region> m_dirtyRegion; Ah I didn't even know we had a region, but of course we do. 754649 4 cdumez 2012-10-30 14:10:01 -0700 Yael, would you please take a look before we commit this? 755397 5 yael 2012-10-31 09:49:40 -0700 (In reply to comment #4) > Yael, would you please take a look before we commit this? LGTM, I am not sure why you did not observe a crash. I got the crash I posted in https://bugs.webkit.org/show_bug.cgi?id=100288#c0 very consistently. 755402 6 cdumez 2012-10-31 09:52:08 -0700 (In reply to comment #5) > (In reply to comment #4) > > Yael, would you please take a look before we commit this? > > LGTM, > > I am not sure why you did not observe a crash. I got the crash I posted in https://bugs.webkit.org/show_bug.cgi?id=100288#c0 very consistently. To be clear, I did not experience the crash with my patch (and yours) applied so it is not surprising that I couldn't reproduce the patch. I merely wanted to make sure that my patch did not cause a regression (i.e. reintroduce the crash you had fixed). Could someone please cq+ then? 755462 7 171402 webkit.review.bot 2012-10-31 11:02:20 -0700 Comment on attachment 171402 Patch Clearing flags on attachment: 171402 Committed r133046: <http://trac.webkit.org/changeset/133046> 755463 8 webkit.review.bot 2012-10-31 11:02:26 -0700 All reviewed patches have been landed. Closing bug. 171402 2012-10-30 02:08:54 -0700 2012-10-31 11:02:20 -0700 Patch 100736_ac_region.patch text/plain 3904 cdumez ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAyZjQ3OTk0Li4zZGZjN2UzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIg QEAKKzIwMTItMTAtMzAgIENocmlzdG9waGUgRHVtZXogIDxjaHJpc3RvcGhlLmR1bWV6QGludGVs LmNvbT4KKworICAgICAgICBbRUZMXVtXSzJdW0FDXSBBdm9pZCBzdG9yaW5nIGRpcnR5IHJlY3Rz IGluIGEgVmVjdG9yIGluc2lkZSBFd2tWaWV3SW1wbAorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAwNzM2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbG9uZ2VyIHN0b3JlIGRpcnR5IHJlY3RhbmdsZXMg aW4gYSB0ZW1wb3JhcnkgVmVjdG9yIGFuZCBjb25zdHJ1Y3QKKyAgICAgICAgYSBXZWJDb3JlOjpS ZWdpb24gZGlyZWN0bHkgZnJvbSB0aGVtIGluc3RlYWQuIFRoaXMgYXZvaWQgaGF2aW5nIHRvCisg ICAgICAgIGl0ZXJhdGUgb3ZlciB0aGUgVmVjdG9yIGluIEV3a1ZpZXdJbXBsOjpkaXNwbGF5VGlt ZXJGaXJlZCgpIHRvIGNvbnN0cnVjdAorICAgICAgICBhIFJlZ2lvbiBvYmplY3QsIHdoaWNoIHNo b3VsZCBiZSBtb3JlIGVmZmljaWVudC4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZWZsL0V3 a1ZpZXdJbXBsLmNwcDoKKyAgICAgICAgKEV3a1ZpZXdJbXBsOjpkaXNwbGF5VGltZXJGaXJlZCk6 CisgICAgICAgIChFd2tWaWV3SW1wbDo6cmVkcmF3UmVnaW9uKToKKyAgICAgICAgKiBVSVByb2Nl c3MvQVBJL2VmbC9Fd2tWaWV3SW1wbC5oOgorICAgICAgICAoV2ViQ29yZSk6CisgICAgICAgIChF d2tWaWV3SW1wbCk6CisKIDIwMTItMTAtMzAgIEJ5dW5nd29vIExlZSAgPGJ3ODAubGVlQHNhbXN1 bmcuY29tPgogCiAgICAgICAgIFtXSzJdIFJlbW92ZSBpbmNvcnJlY3QgdXNlIG9mIHByZXByb2Nl c3NvciBtYWNybyBpbiBBUEkgaGVhZGVycy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJ UHJvY2Vzcy9BUEkvZWZsL0V3a1ZpZXdJbXBsLmNwcCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9BUEkvZWZsL0V3a1ZpZXdJbXBsLmNwcAppbmRleCBlYmU1MTZkLi41ZGQwMzgzIDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2VmbC9Fd2tWaWV3SW1wbC5jcHAKKysr IGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9lZmwvRXdrVmlld0ltcGwuY3BwCkBAIC0y MzIsNyArMjMyLDcgQEAgdm9pZCBFd2tWaWV3SW1wbDo6c2V0Q3Vyc29yKGNvbnN0IEN1cnNvciYg Y3Vyc29yKQogICAgIGVjb3JlX2V2YXNfb2JqZWN0X2N1cnNvcl9zZXQoZWNvcmVFdmFzLCBtX2N1 cnNvck9iamVjdC5nZXQoKSwgRVZBU19MQVlFUl9NQVgsIGhvdHNwb3RYLCBob3RzcG90WSk7CiB9 CiAKLXZvaWQgRXdrVmlld0ltcGw6OmRpc3BsYXlUaW1lckZpcmVkKFdlYkNvcmU6OlRpbWVyPEV3 a1ZpZXdJbXBsPiopCit2b2lkIEV3a1ZpZXdJbXBsOjpkaXNwbGF5VGltZXJGaXJlZChUaW1lcjxF d2tWaWV3SW1wbD4qKQogewogICAgIEV3a19WaWV3X1NtYXJ0X0RhdGEqIHNkID0gc21hcnREYXRh KCk7CiAKQEAgLTI0MywxNyArMjQzLDE0IEBAIHZvaWQgRXdrVmlld0ltcGw6OmRpc3BsYXlUaW1l ckZpcmVkKFdlYkNvcmU6OlRpbWVyPEV3a1ZpZXdJbXBsPiopCiAgICAgRVdLX1ZJRVdfSU1QTF9H RVRfT1JfUkVUVVJOKHNkLCB2aWV3SW1wbCk7CiAjZW5kaWYKIAotICAgIFJlZ2lvbiBkaXJ0eVJl Z2lvbjsKLSAgICBmb3IgKFZlY3RvcjxJbnRSZWN0Pjo6aXRlcmF0b3IgaXQgPSBtX2RpcnR5UmVj dHMuYmVnaW4oKTsgaXQgIT0gbV9kaXJ0eVJlY3RzLmVuZCgpOyArK2l0KQotICAgICAgICBkaXJ0 eVJlZ2lvbi51bml0ZSgqaXQpOworICAgIEFTU0VSVChtX2RpcnR5UmVnaW9uKTsKKyAgICBWZWN0 b3I8SW50UmVjdD4gcmVjdHMgPSBtX2RpcnR5UmVnaW9uLT5yZWN0cygpOworICAgIC8vIENsZWFy IHJlZ2lvbi4KKyAgICBtX2RpcnR5UmVnaW9uLmNsZWFyKCk7CiAKLSAgICBtX2RpcnR5UmVjdHMu Y2xlYXIoKTsKLQotICAgIFZlY3RvcjxJbnRSZWN0PiByZWN0cyA9IGRpcnR5UmVnaW9uLnJlY3Rz KCk7Ci0gICAgVmVjdG9yPEludFJlY3Q+OjppdGVyYXRvciBlbmQgPSByZWN0cy5lbmQoKTsKLQot ICAgIGZvciAoVmVjdG9yPEludFJlY3Q+OjppdGVyYXRvciBpdCA9IHJlY3RzLmJlZ2luKCk7IGl0 ICE9IGVuZDsgKytpdCkgewotICAgICAgICBJbnRSZWN0IHJlY3QgPSAqaXQ7CisgICAgVmVjdG9y PEludFJlY3Q+Ojpjb25zdF9pdGVyYXRvciBlbmQgPSByZWN0cy5lbmQoKTsKKyAgICBmb3IgKFZl Y3RvcjxJbnRSZWN0Pjo6Y29uc3RfaXRlcmF0b3IgaXQgPSByZWN0cy5iZWdpbigpOyBpdCAhPSBl bmQ7ICsraXQpIHsKKyAgICAgICAgY29uc3QgSW50UmVjdCYgcmVjdCA9ICppdDsKICNpZiBVU0Uo Q09PUkRJTkFURURfR1JBUEhJQ1MpCiAgICAgICAgIGV2YXNfZ2xfbWFrZV9jdXJyZW50KHZpZXdJ bXBsLT5tX2V2YXNHbCwgdmlld0ltcGwtPm1fZXZhc0dsU3VyZmFjZSwgdmlld0ltcGwtPm1fZXZh c0dsQ29udGV4dCk7CiAgICAgICAgIHZpZXdJbXBsLT5tX3BhZ2VWaWV3cG9ydENvbnRyb2xsZXJD bGllbnQtPmRpc3BsYXkocmVjdCwgSW50UG9pbnQoc2QtPnZpZXcueCwgc2QtPnZpZXcueSkpOwpA QCAtMjY1LDkgKzI2MiwxNCBAQCB2b2lkIEV3a1ZpZXdJbXBsOjpkaXNwbGF5VGltZXJGaXJlZChX ZWJDb3JlOjpUaW1lcjxFd2tWaWV3SW1wbD4qKQogCiB2b2lkIEV3a1ZpZXdJbXBsOjpyZWRyYXdS ZWdpb24oY29uc3QgSW50UmVjdCYgcmVjdCkKIHsKKyAgICBpZiAobV9kaXJ0eVJlZ2lvbikKKyAg ICAgICAgbV9kaXJ0eVJlZ2lvbi0+dW5pdGUocmVjdCk7CisgICAgZWxzZQorICAgICAgICBtX2Rp cnR5UmVnaW9uID0gYWRvcHRQdHIobmV3IFJlZ2lvbihyZWN0KSk7CisKKyAgICAvLyBVcGRhdGUg ZGlzcGxheSBpbiB0aGUgZXZlbnQgbG9vcC4KICAgICBpZiAoIW1fZGlzcGxheVRpbWVyLmlzQWN0 aXZlKCkpCiAgICAgICAgIG1fZGlzcGxheVRpbWVyLnN0YXJ0T25lU2hvdCgwKTsKLSAgICBtX2Rp cnR5UmVjdHMuYXBwZW5kKHJlY3QpOwogfQogCiAvKioKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQyL1VJUHJvY2Vzcy9BUEkvZWZsL0V3a1ZpZXdJbXBsLmggYi9Tb3VyY2UvV2ViS2l0Mi9VSVBy b2Nlc3MvQVBJL2VmbC9Fd2tWaWV3SW1wbC5oCmluZGV4IDIyMWNkZTIuLjMzN2ZkYzMgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZWZsL0V3a1ZpZXdJbXBsLmgKKysr IGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9lZmwvRXdrVmlld0ltcGwuaApAQCAtODAs NiArODAsNyBAQCBjbGFzcyBDb2xvcjsKIGNsYXNzIEN1cnNvcjsKIGNsYXNzIEludFJlY3Q7CiBj bGFzcyBJbnRTaXplOworY2xhc3MgUmVnaW9uOwogfQogCiBjbGFzcyBFd2tfQmFja19Gb3J3YXJk X0xpc3Q7CkBAIC0yNzEsNyArMjcyLDcgQEAgcHJpdmF0ZToKICAgICBib29sIG1fdG91Y2hFdmVu dHNFbmFibGVkOwogI2VuZGlmCiAgICAgV2ViQ29yZTo6VGltZXI8RXdrVmlld0ltcGw+IG1fZGlz cGxheVRpbWVyOwotICAgIFdURjo6VmVjdG9yIDxXZWJDb3JlOjpJbnRSZWN0PiBtX2RpcnR5UmVj dHM7CisgICAgT3duUHRyPFdlYkNvcmU6OlJlZ2lvbj4gbV9kaXJ0eVJlZ2lvbjsKICAgICBPd25Q dHI8RXdrX1BvcHVwX01lbnU+IG1fcG9wdXBNZW51OwogICAgIE93blB0cjxXZWJLaXQ6OklucHV0 TWV0aG9kQ29udGV4dEVmbD4gbV9pbnB1dE1ldGhvZENvbnRleHQ7CiAgICAgT3duUHRyPEV3a19D b2xvcl9QaWNrZXI+IG1fY29sb3JQaWNrZXI7Cg==