Element focus appearance update should be either immediate or a post-layout task
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Sep 2017 01:17:16 +0000 (01:17 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Sep 2017 01:17:16 +0000 (01:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176193

Reviewed by Antti Koivisto.

This change removes an old mechanism for avoiding element focus appearance updates
while we might be in the middle of attaching a renderer.

Focus appearance updates depend on a clean layout, since they want to be able to
scroll the element into the visible viewport if needed.
We now simply do the updates either immediately after layout in Element::focus(),
or as a post-layout callback when needed for HTMLInputElement::didAttachRenderers().

* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::updateFocusAppearanceSoon): Deleted.
(WebCore::Document::cancelFocusAppearanceUpdate): Deleted.
(WebCore::Document::updateFocusAppearanceTimerFired): Deleted.
* dom/Document.h:
* dom/Element.cpp:
(WebCore::Element::focus):
(WebCore::Element::blur):
(WebCore::Element::clearStyleDerivedDataBeforeDetachingRenderer):
(WebCore::Element::updateFocusAppearanceAfterAttachIfNeeded): Deleted.
(WebCore::Element::cancelFocusAppearanceUpdate): Deleted.
* dom/Element.h:
* dom/ElementRareData.h:
(WebCore::ElementRareData::ElementRareData):
(WebCore::ElementRareData::needsFocusAppearanceUpdateSoonAfterAttach const): Deleted.
(WebCore::ElementRareData::setNeedsFocusAppearanceUpdateSoonAfterAttach): Deleted.
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::didAttachRenderers):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ElementRareData.h
Source/WebCore/html/HTMLInputElement.cpp

index e559272..9270f68 100644 (file)
@@ -1,3 +1,38 @@
+2017-08-31  Andreas Kling  <akling@apple.com>
+
+        Element focus appearance update should be either immediate or a post-layout task
+        https://bugs.webkit.org/show_bug.cgi?id=176193
+
+        Reviewed by Antti Koivisto.
+
+        This change removes an old mechanism for avoiding element focus appearance updates
+        while we might be in the middle of attaching a renderer.
+
+        Focus appearance updates depend on a clean layout, since they want to be able to
+        scroll the element into the visible viewport if needed.
+        We now simply do the updates either immediately after layout in Element::focus(),
+        or as a post-layout callback when needed for HTMLInputElement::didAttachRenderers().
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::updateFocusAppearanceSoon): Deleted.
+        (WebCore::Document::cancelFocusAppearanceUpdate): Deleted.
+        (WebCore::Document::updateFocusAppearanceTimerFired): Deleted.
+        * dom/Document.h:
+        * dom/Element.cpp:
+        (WebCore::Element::focus):
+        (WebCore::Element::blur):
+        (WebCore::Element::clearStyleDerivedDataBeforeDetachingRenderer):
+        (WebCore::Element::updateFocusAppearanceAfterAttachIfNeeded): Deleted.
+        (WebCore::Element::cancelFocusAppearanceUpdate): Deleted.
+        * dom/Element.h:
+        * dom/ElementRareData.h:
+        (WebCore::ElementRareData::ElementRareData):
+        (WebCore::ElementRareData::needsFocusAppearanceUpdateSoonAfterAttach const): Deleted.
+        (WebCore::ElementRareData::setNeedsFocusAppearanceUpdateSoonAfterAttach): Deleted.
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::didAttachRenderers):
+
 2017-08-31  Don Olmstead  <don.olmstead@sony.com>
 
         [CMake] Make USE_CF conditional within Windows
index 9d511c8..2d3e2ed 100644 (file)
@@ -461,7 +461,6 @@ Document::Document(Frame* frame, const URL& url, unsigned documentClasses, unsig
     , m_visitedLinkState(std::make_unique<VisitedLinkState>(*this))
     , m_markers(std::make_unique<DocumentMarkerController>(*this))
     , m_styleRecalcTimer([this] { updateStyleIfNeeded(); })
-    , m_updateFocusAppearanceTimer(*this, &Document::updateFocusAppearanceTimerFired)
     , m_documentCreationTime(MonotonicTime::now())
     , m_scriptRunner(std::make_unique<ScriptRunner>(*this))
     , m_moduleLoader(std::make_unique<ScriptModuleLoader>(*this))
@@ -5480,29 +5479,6 @@ void Document::statePopped(Ref<SerializedScriptValue>&& stateObject)
         m_pendingStateObject = WTFMove(stateObject);
 }
 
