Changeset 52778 in webkit

Timestamp:

Jan 4, 2010 5:35:59 PM (14 years ago)

Author:

Darin Adler

Message:

Selection-related code needs stricter rules about how it relates to layout
​https://bugs.webkit.org/show_bug.cgi?id=32882

Reviewed by Maciej Stachowiak.

Covered by existing tests along with the new assertions.

• dom/Document.cpp:

(WebCore::Document::recalcStyle): Make sure that m_inStyleRecalc is
already false by the time post-attach callbacks are done so that
layout triggered inside those callbacks can work properly.

• editing/SelectionController.cpp:

(WebCore::SelectionController::layout): Added code to trigger a
layout when it's needed.
(WebCore::SelectionController::recomputeCaretRect): Removed unneeded
code to do nothing when FrameView is 0. Added an assertion that layout
is not needed at the time the function is called.
(WebCore::SelectionController::invalidateCaretRect): Added code to
trigger a layout when it's needed.
(WebCore::SelectionController::paintCaret): Added an assertion that
layout is not needed at the time the function is called.

• html/HTMLFormControlElement.cpp:

(WebCore::shouldAutofocus): Added. Helper function that expresses
the rule for which form control elements should auto-focus.
(WebCore::focusPostAttach): Added. Called post-attach to focus an
element if we discover it should be focused during attach.
(WebCore::HTMLFormControlElement::attach): Refactored code for
which elements need auto-focus into a separate function. Instead
of focusing right away, use the focusPostAttach function to focus
after attach is done. Also added calls to suspendPostAttachCallbacks
and resumePostAttachCallbacks so post-attach callbacks happen late
enough. Before, they could run inside the base attach function.

• html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::attach): Added calls to
suspendPostAttachCallbacks and resumePostAttachCallbacks so
post-attach callbacks happen late enough

• page/Frame.cpp:

(WebCore::Frame::revealSelection): Added code to trigger a layout
when it's needed.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (1 diff)
• editing/SelectionController.cpp (modified) (5 diffs)
• html/HTMLFormControlElement.cpp (modified) (3 diffs)
• html/HTMLInputElement.cpp (modified) (3 diffs)
• page/Frame.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-01-04  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Selection-related code needs stricter rules about how it relates to layout
+        https://bugs.webkit.org/show_bug.cgi?id=32882
+
+        Covered by existing tests along with the new assertions.
+
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle): Make sure that m_inStyleRecalc is
+        already false by the time post-attach callbacks are done so that
+        layout triggered inside those callbacks can work properly.
+
+        * editing/SelectionController.cpp:
+        (WebCore::SelectionController::layout): Added code to trigger a
+        layout when it's needed.
+        (WebCore::SelectionController::recomputeCaretRect): Removed unneeded
+        code to do nothing when FrameView is 0. Added an assertion that layout
+        is not needed at the time the function is called.
+        (WebCore::SelectionController::invalidateCaretRect): Added code to
+        trigger a layout when it's needed.
+        (WebCore::SelectionController::paintCaret): Added an assertion that
+        layout is not needed at the time the function is called.
+
+        * html/HTMLFormControlElement.cpp:
+        (WebCore::shouldAutofocus): Added. Helper function that expresses
+        the rule for which form control elements should auto-focus.
+        (WebCore::focusPostAttach): Added. Called post-attach to focus an
+        element if we discover it should be focused during attach.
+        (WebCore::HTMLFormControlElement::attach): Refactored code for
+        which elements need auto-focus into a separate function. Instead
+        of focusing right away, use the focusPostAttach function to focus
+        after attach is done. Also added calls to suspendPostAttachCallbacks
+        and resumePostAttachCallbacks so post-attach callbacks happen late
+        enough. Before, they could run inside the base attach function.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::attach): Added calls to
+        suspendPostAttachCallbacks and resumePostAttachCallbacks so
+        post-attach callbacks happen late enough
+
+        * page/Frame.cpp:
+        (WebCore::Frame::revealSelection): Added code to trigger a layout
+        when it's needed.
+
 2010-01-04  Gavin Barraclough <barraclough@apple.com>
 

trunk/WebCore/dom/Document.cpp

@@ -1295 +1295 @@
     unscheduleStyleRecalc();
 
+    m_inStyleRecalc = false;
+
     if (view())
         view()->resumeScheduledEvents();
     RenderWidget::resumeWidgetHierarchyUpdates();
     resumePostAttachCallbacks();
-    m_inStyleRecalc = false;
 
     // If we wanted to call implicitClose() during recalcStyle, do so now that we're finished.

trunk/WebCore/editing/SelectionController.cpp

