174336 2017-07-10 16:56:09 -0700 RenderImage should not add itself as a RelevantRepaintedObject if its image frame is being decoded 2017-07-11 13:56:31 -0700 1 1 1 Unclassified WebKit Images WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 sabouhallawa sabouhallawa commit-queue kling simon.fraser thorton oldest_to_newest 1327269 0 sabouhallawa 2017-07-10 16:56:09 -0700 Since nothing will be drawn till the image frame finishes decoding we should treat returning ImageDrawResult::DidRequestDecoding from BitmapImage the same as we do when the image is still loading. 1327286 1 315050 sabouhallawa 2017-07-10 17:18:30 -0700 Created attachment 315050 Patch 1327385 2 315050 kling 2017-07-10 22:19:52 -0700 Comment on attachment 315050 Patch Is this about avoiding unnecessary repaints? If so, could we make a repaint test for it? 1327635 3 sabouhallawa 2017-07-11 11:59:30 -0700 (In reply to Andreas Kling from comment #2) > Comment on attachment 315050 [details] > Patch > > Is this about avoiding unnecessary repaints? If so, could we make a repaint > test for it? No. Adding the rectangle of a renderer to the Page::m_topRelevantPaintedRegion or m_bottomRelevantPaintedRegion helps making the decision whether the page reached the milestone 'DidHitRelevantRepaintedObjectsAreaThreshold' or not. Reaching this milestone means that the page has drawn enough relevant contents in the viewport such that we can consider, the page has completed its first draw. This patch is fixing false information which the RenderImage is providing to the page when it says, it has drawn itself when it has just requested asynchronous decoding. I don't know how this patch can be tested without adding new functions to the Internals. The problem also is Page::didFinishLoad() cancels counting the relevant repaint objects by calling resetRelevantPaintedObjectCounter(). 1327638 4 thorton 2017-07-11 12:04:43 -0700 (In reply to Said Abou-Hallawa from comment #3) > (In reply to Andreas Kling from comment #2) > > Comment on attachment 315050 [details] > > Patch > > > > Is this about avoiding unnecessary repaints? If so, could we make a repaint > > test for it? > > No. Adding the rectangle of a renderer to the > Page::m_topRelevantPaintedRegion or m_bottomRelevantPaintedRegion helps > making the decision whether the page reached the milestone > 'DidHitRelevantRepaintedObjectsAreaThreshold' or not. Reaching this > milestone means that the page has drawn enough relevant contents in the > viewport such that we can consider, the page has completed its first draw. > > This patch is fixing false information which the RenderImage is providing to > the page when it says, it has drawn itself when it has just requested > asynchronous decoding. > > I don't know how this patch can be tested without adding new functions to > the Internals. The problem also is Page::didFinishLoad() cancels counting > the relevant repaint objects by calling resetRelevantPaintedObjectCounter(). You can block didFinishLoad by making the main resource load hang (with either an HTTP test or an API test that gets its data from e.g. a custom URL scheme with WKURLSchemeHandler). We should have milestone callbacks in Internals or test runner or somewhere (or if it's an API test, it's easy to get them) 1327724 5 315050 commit-queue 2017-07-11 13:56:29 -0700 Comment on attachment 315050 Patch Clearing flags on attachment: 315050 Committed r219360: <http://trac.webkit.org/changeset/219360> 1327725 6 commit-queue 2017-07-11 13:56:31 -0700 All reviewed patches have been landed. Closing bug. 315050 2017-07-10 17:18:30 -0700 2017-07-11 13:56:29 -0700 Patch bug-174336-20170710171830.patch text/plain 4193 sabouhallawa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxOTMxNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDE3LTA3LTEwICBTYWlkIEFi b3UtSGFsbGF3YSAgPHNhYm91aGFsbGF3YUBhcHBsZS5jb20+CisKKyAgICAgICAgUmVuZGVySW1h Z2Ugc2hvdWxkIG5vdCBhZGQgaXRzZWxmIGFzIGEgUmVsZXZhbnRSZXBhaW50ZWRPYmplY3QgaWYg aXRzIGltYWdlIGZyYW1lIGlzIGJlaW5nIGRlY29kZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3NDMzNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFNpbmNlIG5vdGhpbmcgd2lsbCBiZSBkcmF3biB0aWxs IHRoZSBpbWFnZSBmcmFtZSBmaW5pc2hlcyBkZWNvZGluZyB3ZSBzaG91bGQKKyAgICAgICAgdHJl YXQgcmV0dXJuaW5nIEltYWdlRHJhd1Jlc3VsdDo6RGlkUmVxdWVzdERlY29kaW5nIGZyb20gQml0 bWFwSW1hZ2U6OmRyYXcKKyAgICAgICAgdGhlIHNhbWUgYXMgd2UgZG8gd2hlbiB0aGUgaW1hZ2Ug aXMgc3RpbGwgbG9hZGluZy4KKworICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJJbWFnZS5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJJbWFnZTo6cGFpbnRSZXBsYWNlZCk6CisgICAgICAg IChXZWJDb3JlOjpSZW5kZXJJbWFnZTo6cGFpbnRJbnRvUmVjdCk6CisgICAgICAgICogcmVuZGVy aW5nL1JlbmRlckltYWdlLmg6CisKIDIwMTctMDctMTAgIERldmluIFJvdXNzbyAgPGRyb3Vzc29A YXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IEhpZ2hsaWdodCBtYXRjaGluZyBD U1MgY2FudmFzIGNsaWVudHMgd2hlbiBob3ZlcmluZyBjb250ZXh0cyBpbiB0aGUgUmVzb3VyY2Vz IHRhYgpJbmRleDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckltYWdlLmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVySW1hZ2UuY3BwCShyZXZp c2lvbiAyMTkzMTQpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVySW1hZ2UuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC00NzUsMTMgKzQ3NSwxMyBAQCB2b2lkIFJlbmRlckltYWdlOjpw YWludFJlcGxhY2VkKFBhaW50SW5mCiAgICAgaWYgKGNsaXApCiAgICAgICAgIGNvbnRleHQuY2xp cChjb250ZW50Qm94UmVjdCk7CiAKLSAgICBwYWludEludG9SZWN0KHBhaW50SW5mbywgc25hcFJl Y3RUb0RldmljZVBpeGVscyhyZXBsYWNlZENvbnRlbnRSZWN0LCBkZXZpY2VTY2FsZUZhY3Rvcikp OworICAgIEltYWdlRHJhd1Jlc3VsdCByZXN1bHQgPSBwYWludEludG9SZWN0KHBhaW50SW5mbywg c25hcFJlY3RUb0RldmljZVBpeGVscyhyZXBsYWNlZENvbnRlbnRSZWN0LCBkZXZpY2VTY2FsZUZh Y3RvcikpOwogICAgIAogICAgIGlmIChjYWNoZWRJbWFnZSgpICYmIHBhaW50SW5mby5waGFzZSA9 PSBQYWludFBoYXNlRm9yZWdyb3VuZCkgewogICAgICAgICAvLyBGb3Igbm93LCBjb3VudCBpbWFn ZXMgYXMgdW5wYWludGVkIGlmIHRoZXkgYXJlIHN0aWxsIHByb2dyZXNzaXZlbHkgbG9hZGluZy4g V2UgbWF5IHdhbnQgCiAgICAgICAgIC8vIHRvIHJlZmluZSB0aGlzIGluIHRoZSBmdXR1cmUgdG8g YWNjb3VudCBmb3IgdGhlIHBvcnRpb24gb2YgdGhlIGltYWdlIHRoYXQgaGFzIHBhaW50ZWQuCiAg ICAgICAgIExheW91dFJlY3QgdmlzaWJsZVJlY3QgPSBpbnRlcnNlY3Rpb24ocmVwbGFjZWRDb250 ZW50UmVjdCwgY29udGVudEJveFJlY3QpOwotICAgICAgICBpZiAoY2FjaGVkSW1hZ2UoKS0+aXNM b2FkaW5nKCkpCisgICAgICAgIGlmIChjYWNoZWRJbWFnZSgpLT5pc0xvYWRpbmcoKSB8fCByZXN1 bHQgPT0gSW1hZ2VEcmF3UmVzdWx0OjpEaWRSZXF1ZXN0RGVjb2RpbmcpCiAgICAgICAgICAgICBw YWdlKCkuYWRkUmVsZXZhbnRVbnBhaW50ZWRPYmplY3QodGhpcywgdmlzaWJsZVJlY3QpOwogICAg ICAgICBlbHNlCiAgICAgICAgICAgICBwYWdlKCkuYWRkUmVsZXZhbnRSZXBhaW50ZWRPYmplY3Qo dGhpcywgdmlzaWJsZVJlY3QpOwpAQCAtNTU5LDE0ICs1NTksMTQgQEAgdm9pZCBSZW5kZXJJbWFn ZTo6YXJlYUVsZW1lbnRGb2N1c0NoYW5nZQogICAgIHJlcGFpbnQoKTsKIH0KIAotdm9pZCBSZW5k ZXJJbWFnZTo6cGFpbnRJbnRvUmVjdChQYWludEluZm8mIHBhaW50SW5mbywgY29uc3QgRmxvYXRS ZWN0JiByZWN0KQorSW1hZ2VEcmF3UmVzdWx0IFJlbmRlckltYWdlOjpwYWludEludG9SZWN0KFBh aW50SW5mbyYgcGFpbnRJbmZvLCBjb25zdCBGbG9hdFJlY3QmIHJlY3QpCiB7CiAgICAgaWYgKCFp bWFnZVJlc291cmNlKCkuY2FjaGVkSW1hZ2UoKSB8fCBpbWFnZVJlc291cmNlKCkuZXJyb3JPY2N1 cnJlZCgpIHx8IHJlY3Qud2lkdGgoKSA8PSAwIHx8IHJlY3QuaGVpZ2h0KCkgPD0gMCkKLSAgICAg ICAgcmV0dXJuOworICAgICAgICByZXR1cm4gSW1hZ2VEcmF3UmVzdWx0OjpEaWROb3RoaW5nOwog CiAgICAgUmVmUHRyPEltYWdlPiBpbWcgPSBpbWFnZVJlc291cmNlKCkuaW1hZ2UoZmxvb3JlZElu dFNpemUocmVjdC5zaXplKCkpKTsKICAgICBpZiAoIWltZyB8fCBpbWctPmlzTnVsbCgpKQotICAg ICAgICByZXR1cm47CisgICAgICAgIHJldHVybiBJbWFnZURyYXdSZXN1bHQ6OkRpZE5vdGhpbmc7 CiAKICAgICBIVE1MSW1hZ2VFbGVtZW50KiBpbWFnZUVsZW1lbnQgPSBpczxIVE1MSW1hZ2VFbGVt ZW50PihlbGVtZW50KCkpID8gZG93bmNhc3Q8SFRNTEltYWdlRWxlbWVudD4oZWxlbWVudCgpKSA6 IG51bGxwdHI7CiAgICAgQ29tcG9zaXRlT3BlcmF0b3IgY29tcG9zaXRlT3BlcmF0b3IgPSBpbWFn ZUVsZW1lbnQgPyBpbWFnZUVsZW1lbnQtPmNvbXBvc2l0ZU9wZXJhdG9yKCkgOiBDb21wb3NpdGVT b3VyY2VPdmVyOwpAQCAtNTg5LDYgKzU4OSw3IEBAIHZvaWQgUmVuZGVySW1hZ2U6OnBhaW50SW50 b1JlY3QoUGFpbnRJbmYKICAgICBhdXRvIGRyYXdSZXN1bHQgPSBwYWludEluZm8uY29udGV4dCgp LmRyYXdJbWFnZSgqaW1nLCByZWN0LCBJbWFnZVBhaW50aW5nT3B0aW9ucyhjb21wb3NpdGVPcGVy YXRvciwgQmxlbmRNb2RlTm9ybWFsLCBkZWNvZGluZ01vZGUsIG9yaWVudGF0aW9uRGVzY3JpcHRp b24sIGludGVycG9sYXRpb24pKTsKICAgICBpZiAoZHJhd1Jlc3VsdCA9PSBJbWFnZURyYXdSZXN1 bHQ6OkRpZFJlcXVlc3REZWNvZGluZykKICAgICAgICAgaW1hZ2VSZXNvdXJjZSgpLmNhY2hlZElt YWdlKCktPmFkZFBlbmRpbmdJbWFnZURyYXdpbmdDbGllbnQoKnRoaXMpOworICAgIHJldHVybiBk cmF3UmVzdWx0OwogfQogCiBib29sIFJlbmRlckltYWdlOjpib3hTaGFkb3dTaG91bGRCZUFwcGxp ZWRUb0JhY2tncm91bmQoY29uc3QgTGF5b3V0UG9pbnQmIHBhaW50T2Zmc2V0LCBCYWNrZ3JvdW5k QmxlZWRBdm9pZGFuY2UgYmxlZWRBdm9pZGFuY2UsIElubGluZUZsb3dCb3gqKSBjb25zdApJbmRl eDogU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckltYWdlLmgKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckltYWdlLmgJKHJldmlzaW9uIDIxOTMxNCkK KysrIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJJbWFnZS5oCSh3b3JraW5nIGNvcHkp CkBAIC04OCw3ICs4OCw3IEBAIHByb3RlY3RlZDoKIAogICAgIHZvaWQgaW1hZ2VDaGFuZ2VkKFdy YXBwZWRJbWFnZVB0ciwgY29uc3QgSW50UmVjdCogPSBudWxscHRyKSBvdmVycmlkZTsKIAotICAg IHZvaWQgcGFpbnRJbnRvUmVjdChQYWludEluZm8mLCBjb25zdCBGbG9hdFJlY3QmKTsKKyAgICBJ bWFnZURyYXdSZXN1bHQgcGFpbnRJbnRvUmVjdChQYWludEluZm8mLCBjb25zdCBGbG9hdFJlY3Qm KTsKICAgICB2b2lkIHBhaW50KFBhaW50SW5mbyYsIGNvbnN0IExheW91dFBvaW50JikgZmluYWw7 CiAgICAgdm9pZCBsYXlvdXQoKSBvdmVycmlkZTsKIAo=