-void Document::updateFocusAppearanceSoon(SelectionRestorationMode mode)
-{
-    m_updateFocusAppearanceRestoresSelection = mode;
-    if (!m_updateFocusAppearanceTimer.isActive())
-        m_updateFocusAppearanceTimer.startOneShot(0_s);
-}
-
-void Document::cancelFocusAppearanceUpdate()
-{
-    m_updateFocusAppearanceTimer.stop();
-}
-
-void Document::updateFocusAppearanceTimerFired()
-{
-    Element* element = focusedElement();
-    if (!element)
-        return;
-
-    updateLayout();
-    if (element->isFocusable())
-        element->updateFocusAppearance(m_updateFocusAppearanceRestoresSelection);
-}
-
 void Document::attachRange(Range* range)
 {
     ASSERT(!m_ranges.contains(range));
index c61377e..d6df20b 100644 (file)
@@ -966,9 +966,6 @@ public:
     bool hasNodesWithMissingStyle() const { return m_hasNodesWithMissingStyle; }
     void setHasNodesWithMissingStyle() { m_hasNodesWithMissingStyle = true; }
 
-    void updateFocusAppearanceSoon(SelectionRestorationMode);
-    void cancelFocusAppearanceUpdate();
-
     // Extension for manipulating canvas drawing contexts for use in CSS
     std::optional<RenderingContext> getCSSCanvasContext(const String& type, const String& name, int width, int height);
     HTMLCanvasElement* getCSSCanvasElement(const String& name);
@@ -1412,7 +1409,6 @@ private:
 
     void updateTitleFromTitleElement();
     void updateTitle(const StringWithDirection&);
-    void updateFocusAppearanceTimerFired();
     void updateBaseURL();
 
     void buildAccessKeyMap(TreeScope* root);
@@ -1537,7 +1533,6 @@ private:
     const std::unique_ptr<DocumentMarkerController> m_markers;
     
     Timer m_styleRecalcTimer;
-    Timer m_updateFocusAppearanceTimer;
 
     Element* m_cssTarget { nullptr };
 
@@ -1732,7 +1727,6 @@ private:
     PageCacheState m_pageCacheState { NotInPageCache };
     ReferrerPolicy m_referrerPolicy { ReferrerPolicy::NoReferrerWhenDowngrade };
     ReadyState m_readyState { Complete };
-    SelectionRestorationMode m_updateFocusAppearanceRestoresSelection { SelectionRestorationMode::SetDefault };
 
     MutationObserverOptions m_mutationObserverTypes { 0 };
 
index 1d0ccf7..f8a1c26 100644 (file)
@@ -2458,13 +2458,6 @@ void Element::focus(bool restorePreviousSelection, FocusDirection direction)
     // Setting the focused node above might have invalidated the layout due to scripts.
     document().updateLayoutIgnorePendingStylesheets();
 
-    if (!isFocusable()) {
-        ensureElementRareData().setNeedsFocusAppearanceUpdateSoonAfterAttach(true);
-        return;
-    }
-        
-    cancelFocusAppearanceUpdate();
-
     SelectionRevealMode revealMode = SelectionRevealMode::Reveal;
 #if PLATFORM(IOS)
     // Focusing a form element triggers animation in UIKit to scroll to the right position.
@@ -2478,18 +2471,6 @@ void Element::focus(bool restorePreviousSelection, FocusDirection direction)
     updateFocusAppearance(restorePreviousSelection ? SelectionRestorationMode::Restore : SelectionRestorationMode::SetDefault, revealMode);
 }
 
