See summary.
Created attachment 116448 [details] Patch
Comment on attachment 116448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116448&action=review > Source/WebKit/chromium/public/WebFrame.h:168 > + virtual WebSize visibleFrameSize() const = 0; nit: you might want to put this above contentsSize since the next one in the list is also prefixed with content*. nit: maybe this method should just be called size()? it seems correct to refer to it as the "size of the frame", which translates naturally to WebFrame::size(). we also have WebWidget::size(), which is the size of the WebWidget (the size of the viewport). > Source/WebKit/chromium/public/WebFrameClient.h:125 > + virtual bool shouldAutoSize(WebFrame*, WebSize* /* minSize */, WebSize* /* maxSize */) { return false; } nit: is it really necessary to comment out the parameter names? we normally don't compile with that warning enabled. has something changed? do you really have use cases for asking if an iframe should be autosized? or do you only care about this for the main frame? if only for the main frame, then maybe this should just be a method on WebViewClient?
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Created attachment 116456 [details] Patch
All done as requested.
Comment on attachment 116456 [details] Patch Investigating another approach per discussion with Darin.
Created attachment 117452 [details] Patch
Comment on attachment 117452 [details] Patch Attachment 117452 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10696750
(In reply to comment #8) > (From update of attachment 117452 [details]) > Attachment 117452 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/10696750 Due to 73420 not being landed yet.
Comment on attachment 117452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117452&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:2311 > +void WebViewImpl::queueBothResizeEventAndPaint() why is this method called "queueBoth" and not just sendResizeEventAndPaint. It seems to do exactly those functions without queuing anything. I think you may want to use the term "repaint" instead of "paint" since I believe calling didInvalidateRect is really about requesting a paint instead of actually doing the painting. Plus, I think ChromeClient (and WebCore) uses the term repaint for this function.
Comment on attachment 117452 [details] Patch R=me assuming the renaming happens.
Committed as http://trac.webkit.org/changeset/101759 Did renaming. I also went through the change made settled on the resize naming for everything in this patch. (I need to go back and change FrameView. I left m_minAutoSize as is because m_minAutoResize didn't make sense to me.)
Comment on attachment 117452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117452&action=review >> Source/WebKit/chromium/src/WebViewImpl.cpp:2311 >> +void WebViewImpl::queueBothResizeEventAndPaint() > > why is this method called "queueBoth" and not just sendResizeEventAndPaint. It seems to do exactly those functions without queuing anything. > > I think you may want to use the term "repaint" instead of "paint" since I believe calling didInvalidateRect is really about requesting a paint instead of actually doing the painting. Plus, I think ChromeClient (and WebCore) uses the term repaint for this function. I ran across this code today. I think it's wrong. WebViewImpl should never be responsible for sending either repaint or resize events. That's the FrameView's job. I'm fixing FrameView to not delay initial layout. That's causing tests for resizeEvents to fail, as we now (correctly) are sending 2 resize events. One from the FrameView code (which was being suppressed because layout was suppressed for the first 250ms) and a second from this code. :(