173153 2017-06-09 04:50:03 -0700 [CoordinatedGraphics] Behavior change of DrawingAreaImpl after r217779 2017-06-12 08:27:35 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 daewoong.jang daewoong.jang bugs-noreply cgarcia commit-queue magomez mcatanzaro yoon zan oldest_to_newest 1317356 0 daewoong.jang 2017-06-09 04:50:03 -0700 By changeset 217779(https://bugs.webkit.org/show_bug.cgi?id=172496), DrawingAreaImpl::mainFrameContentSizeChanged() was moved to AcceleratedDrawingArea::mainFrameContentSizeChanged(). However the move has caused behavior change of DrawingAreaImpl, as it revives the existing implementation of AcceleratedDrawingArea which was shadowed by both DrawingAreaImpl and DrawingAreaWPE before the change. It seems that the behavior change is unintended. 1317357 1 312427 daewoong.jang 2017-06-09 04:52:08 -0700 Created attachment 312427 patch 1318120 2 cgarcia 2017-06-11 23:22:14 -0700 Has this broken something? I think it was the previous code what was wrong, we probably didn't notice it because we don't expose fixed layout. 1318121 3 daewoong.jang 2017-06-11 23:26:44 -0700 (In reply to Carlos Garcia Campos from comment #2) > Has this broken something? I think it was the previous code what was wrong, > we probably didn't notice it because we don't expose fixed layout. I'm using fixed layout and CoordinatedGraphics stopped displaying pages since the patch. I'd appreciate if you could investigate further. Thanks for reply! 1318129 4 312427 cgarcia 2017-06-12 01:21:00 -0700 Comment on attachment 312427 patch View in context: https://bugs.webkit.org/attachment.cgi?id=312427&action=review > Source/WebKit2/WebProcess/WebPage/AcceleratedDrawingArea.cpp:170 > if (m_webPage.useFixedLayout()) { > if (m_layerTreeHost) > m_layerTreeHost->sizeDidChange(size); > else if (m_previousLayerTreeHost) > m_previousLayerTreeHost->sizeDidChange(size); Ok, this comes from CoordinatedDrawingArea, it was merged into AcceleratedDrawingArea, but never used by us because the DrawingAreaImpl never chained up. I'm not sure if that was on purpose because this was only used by the multiprocess coordinated graphics implementation or if it was a bug, but since we only have threaded coordinated graphics now, I think we can simply remove this. > Source/WebKit2/WebProcess/WebPage/AcceleratedDrawingArea.cpp:172 > m_webPage.mainFrame()->pageOverlayController().didChangeDocumentSize(); And this can be removed too, since FrameView already takes care of this. 1318142 5 daewoong.jang 2017-06-12 02:21:45 -0700 (In reply to Carlos Garcia Campos from comment #4) > Comment on attachment 312427 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=312427&action=review > > > Source/WebKit2/WebProcess/WebPage/AcceleratedDrawingArea.cpp:170 > > if (m_webPage.useFixedLayout()) { > > if (m_layerTreeHost) > > m_layerTreeHost->sizeDidChange(size); > > else if (m_previousLayerTreeHost) > > m_previousLayerTreeHost->sizeDidChange(size); > > Ok, this comes from CoordinatedDrawingArea, it was merged into > AcceleratedDrawingArea, but never used by us because the DrawingAreaImpl > never chained up. I'm not sure if that was on purpose because this was only > used by the multiprocess coordinated graphics implementation or if it was a > bug, but since we only have threaded coordinated graphics now, I think we > can simply remove this. > > > Source/WebKit2/WebProcess/WebPage/AcceleratedDrawingArea.cpp:172 > > m_webPage.mainFrame()->pageOverlayController().didChangeDocumentSize(); > > And this can be removed too, since FrameView already takes care of this. Alright. I will update the patch as you suggested. Thank you for review. 1318193 6 312656 daewoong.jang 2017-06-12 05:53:56 -0700 Created attachment 312656 updated patch 1318197 7 312656 cgarcia 2017-06-12 05:59:12 -0700 Comment on attachment 312656 updated patch Thanks! 1318231 8 312656 commit-queue 2017-06-12 08:27:33 -0700 Comment on attachment 312656 updated patch Clearing flags on attachment: 312656 Committed r218102: <http://trac.webkit.org/changeset/218102> 1318232 9 commit-queue 2017-06-12 08:27:35 -0700 All reviewed patches have been landed. Closing bug. 312427 2017-06-09 04:52:08 -0700 2017-06-12 05:53:56 -0700 patch bug173153.diff text/plain 2001 daewoong.jang SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxNzk3OCkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE3LTA2LTA5ICBEYWV3b29u ZyBKYW5nICA8ZGFld29vbmcuamFuZ0BuYXZlcmNvcnAuY29tPgorCisgICAgICAgIFtDb29yZGlu YXRlZEdyYXBoaWNzXSBCZWhhdmlvciBjaGFuZ2Ugb2YgRHJhd2luZ0FyZWFJbXBsIGFmdGVyIHIy MTc3NzkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3 MzE1MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICog V2ViUHJvY2Vzcy9XZWJQYWdlL0FjY2VsZXJhdGVkRHJhd2luZ0FyZWEuY3BwOgorICAgICAgICAo V2ViS2l0OjpBY2NlbGVyYXRlZERyYXdpbmdBcmVhOjptYWluRnJhbWVDb250ZW50U2l6ZUNoYW5n ZWQpOgorCiAyMDE3LTA2LTA5ICBEYWV3b29uZyBKYW5nICA8ZGFld29vbmcuamFuZ0BuYXZlcmNv cnAuY29tPgogCiAgICAgICAgIFVzZSBXVEY6OmdldEN1cnJlbnRQcm9jZXNzSUQoKSBpbnN0ZWFk IG9mIGdldHBpZCgpCkluZGV4OiBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvQWNj ZWxlcmF0ZWREcmF3aW5nQXJlYS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvV2Vi UHJvY2Vzcy9XZWJQYWdlL0FjY2VsZXJhdGVkRHJhd2luZ0FyZWEuY3BwCShyZXZpc2lvbiAyMTc5 NTQpCisrKyBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvQWNjZWxlcmF0ZWREcmF3 aW5nQXJlYS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE1NywyMCArMTU3LDIwIEBAIHZvaWQgQWNj ZWxlcmF0ZWREcmF3aW5nQXJlYTo6dXBkYXRlUHJlZmUKIAogdm9pZCBBY2NlbGVyYXRlZERyYXdp bmdBcmVhOjptYWluRnJhbWVDb250ZW50U2l6ZUNoYW5nZWQoY29uc3QgSW50U2l6ZSYgc2l6ZSkK IHsKKyNpZiBVU0UoQ09PUkRJTkFURURfR1JBUEhJQ1NfVEhSRUFERUQpCisgICAgaWYgKG1fbGF5 ZXJUcmVlSG9zdCkKKyAgICAgICAgbV9sYXllclRyZWVIb3N0LT5jb250ZW50c1NpemVDaGFuZ2Vk KHNpemUpOworICAgIGVsc2UgaWYgKG1fcHJldmlvdXNMYXllclRyZWVIb3N0KQorICAgICAgICBt X3ByZXZpb3VzTGF5ZXJUcmVlSG9zdC0+Y29udGVudHNTaXplQ2hhbmdlZChzaXplKTsKKyNlbHNl CiAgICAgaWYgKG1fd2ViUGFnZS51c2VGaXhlZExheW91dCgpKSB7CiAgICAgICAgIGlmIChtX2xh eWVyVHJlZUhvc3QpCiAgICAgICAgICAgICBtX2xheWVyVHJlZUhvc3QtPnNpemVEaWRDaGFuZ2Uo c2l6ZSk7CiAgICAgICAgIGVsc2UgaWYgKG1fcHJldmlvdXNMYXllclRyZWVIb3N0KQogICAgICAg ICAgICAgbV9wcmV2aW91c0xheWVyVHJlZUhvc3QtPnNpemVEaWRDaGFuZ2Uoc2l6ZSk7Ci0gICAg fSBlbHNlIHsKLSNpZiBVU0UoQ09PUkRJTkFURURfR1JBUEhJQ1NfVEhSRUFERUQpCi0gICAgICAg IGlmIChtX2xheWVyVHJlZUhvc3QpCi0gICAgICAgICAgICBtX2xheWVyVHJlZUhvc3QtPmNvbnRl bnRzU2l6ZUNoYW5nZWQoc2l6ZSk7Ci0gICAgICAgIGVsc2UgaWYgKG1fcHJldmlvdXNMYXllclRy ZWVIb3N0KQotICAgICAgICAgICAgbV9wcmV2aW91c0xheWVyVHJlZUhvc3QtPmNvbnRlbnRzU2l6 ZUNoYW5nZWQoc2l6ZSk7Ci0jZW5kaWYKICAgICB9CiAgICAgbV93ZWJQYWdlLm1haW5GcmFtZSgp LT5wYWdlT3ZlcmxheUNvbnRyb2xsZXIoKS5kaWRDaGFuZ2VEb2N1bWVudFNpemUoKTsKKyNlbmRp ZgogfQogCiB2b2lkIEFjY2VsZXJhdGVkRHJhd2luZ0FyZWE6OmxheWVySG9zdERpZEZsdXNoTGF5 ZXJzKCkK 312656 2017-06-12 05:53:56 -0700 2017-06-12 08:27:33 -0700 updated patch bug173153.diff text/plain 1963 daewoong.jang SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxODA5NikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE3LTA2LTEyICBEYWV3b29u ZyBKYW5nICA8ZGFld29vbmcuamFuZ0BuYXZlcmNvcnAuY29tPgorCisgICAgICAgIFtDb29yZGlu YXRlZEdyYXBoaWNzXSBCZWhhdmlvciBjaGFuZ2Ugb2YgRHJhd2luZ0FyZWFJbXBsIGFmdGVyIHIy MTc3NzkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3 MzE1MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICog V2ViUHJvY2Vzcy9XZWJQYWdlL0FjY2VsZXJhdGVkRHJhd2luZ0FyZWEuY3BwOgorICAgICAgICAo V2ViS2l0OjpBY2NlbGVyYXRlZERyYXdpbmdBcmVhOjptYWluRnJhbWVDb250ZW50U2l6ZUNoYW5n ZWQpOgorCiAyMDE3LTA2LTEyICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlh LmNvbT4KIAogICAgICAgICBbR1RLXSBNb3ZlIFdlYktpdCBHVHlwZSBtYWNyb3MgdG8gV1RGCklu ZGV4OiBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvQWNjZWxlcmF0ZWREcmF3aW5n QXJlYS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdl L0FjY2VsZXJhdGVkRHJhd2luZ0FyZWEuY3BwCShyZXZpc2lvbiAyMTgwOTUpCisrKyBTb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvQWNjZWxlcmF0ZWREcmF3aW5nQXJlYS5jcHAJKHdv cmtpbmcgY29weSkKQEAgLTE1NywyMCArMTU3LDE0IEBAIHZvaWQgQWNjZWxlcmF0ZWREcmF3aW5n QXJlYTo6dXBkYXRlUHJlZmUKIAogdm9pZCBBY2NlbGVyYXRlZERyYXdpbmdBcmVhOjptYWluRnJh bWVDb250ZW50U2l6ZUNoYW5nZWQoY29uc3QgSW50U2l6ZSYgc2l6ZSkKIHsKLSAgICBpZiAobV93 ZWJQYWdlLnVzZUZpeGVkTGF5b3V0KCkpIHsKLSAgICAgICAgaWYgKG1fbGF5ZXJUcmVlSG9zdCkK LSAgICAgICAgICAgIG1fbGF5ZXJUcmVlSG9zdC0+c2l6ZURpZENoYW5nZShzaXplKTsKLSAgICAg ICAgZWxzZSBpZiAobV9wcmV2aW91c0xheWVyVHJlZUhvc3QpCi0gICAgICAgICAgICBtX3ByZXZp b3VzTGF5ZXJUcmVlSG9zdC0+c2l6ZURpZENoYW5nZShzaXplKTsKLSAgICB9IGVsc2UgewogI2lm IFVTRShDT09SRElOQVRFRF9HUkFQSElDU19USFJFQURFRCkKLSAgICAgICAgaWYgKG1fbGF5ZXJU cmVlSG9zdCkKLSAgICAgICAgICAgIG1fbGF5ZXJUcmVlSG9zdC0+Y29udGVudHNTaXplQ2hhbmdl ZChzaXplKTsKLSAgICAgICAgZWxzZSBpZiAobV9wcmV2aW91c0xheWVyVHJlZUhvc3QpCi0gICAg ICAgICAgICBtX3ByZXZpb3VzTGF5ZXJUcmVlSG9zdC0+Y29udGVudHNTaXplQ2hhbmdlZChzaXpl KTsKKyAgICBpZiAobV9sYXllclRyZWVIb3N0KQorICAgICAgICBtX2xheWVyVHJlZUhvc3QtPmNv bnRlbnRzU2l6ZUNoYW5nZWQoc2l6ZSk7CisgICAgZWxzZSBpZiAobV9wcmV2aW91c0xheWVyVHJl ZUhvc3QpCisgICAgICAgIG1fcHJldmlvdXNMYXllclRyZWVIb3N0LT5jb250ZW50c1NpemVDaGFu Z2VkKHNpemUpOworI2Vsc2UKKyAgICBVTlVTRURfUEFSQU0oc2l6ZSk7CiAjZW5kaWYKLSAgICB9 Ci0gICAgbV93ZWJQYWdlLm1haW5GcmFtZSgpLT5wYWdlT3ZlcmxheUNvbnRyb2xsZXIoKS5kaWRD aGFuZ2VEb2N1bWVudFNpemUoKTsKIH0KIAogdm9pZCBBY2NlbGVyYXRlZERyYXdpbmdBcmVhOjps YXllckhvc3REaWRGbHVzaExheWVycygpCg==