-void Element::updateFocusAppearanceAfterAttachIfNeeded()
-{
-    if (!hasRareData())
-        return;
-    ElementRareData* data = elementRareData();
-    if (!data->needsFocusAppearanceUpdateSoonAfterAttach())
-        return;
-    if (isFocusable() && document().focusedElement() == this)
-        document().updateFocusAppearanceSoon(SelectionRestorationMode::SetDefault);
-    data->setNeedsFocusAppearanceUpdateSoonAfterAttach(false);
-}
-
 void Element::updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode revealMode)
 {
     if (isRootEditableElement()) {
@@ -2518,7 +2499,6 @@ void Element::updateFocusAppearance(SelectionRestorationMode, SelectionRevealMod
 
 void Element::blur()
 {
-    cancelFocusAppearanceUpdate();
     if (treeScope().focusedElementInScope() == this) {
         if (Frame* frame = document().frame())
             frame->page()->focusController().setFocusedElement(nullptr, *frame);
@@ -2927,14 +2907,6 @@ Locale& Element::locale() const
     return document().getCachedLocale(computeInheritedLanguage());
 }
 
-void Element::cancelFocusAppearanceUpdate()
-{
-    if (hasRareData())
-        elementRareData()->setNeedsFocusAppearanceUpdateSoonAfterAttach(false);
-    if (document().focusedElement() == this)
-        document().cancelFocusAppearanceUpdate();
-}
-
 void Element::normalizeAttributes()
 {
     if (!hasAttributes())
@@ -3528,7 +3500,6 @@ void Element::resetStyleRelations()
 void Element::clearStyleDerivedDataBeforeDetachingRenderer()
 {
     unregisterNamedFlowContentElement();
-    cancelFocusAppearanceUpdate();
     clearBeforePseudoElement();
     clearAfterPseudoElement();
 }
index 42f46cf..a2fbdf8 100644 (file)
@@ -639,8 +639,6 @@ private:
     void formatForDebugger(char* buffer, unsigned length) const override;
 #endif
 
-    void cancelFocusAppearanceUpdate();
-
     // The cloneNode function is private so that non-virtual cloneElementWith/WithoutChildren are used instead.
     Ref<Node> cloneNodeInternal(Document&, CloningOperation) override;
     virtual Ref<Element> cloneElementWithoutAttributesAndChildren(Document&);
index 3b2f29d..bc537e6 100644 (file)
@@ -51,9 +51,6 @@ public:
     bool tabIndexSetExplicitly() const { return m_tabIndexWasSetExplicitly; }
     void clearTabIndexExplicitly() { m_tabIndex = 0; m_tabIndexWasSetExplicitly = false; }
 
-    bool needsFocusAppearanceUpdateSoonAfterAttach() const { return m_needsFocusAppearanceUpdateSoonAfterAttach; }
-    void setNeedsFocusAppearanceUpdateSoonAfterAttach(bool needs) { m_needsFocusAppearanceUpdateSoonAfterAttach = needs; }
-
     bool styleAffectedByActive() const { return m_styleAffectedByActive; }
     void setStyleAffectedByActive(bool value) { m_styleAffectedByActive = value; }
 
@@ -120,7 +117,6 @@ private:
     int m_tabIndex;
     unsigned short m_childIndex;
     unsigned m_tabIndexWasSetExplicitly : 1;
-    unsigned m_needsFocusAppearanceUpdateSoonAfterAttach : 1;
     unsigned m_styleAffectedByActive : 1;
     unsigned m_styleAffectedByEmpty : 1;
     unsigned m_styleAffectedByFocusWithin : 1;
@@ -166,7 +162,6 @@ inline ElementRareData::ElementRareData(RenderElement* renderer)
     , m_tabIndex(0)
     , m_childIndex(0)
     , m_tabIndexWasSetExplicitly(false)
-    , m_needsFocusAppearanceUpdateSoonAfterAttach(false)
     , m_styleAffectedByActive(false)
     , m_styleAffectedByEmpty(false)
     , m_styleAffectedByFocusWithin(false)
index 373779f..c08cf33 100644 (file)
@@ -817,8 +817,11 @@ void HTMLInputElement::didAttachRenderers()
 
     m_inputType->attach();
 
-    if (document().focusedElement() == this)
-        document().updateFocusAppearanceSoon(SelectionRestorationMode::Restore);
+    if (document().focusedElement() == this) {
+        document().view()->queuePostLayoutCallback([protectedThis = makeRef(*this)] {
+            protectedThis->updateFocusAppearance(SelectionRestorationMode::Restore, SelectionRevealMode::Reveal);
+        });
+    }
 }
 
 void HTMLInputElement::didDetachRenderers()