Changeset 222831 in webkit

Timestamp:

Oct 3, 2017 11:54:20 PM (7 years ago)

Author:

Antti Koivisto

Message:

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (4 diffs)
• dom/Document.h (modified) (2 diffs)
• rendering/RenderFullScreen.cpp (modified) (5 diffs)
• rendering/RenderFullScreen.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -2248 +2248 @@
         view()->willDestroyRenderTree();
 
-#if ENABLE(FULLSCREEN_API)
-    if (m_fullScreenRenderer)
-        setFullScreenRenderer(nullptr);
-#endif
-
     if (m_documentElement)
         RenderTreeUpdater::tearDownRenderers(*m_documentElement);
@@ -6029 +6024 @@
     ASSERT(page()->settings().fullScreenEnabled());
 
-    unwrapFullScreenRenderer(m_fullScreenRenderer, m_fullScreenElement.get());
+    unwrapFullScreenRenderer(m_fullScreenRenderer.get(), m_fullScreenElement.get());
 
     if (element)
@@ -6096 +6091 @@
     m_areKeysEnabledInFullScreen = false;
 
-    unwrapFullScreenRenderer(m_fullScreenRenderer, m_fullScreenElement.get());
+    unwrapFullScreenRenderer(m_fullScreenRenderer.get(), m_fullScreenElement.get());
 
     m_fullScreenElement = nullptr;
@@ -6115 +6110 @@
         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);
 }
 

trunk/Source/WebCore/dom/Document.h

@@ -1104 +1104 @@
     
     void setFullScreenRenderer(RenderFullScreen*);
-    RenderFullScreen* fullScreenRenderer() const { return m_fullScreenRenderer; }
-    void fullScreenRendererDestroyed();
-    
+    RenderFullScreen* fullScreenRenderer() const { return m_fullScreenRenderer.get(); }
+
     void fullScreenChangeDelayTimerFired();
     bool fullScreenIsAllowedForElement(Element*) const;
@@ -1620 +1619 @@
     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;

trunk/Source/WebCore/rendering/RenderFullScreen.cpp

@@ -37 +37 @@
 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))
     {
     }
@@ -45 +44 @@
 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); 
@@ -65 +55 @@
 {
     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();
@@ -195 +179 @@
 }
 
-void RenderFullScreen::setPlaceholder(RenderBlock* placeholder)
-{
-    m_placeholder = placeholder;
-}
-
 void RenderFullScreen::createPlaceholder(std::unique_ptr<RenderStyle> style, const LayoutRect& frameRect)
 {
@@ -215 +194 @@
         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);

trunk/Source/WebCore/rendering/RenderFullScreen.h

@@ -37 +37 @@
     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);
 
@@ -53 +52 @@
 
 protected:
-    RenderBlock* m_placeholder;
+    WeakPtr<RenderBlock> m_placeholder;
 };