@@ -820 +820 @@
     }
 
-    m_selection.start().node()->document()->updateStyleIfNeeded();
-    
     m_caretRect = IntRect();
         
     if (isCaret()) {
+        Document* document = m_selection.start().node()->document();
+
+        document->updateStyleIfNeeded();
+        if (FrameView* view = document->view()) {
+            if (view->needsLayout())
+                view->layout();
+        }
+
         VisiblePosition pos(m_selection.start(), m_selection.affinity());
         if (pos.isNotNull()) {
@@ -921 +927 @@
         return false;
         
-    FrameView* v = m_frame->document()->view();
-    if (!v)
-        return false;
+    ASSERT(!m_frame->view() || !m_frame->view()->needsLayout());
 
     if (!m_needsLayout)
@@ -959 +963 @@
         return;
 
-    Document* d = m_selection.start().node()->document();
+    Document* document = m_selection.start().node()->document();
+
+    if (FrameView* frameView = document->view()) {
+        if (frameView->needsLayout())
+            frameView->layout();
+    }
 
     // recomputeCaretRect will always return false for the drag caret,
@@ -979 +988 @@
 
     if (!caretRectChanged) {
-        if (RenderView* view = toRenderView(d->renderer()))
+        if (RenderView* view = toRenderView(document->renderer()))
             view->repaintRectangleInViewAndCompositedLayers(caretRepaintRect(), false);
     }
@@ -986 +995 @@
 void SelectionController::paintCaret(GraphicsContext* p, int tx, int ty, const IntRect& clipRect)
 {
-    if (! m_selection.isCaret())
+    ASSERT(!m_frame || !m_frame->view() || !m_frame->view()->needsLayout());
+
+    if (!isCaret())
         return;
 

trunk/WebCore/html/HTMLFormControlElement.cpp

@@ -3 +3 @@
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
- * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
@@ -117 +117 @@
 }
 
+static bool shouldAutofocus(HTMLFormControlElement* element)
+{
+    if (!element->autofocus())
+        return false;
+    if (!element->renderer())
+        return false;
+    if (element->document()->ignoreAutofocus())
+        return false;
+    if (element->isReadOnlyFormControl())
+        return false;
+
+    // FIXME: Should this set of hasTagName checks be replaced by a
+    // virtual member function?
+    if (element->hasTagName(inputTag))
+        return !static_cast<HTMLInputElement*>(element)->isInputTypeHidden();
+    if (element->hasTagName(selectTag))
+        return true;
+    if (element->hasTagName(buttonTag))
+        return true;
+    if (element->hasTagName(textareaTag))
+        return true;
+
+    return false;
+}
+
+static void focusPostAttach(Node* element)
+{
+    static_cast<Element*>(element)->focus();
+    element->deref();
+}
+
 void HTMLFormControlElement::attach()
 {
     ASSERT(!attached());
+
+    suspendPostAttachCallbacks();
 
     HTMLElement::attach();
@@ -128 +161 @@
     if (renderer())
         renderer()->updateFromElement();
-        
-    // Focus the element if it should honour its autofocus attribute.
-    // We have to determine if the element is a TextArea/Input/Button/Select,
-    // if input type hidden ignore autofocus. So if disabled or readonly.
-    bool isInputTypeHidden = false;
-    if (hasTagName(inputTag))
-        isInputTypeHidden = static_cast<HTMLInputElement*>(this)->isInputTypeHidden();
-
-    if (autofocus() && renderer() && !document()->ignoreAutofocus() && !isReadOnlyFormControl() &&
-            ((hasTagName(inputTag) && !isInputTypeHidden) || hasTagName(selectTag) ||
-              hasTagName(buttonTag) || hasTagName(textareaTag)))
-         focus();
+
+    if (shouldAutofocus(this)) {
+        ref();
+        queuePostAttachCallback(focusPostAttach, this);
+    }
+
+    resumePostAttachCallbacks();
 }
 

trunk/WebCore/html/HTMLInputElement.cpp

@@ -3 +3 @@
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
- * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2007 Samuel Weinig (sam@webkit.org)
@@ -988 +988 @@
 void HTMLInputElement::attach()
 {
+    suspendPostAttachCallbacks();
+
     if (!m_inited) {
         if (!m_haveType)
@@ -1010 +1012 @@
         }
     }
+
+    resumePostAttachCallbacks();
 }
 

trunk/WebCore/page/Frame.cpp

@@ -1352 +1352 @@
 void Frame::revealSelection(const ScrollAlignment& alignment, bool revealExtent)
 {
+    if (view()->needsLayout())
+        view()->layout();
+
     IntRect rect;