Changeset 172487 in webkit

Timestamp:

Aug 12, 2014 1:37:37 PM (10 years ago)

Author:

Antti Koivisto

Message:

Remove isInCanvasSubtree bit
​https://bugs.webkit.org/show_bug.cgi?id=135837

Reviewed by Andreas Kling.

The logic to update this bit is in a wrong place and it is not clear it does
the right thing in all cases. Also the optimization doesn't seem necessary,
the focus code is not that hot.

• accessibility/AXObjectCache.cpp:

(WebCore::AXObjectCache::getOrCreate):

• dom/Element.cpp:

(WebCore::Element::isFocusable):
(WebCore::Element::clearStyleDerivedDataBeforeDetachingRenderer):
(WebCore::Element::setIsInCanvasSubtree): Deleted.
(WebCore::Element::isInCanvasSubtree): Deleted.

• dom/Element.h:
• dom/ElementRareData.h:

(WebCore::ElementRareData::ElementRareData):
(WebCore::ElementRareData::isInCanvasSubtree): Deleted.
(WebCore::ElementRareData::setIsInCanvasSubtree): Deleted.

• html/HTMLAnchorElement.cpp:

(WebCore::HTMLAnchorElement::isKeyboardFocusable):

• html/HTMLCanvasElement.cpp:

(WebCore::HTMLCanvasElement::HTMLCanvasElement):
(WebCore::HTMLCanvasElement::willAttachRenderers): Deleted.

• html/HTMLCanvasElement.h:
• style/StyleResolveTree.cpp:

(WebCore::Style::attachRenderTree):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• accessibility/AXObjectCache.cpp (modified) (2 diffs)
• dom/Element.cpp (modified) (3 diffs)
• dom/Element.h (modified) (1 diff)
• dom/ElementRareData.h (modified) (3 diffs)
• html/HTMLAnchorElement.cpp (modified) (3 diffs)
• html/HTMLCanvasElement.cpp (modified) (2 diffs)
• html/HTMLCanvasElement.h (modified) (1 diff)
• style/StyleResolveTree.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-08-12  Antti Koivisto  <antti@apple.com>
+
+        Remove isInCanvasSubtree bit
+        https://bugs.webkit.org/show_bug.cgi?id=135837
+
+        Reviewed by Andreas Kling.
+
+        The logic to update this bit is in a wrong place and it is not clear it does 
+        the right thing in all cases. Also the optimization doesn't seem necessary,
+        the focus code is not that hot.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::getOrCreate):
+        * dom/Element.cpp:
+        (WebCore::Element::isFocusable):
+        (WebCore::Element::clearStyleDerivedDataBeforeDetachingRenderer):
+        (WebCore::Element::setIsInCanvasSubtree): Deleted.
+        (WebCore::Element::isInCanvasSubtree): Deleted.
+        * dom/Element.h:
+        * dom/ElementRareData.h:
+        (WebCore::ElementRareData::ElementRareData):
+        (WebCore::ElementRareData::isInCanvasSubtree): Deleted.
+        (WebCore::ElementRareData::setIsInCanvasSubtree): Deleted.
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::isKeyboardFocusable):
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::HTMLCanvasElement):
+        (WebCore::HTMLCanvasElement::willAttachRenderers): Deleted.
+        * html/HTMLCanvasElement.h:
+        * style/StyleResolveTree.cpp:
+        (WebCore::Style::attachRenderTree):
+
 2014-08-11  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebCore/accessibility/AXObjectCache.cpp

@@ -59 +59 @@
 #include "Document.h"
 #include "Editor.h"
+#include "ElementIterator.h"
 #include "FocusController.h"
 #include "Frame.h"
 #include "HTMLAreaElement.h"
+#include "HTMLCanvasElement.h"
 #include "HTMLImageElement.h"
 #include "HTMLInputElement.h"
@@ -380 +382 @@
     // It's only allowed to create an AccessibilityObject from a Node if it's in a canvas subtree.
     // Or if it's a hidden element, but we still want to expose it because of other ARIA attributes.
-    bool inCanvasSubtree = node->parentElement()->isInCanvasSubtree();
-    bool isHidden = !node->renderer() && isNodeAriaVisible(node);
+    bool inCanvasSubtree = lineageOfType<HTMLCanvasElement>(*node->parentElement()).first();
+    bool isHidden = isNodeAriaVisible(node);
 
     bool insideMeterElement = false;

trunk/Source/WebCore/dom/Element.cpp

@@ -435 +435 @@
         return false;
 
