Now that we're getting better at felling render trees, it seems like we can make some promises about there always being a Page while a tree is standing. Having a Page& RenderObject::page() will get rid of a ton of sketchy branches.
Created attachment 298118 [details] Patch for EWS
Created attachment 298123 [details] Patch II for EWS
Created attachment 298127 [details] Patch
Created attachment 298128 [details] Patch
Comment on attachment 298128 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=298128&action=review Almost enough to make me think I don’t need to keep moving things from Page to MainFrame. > Source/WebCore/rendering/RenderLayerCompositor.cpp:4145 > +Page& RenderLayerCompositor::page() const > +{ > + return m_renderView.page(); > +} Can this be inlined, or would that require adding more includes? > Source/WebCore/rendering/RenderObject.cpp:146 > RenderTheme& RenderObject::theme() const > { > - ASSERT(document().page()); > - return document().page()->theme(); > + return page().theme(); > } Can this be inlined, or would that require adding more includes? > Source/WebCore/rendering/RenderObject.h:1013 > + ASSERT(frame().page()); I would love it if we added a brief comment here explaining why this is guaranteed to be non-null. > Source/WebCore/rendering/RenderTheme.cpp:789 > + return o.page().focusController().isActive(); How about the word object instead of "o"?
(In reply to comment #5) > Comment on attachment 298128 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=298128&action=review > > Almost enough to make me think I don’t need to keep moving things from Page > to MainFrame. Almost! > > Source/WebCore/rendering/RenderLayerCompositor.cpp:4145 > > +Page& RenderLayerCompositor::page() const > > +{ > > + return m_renderView.page(); > > +} > > Can this be inlined, or would that require adding more includes? Yeah, include cycle problem. We can solve it but I don't think it's important. > > Source/WebCore/rendering/RenderObject.cpp:146 > > RenderTheme& RenderObject::theme() const > > { > > - ASSERT(document().page()); > > - return document().page()->theme(); > > + return page().theme(); > > } > > Can this be inlined, or would that require adding more includes? Same here. > > Source/WebCore/rendering/RenderObject.h:1013 > > + ASSERT(frame().page()); > > I would love it if we added a brief comment here explaining why this is > guaranteed to be non-null. Ok, I will add a comment here, and a longer comment in Frame::willDetachPage().
Created attachment 298197 [details] Patch for landing
Comment on attachment 298197 [details] Patch for landing Clearing flags on attachment: 298197 Committed r210436: <http://trac.webkit.org/changeset/210436>
All reviewed patches have been landed. Closing bug.