Use m_webView->size() for viewport update
Created attachment 175128 [details] Patch
Created attachment 175130 [details] Patch
Comment on attachment 175130 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175130&action=review I think the general concept here is good, but I think there is relatively weak evidence that this won't have unexpected changes in behavior. > Source/WebKit/chromium/ChangeLog:12 > + rendering purpose. While the intent of this change seems good, it seems scary to apply such a modification without at least identifying some test cases that cover this behavior. From the sound of the bug, it should be possible to construct a test case illustrating the case of a pending resize that should not be used for the current rendering.
(In reply to comment #3) > (From update of attachment 175130 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=175130&action=review > > I think the general concept here is good, but I think there is relatively weak evidence that this won't have unexpected changes in behavior. > > > Source/WebKit/chromium/ChangeLog:12 > > + rendering purpose. > > While the intent of this change seems good, it seems scary to apply such a modification without at least identifying some test cases that cover this behavior. From the sound of the bug, it should be possible to construct a test case illustrating the case of a pending resize that should not be used for the current rendering. Makes sense. The real life bug I have is this one: https://code.google.com/p/chromium/issues/detail?id=160619 That windowRect will be one frame behind when rotating an Android device. I think I can probably do a WebFrameTest for that. I'll try tomorrow.
"Your client does not have permission to get URL /p/chromium/issues/detail?id=160619 from this server."
Comment on attachment 175130 [details] Patch Can you add a test?
(In reply to comment #3) > (From update of attachment 175130 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=175130&action=review > > I think the general concept here is good, but I think there is relatively weak evidence that this won't have unexpected changes in behavior. > > > Source/WebKit/chromium/ChangeLog:12 > > + rendering purpose. > > While the intent of this change seems good, it seems scary to apply such a modification without at least identifying some test cases that cover this behavior. From the sound of the bug, it should be possible to construct a test case illustrating the case of a pending resize that should not be used for the current rendering. I just added some debug output to RenderWidget::setWindowRect and RenderWidget::OnResize. I think it is very wrong to use windowRect in the first place. The semantics of windowRect is the outer rect of the window, that is, including all decorations like the title bar, status bar, navigation buttons and such. I don't think it makes any sense to use it for layout purpose. (In reply to comment #6) > (From update of attachment 175130 [details]) > Can you add a test? Instead of adding tests, I think we should modify all tests in WebFrameTest to remove the windowRect() override in WebViewClient implementations. The tests should still work without windowRect().
> Instead of adding tests, I think we should modify all tests in WebFrameTest to remove the windowRect() override in WebViewClient implementations. The tests should still work without windowRect(). That sounds worth trying.
Created attachment 175313 [details] Patch
Comment on attachment 175313 [details] Patch Thanks for updating the tests.
Comment on attachment 175313 [details] Patch Clearing flags on attachment: 175313 Committed r135442: <http://trac.webkit.org/changeset/135442>
All reviewed patches have been landed. Closing bug.