-    // Elements in canvas fallback content are not rendered, but they are allowed to be
-    // focusable as long as their canvas is displayed and visible.
-    if (isInCanvasSubtree()) {
-        ASSERT(lineageOfType<HTMLCanvasElement>(*this).first());
-        auto& canvas = *lineageOfType<HTMLCanvasElement>(*this).first();
-        return canvas.renderer() && canvas.renderer()->style().visibility() == VISIBLE;
-    }
-
     if (!renderer()) {
         // If the node is in a display:none tree it might say it needs style recalc but
         // the whole document is actually up to date.
         ASSERT(!needsStyleRecalc() || !document().childNeedsStyleRecalc());
+
+        // Elements in canvas fallback content are not rendered, but they are allowed to be
+        // focusable as long as their canvas is displayed and visible.
+        if (auto* canvas = ancestorsOfType<HTMLCanvasElement>(*this).first())
+            return canvas->renderer() && canvas->renderer()->style().visibility() == VISIBLE;
     }
 
@@ -2213 +2210 @@
 }
 
-void Element::setIsInCanvasSubtree(bool isInCanvasSubtree)
-{
-    ensureElementRareData().setIsInCanvasSubtree(isInCanvasSubtree);
-}
-
-bool Element::isInCanvasSubtree() const
-{
-    return hasRareData() && elementRareData()->isInCanvasSubtree();
-}
-
 void Element::setRegionOversetState(RegionOversetState state)
 {
@@ -2891 +2878 @@
         return;
     ElementRareData* data = elementRareData();
-    data->setIsInCanvasSubtree(false);
     data->resetComputedStyle();
     data->resetDynamicRestyleObservations();

trunk/Source/WebCore/dom/Element.h

@@ -383 +383 @@
     void setChildIndex(unsigned);
 
-    void setIsInCanvasSubtree(bool);
-    bool isInCanvasSubtree() const;
-
     void setRegionOversetState(RegionOversetState);
     RegionOversetState regionOversetState() const;

trunk/Source/WebCore/dom/ElementRareData.h

@@ -61 +61 @@
     void setStyleAffectedByEmpty(bool value) { m_styleAffectedByEmpty = value; }
 
-    bool isInCanvasSubtree() const { return m_isInCanvasSubtree; }
-    void setIsInCanvasSubtree(bool value) { m_isInCanvasSubtree = value; }
-
     RegionOversetState regionOversetState() const { return m_regionOversetState; }
     void setRegionOversetState(RegionOversetState state) { m_regionOversetState = state; }
@@ -125 +122 @@
     unsigned m_needsFocusAppearanceUpdateSoonAfterAttach : 1;
     unsigned m_styleAffectedByEmpty : 1;
-    unsigned m_isInCanvasSubtree : 1;
 #if ENABLE(FULLSCREEN_API)
     unsigned m_containsFullScreenElement : 1;
@@ -170 +166 @@
     , m_needsFocusAppearanceUpdateSoonAfterAttach(false)
     , m_styleAffectedByEmpty(false)
-    , m_isInCanvasSubtree(false)
 #if ENABLE(FULLSCREEN_API)
     , m_containsFullScreenElement(false)

trunk/Source/WebCore/html/HTMLAnchorElement.cpp

@@ -27 +27 @@
 #include "Attribute.h"
 #include "DNS.h"
+#include "ElementIterator.h"
 #include "EventHandler.h"
 #include "EventNames.h"
@@ -34 +35 @@
 #include "FrameLoaderTypes.h"
 #include "FrameSelection.h"
+#include "HTMLCanvasElement.h"
 #include "HTMLImageElement.h"
 #include "HTMLParserIdioms.h"
@@ -143 +145 @@
         return false;
 
-    if (isInCanvasSubtree())
+    if (!renderer() && ancestorsOfType<HTMLCanvasElement>(*this).first())
         return true;
 

trunk/Source/WebCore/html/HTMLCanvasElement.cpp

@@ -86 +86 @@
 {
     ASSERT(hasTagName(canvasTag));
-    setHasCustomStyleResolveCallbacks();
 }
 
@@ -124 +123 @@
     m_rendererIsCanvas = false;
     return HTMLElement::createElementRenderer(WTF::move(style));
-}
-
-void HTMLCanvasElement::willAttachRenderers()
-{
-    setIsInCanvasSubtree(true);
 }
 

trunk/Source/WebCore/html/HTMLCanvasElement.h

@@ -143 +143 @@
     virtual void parseAttribute(const QualifiedName&, const AtomicString&) override;
     virtual RenderPtr<RenderElement> createElementRenderer(PassRef<RenderStyle>) override;
-    virtual void willAttachRenderers() override;
 
     virtual bool canContainRangeEndPoint() const override;

trunk/Source/WebCore/style/StyleResolveTree.cpp

@@ -588 +588 @@
     createRendererIfNeeded(current, inheritedStyle, renderTreePosition, resolvedStyle);
 
-    if (current.parentElement() && current.parentElement()->isInCanvasSubtree())
-        current.setIsInCanvasSubtree(true);
-
     StyleResolverParentPusher parentPusher(&current);