[webkit-reviews] review denied: [Bug 69298] Add an option to suppress rendering until the document's load event fires. : [Attachment 109565] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 4 09:49:10 PDT 2011


Simon Fraser (smfr) <simon.fraser at apple.com> has denied Andy Estes
<aestes at apple.com>'s request for review:
Bug 69298: Add an option to suppress rendering until the document's load event
fires.
https://bugs.webkit.org/show_bug.cgi?id=69298

Attachment 109565: Patch
https://bugs.webkit.org/attachment.cgi?id=109565&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=109565&action=review


r- pending discussion of how to slice the preferences.

> Source/WebCore/page/Settings.h:469
> +	   void setSuppressRenderingWhileInitiallyLoading(bool flag) {
m_suppressRenderingWhileInitiallyLoading = flag; }
> +	   bool suppressRenderingWhileInitiallyLoading() const { return
m_suppressRenderingWhileInitiallyLoading; }

Is this a good name for the setting? I wonder if we might want to extend the
influence of this setting layer to do things like avoiding painting
partially-loaded images. There's a risk of setting-creep here.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:251
> +	   &&
m_renderView->document()->settings()->suppressRenderingWhileInitiallyLoading()
> +	   && !m_renderView->document()->loadEventFinished())

Would be nice to wrap these two tests in their own method (on document?), like
Document::visibleUpdatesAllowed() or something.


More information about the webkit-reviews mailing list