Use WeakPtr in RenderFullScreen
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Oct 2017 06:54:20 +0000 (06:54 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Oct 2017 06:54:20 +0000 (06:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177827

Reviewed by Zalan Bujtas.

Make Document::m_fullScreenRenderer RenderFullScreen::m_placeholder WeakPtrs
and get rid of the custom nulling code.

* dom/Document.cpp:
(WebCore::Document::destroyRenderTree):
(WebCore::Document::webkitWillEnterFullScreenForElement):
(WebCore::Document::webkitDidExitFullScreenForElement):
(WebCore::Document::setFullScreenRenderer):
(WebCore::Document::fullScreenRendererDestroyed): Deleted.
* dom/Document.h:
(WebCore::Document::fullScreenRenderer const):
* rendering/RenderFullScreen.cpp:
(WebCore::RenderFullScreen::RenderFullScreen):
(WebCore::RenderFullScreen::willBeDestroyed):
(WebCore::RenderFullScreen::createPlaceholder):
(WebCore::RenderFullScreenPlaceholder::willBeDestroyed): Deleted.
(WebCore::RenderFullScreen::setPlaceholder): Deleted.
* rendering/RenderFullScreen.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222831 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/rendering/RenderFullScreen.cpp
Source/WebCore/rendering/RenderFullScreen.h

index e7e3be5..3852cd9 100644 (file)
@@ -1,3 +1,29 @@
+2017-10-03  Antti Koivisto  <antti@apple.com>
+
+        Use WeakPtr in RenderFullScreen
+        https://bugs.webkit.org/show_bug.cgi?id=177827
+
+        Reviewed by Zalan Bujtas.
+
+        Make Document::m_fullScreenRenderer RenderFullScreen::m_placeholder WeakPtrs
+        and get rid of the custom nulling code.
+
+        * dom/Document.cpp:
+        (WebCore::Document::destroyRenderTree):
+        (WebCore::Document::webkitWillEnterFullScreenForElement):
+        (WebCore::Document::webkitDidExitFullScreenForElement):
+        (WebCore::Document::setFullScreenRenderer):
+        (WebCore::Document::fullScreenRendererDestroyed): Deleted.
+        * dom/Document.h:
+        (WebCore::Document::fullScreenRenderer const):
+        * rendering/RenderFullScreen.cpp:
+        (WebCore::RenderFullScreen::RenderFullScreen):
+        (WebCore::RenderFullScreen::willBeDestroyed):
+        (WebCore::RenderFullScreen::createPlaceholder):
+        (WebCore::RenderFullScreenPlaceholder::willBeDestroyed): Deleted.
+        (WebCore::RenderFullScreen::setPlaceholder): Deleted.
+        * rendering/RenderFullScreen.h:
+
 2017-10-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         Enable pasteboard custom data in macOS 10.12 and earlier
index 35fe2c6..f39dd16 100644 (file)
@@ -2247,11 +2247,6 @@ void Document::destroyRenderTree()
     if (view())
         view()->willDestroyRenderTree();
 
-#if ENABLE(FULLSCREEN_API)
-    if (m_fullScreenRenderer)
-        setFullScreenRenderer(nullptr);
-#endif
-
     if (m_documentElement)
         RenderTreeUpdater::tearDownRenderers(*m_documentElement);
 
@@ -6028,7 +6023,7 @@ void Document::webkitWillEnterFullScreenForElement(Element* element)
 
     ASSERT(page()->settings().fullScreenEnabled());
 
-    unwrapFullScreenRenderer(m_fullScreenRenderer, m_fullScreenElement.get());
+    unwrapFullScreenRenderer(m_fullScreenRenderer.get(), m_fullScreenElement.get());
 
     if (element)
         element->willBecomeFullscreenElement();
@@ -6095,7 +6090,7 @@ void Document::webkitDidExitFullScreenForElement(Element*)
 
     m_areKeysEnabledInFullScreen = false;
 
-    unwrapFullScreenRenderer(m_fullScreenRenderer, m_fullScreenElement.get());
+    unwrapFullScreenRenderer(m_fullScreenRenderer.get(), m_fullScreenElement.get());
 
     m_fullScreenElement = nullptr;
     scheduleForcedStyleRecalc();
@@ -6114,23 +6109,20 @@ void Document::setFullScreenRenderer(RenderFullScreen* renderer)
     if (renderer == m_fullScreenRenderer)
         return;
 
-    if (renderer && m_savedPlaceholderRenderStyle) 
-        renderer->createPlaceholder(WTFMove(m_savedPlaceholderRenderStyle), m_savedPlaceholderFrameRect);
-    else if (renderer && m_fullScreenRenderer && m_fullScreenRenderer->placeholder()) {
-        RenderBlock* placeholder = m_fullScreenRenderer->placeholder();
-        renderer->createPlaceholder(RenderStyle::clonePtr(placeholder->style()), placeholder->frameRect());
+    if (renderer) {
+        if (m_savedPlaceholderRenderStyle)
+            renderer->createPlaceholder(WTFMove(m_savedPlaceholderRenderStyle), m_savedPlaceholderFrameRect);
+        else if (m_fullScreenRenderer && m_fullScreenRenderer->placeholder()) {
+            auto* placeholder = m_fullScreenRenderer->placeholder();
+            renderer->createPlaceholder(RenderStyle::clonePtr(placeholder->style()), placeholder->frameRect());
+        }
     }
 
     if (m_fullScreenRenderer)
-        m_fullScreenRenderer->destroy();
+        m_fullScreenRenderer->removeFromParentAndDestroy();
     ASSERT(!m_fullScreenRenderer);
 
-    m_fullScreenRenderer = renderer;
-}
-
-void Document::fullScreenRendererDestroyed()
-{
-    m_fullScreenRenderer = nullptr;
+    m_fullScreenRenderer = makeWeakPtr(renderer);
 }
 
 void Document::fullScreenChangeDelayTimerFired()
index 0fbf106..d5f7e8b 100644 (file)
@@ -1103,9 +1103,8 @@ public:
     WEBCORE_EXPORT void webkitDidExitFullScreenForElement(Element*);
     
     void setFullScreenRenderer(RenderFullScreen*);
-    RenderFullScreen* fullScreenRenderer() const { return m_fullScreenRenderer; }
-    void fullScreenRendererDestroyed();
-    
+    RenderFullScreen* fullScreenRenderer() const { return m_fullScreenRenderer.get(); }
+
     void fullScreenChangeDelayTimerFired();
     bool fullScreenIsAllowedForElement(Element*) const;
     void fullScreenElementRemoved();
@@ -1619,7 +1618,7 @@ private:
 #if ENABLE(FULLSCREEN_API)
     RefPtr<Element> m_fullScreenElement;
     Vector<RefPtr<Element>> m_fullScreenElementStack;
-    RenderFullScreen* m_fullScreenRenderer { nullptr };
+    WeakPtr<RenderFullScreen> m_fullScreenRenderer { nullptr };
     Timer m_fullScreenChangeDelayTimer;
     Deque<RefPtr<Node>> m_fullScreenChangeEventTargetQueue;
     Deque<RefPtr<Node>> m_fullScreenErrorEventTargetQueue;
index 69888cd..94aeb11 100644 (file)
@@ -36,27 +36,17 @@ namespace WebCore {
 
 class RenderFullScreenPlaceholder final : public RenderBlockFlow {
 public:
-    RenderFullScreenPlaceholder(RenderFullScreen& owner, RenderStyle&& style)
-        : RenderBlockFlow(owner.document(), WTFMove(style))
-        , m_owner(owner) 
+    RenderFullScreenPlaceholder(Document& document, RenderStyle&& style)
+        : RenderBlockFlow(document, WTFMove(style))
     {
     }
 
 private:
     bool isRenderFullScreenPlaceholder() const override { return true; }
-    void willBeDestroyed() override;
-    RenderFullScreen& m_owner;
 };
 
-void RenderFullScreenPlaceholder::willBeDestroyed()
-{
-    m_owner.setPlaceholder(0);
-    RenderBlockFlow::willBeDestroyed();
-}
-
 RenderFullScreen::RenderFullScreen(Document& document, RenderStyle&& style)
     : RenderFlexibleBox(document, WTFMove(style))
-    , m_placeholder(0)
 {
     setReplaced(false); 
 }
@@ -64,16 +54,10 @@ RenderFullScreen::RenderFullScreen(Document& document, RenderStyle&& style)
 void RenderFullScreen::willBeDestroyed()
 {
     if (m_placeholder) {
-        if (!m_placeholder->beingDestroyed())
-            m_placeholder->destroy();
+        m_placeholder->removeFromParentAndDestroy();
         ASSERT(!m_placeholder);
     }
 
-    // RenderObjects are unretained, so notify the document (which holds a pointer to a RenderFullScreen)
-    // if it's RenderFullScreen is destroyed.
-    if (document().fullScreenRenderer() == this)
-        document().fullScreenRendererDestroyed();
-
     RenderFlexibleBox::willBeDestroyed();
 }
 
@@ -194,11 +178,6 @@ void RenderFullScreen::unwrapRenderer(bool& requiresRenderTreeRebuild)
     removeFromParentAndDestroy();
 }
 
-void RenderFullScreen::setPlaceholder(RenderBlock* placeholder)
-{
-    m_placeholder = placeholder;
-}
-
 void RenderFullScreen::createPlaceholder(std::unique_ptr<RenderStyle> style, const LayoutRect& frameRect)
 {
     if (style->width().isAuto())
@@ -214,10 +193,10 @@ void RenderFullScreen::createPlaceholder(std::unique_ptr<RenderStyle> style, con
     if (!parent())
         return;
 
-    auto newPlaceholder = createRenderer<RenderFullScreenPlaceholder>(*this, WTFMove(*style));
+    auto newPlaceholder = createRenderer<RenderFullScreenPlaceholder>(document(), WTFMove(*style));
     newPlaceholder->initializeStyle();
 
-    m_placeholder = newPlaceholder.get();
+    m_placeholder = makeWeakPtr(*newPlaceholder);
 
     parent()->addChild(WTFMove(newPlaceholder), this);
     parent()->setNeedsLayoutAndPrefWidthsRecalc();
index 337fcd7..ce3cbf5 100644 (file)
@@ -36,8 +36,7 @@ public:
 
     const char* renderName() const override { return "RenderFullScreen"; }
 
-    void setPlaceholder(RenderBlock*);
-    RenderBlock* placeholder() { return m_placeholder; }
+    RenderBlock* placeholder() { return m_placeholder.get(); }
     void createPlaceholder(std::unique_ptr<RenderStyle>, const LayoutRect& frameRect);
 
     static RenderPtr<RenderFullScreen> wrapNewRenderer(RenderPtr<RenderElement>, RenderElement& parent, Document&);
@@ -52,7 +51,7 @@ private:
     bool isFlexibleBoxImpl() const override { return true; }
 
 protected:
-    RenderBlock* m_placeholder;
+    WeakPtr<RenderBlock> m_placeholder;
 };
 
 } // namespace WebCore