Safari's Speedometer score massively regresses when accessibility is enabled
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 02:54:00 +0000 (02:54 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 02:54:00 +0000 (02:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173912

Reviewed by Chris Fleizach.

The bug was caused by HTMLTextFormControlElement::setInnerTextValue triggering a synchronous layout
via constructing VisiblePosition when the accessibility tree is present.

Added AXObjectCache::postTextReplacementNotificationForTextControl which avoids the construction of
VisiblePosition and other means of triggering a synchronous layout. This patch also fixes a subtle bug
that HTMLTextFormControlElement was creating TextMarkerData with axID set to that of the text control
element instead of the root editable element inside its shadow tree even though the typing command uses
axID of the root editable element. While I couldn't find any user-visible behavioral change from this
code change, new code is more self-consistent.

Also added LayoutDisallowedScope which asserts that no synchronous layout happens in setInnerTextValue
so that we don't introduce a new performance regression like this in the future.

No new tests. Existing tests in accessibility directory covers this.

* CMakeLists.txt: Added LayoutDisallowedScope.cpp.
* WebCore.xcodeproj/project.pbxproj: Ditto.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::postTextReplacementNotificationForTextControl): Added.
(WebCore::AXObjectCache::textMarkerDataForVisiblePosition): Modernized. Returns optional<TextMarkerData>
instead of taking TextMarkerData as an out-argument, and returning with axID of 0.
(WebCore::AXObjectCache::textMarkerDataForFirstPositionInTextControl): Added. This specialized version
constructs TextMarkerData for the first position inside the editable region in a text control without
triggering a synchronous layout.

* accessibility/AXObjectCache.h:
(WebCore::TextMarkerData): Initialize each member automatically.
(WebCore::AXObjectCache::postTextReplacementNotificationForTextControl):

* accessibility/ios/AXObjectCacheIOS.mm:
(WebCore::AXObjectCache::postTextReplacementPlatformNotificationForTextControl): Added.

* accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
(+[WebAccessibilityTextMarker textMarkerWithVisiblePosition:cache:]):

* accessibility/mac/AXObjectCacheMac.mm:
(WebCore::addTextMarkerFor): Extracted from textReplacementChangeDictionary. Added a new variant which
takes a text form control instead.
(WebCore::textReplacementChangeDictionary): Templatized this function to either take VisiblePosition
and call textMarkerForVisiblePosition or take HTMLTextFormControlElement and call
textMarkerForFirstPositionInTextControl.
(WebCore::postUserInfoForChanges): Extracted from postTextReplacementPlatformNotification.
(WebCore::AXObjectCache::postTextReplacementPlatformNotification):
(WebCore::AXObjectCache::postTextReplacementPlatformNotificationForTextControl): Added.

* accessibility/mac/WebAccessibilityObjectWrapperBase.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.h:

* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(textMarkerForVisiblePosition):
(-[WebAccessibilityObjectWrapper textMarkerForFirstPositionInTextControl:]): Added.

* dom/Document.cpp:
(WebCore::Document::updateLayout): Assert that LayoutDisallowedScope is not in the stack frame.

* html/HTMLTextFormControlElement.cpp:
(WebCore::HTMLTextFormControlElement::setInnerTextValue): Call postTextReplacementNotificationForTextControl
to avoid triggering a synchronous layout. Also create LayoutDisallowedScope to avoid a similar performance
regression from being introduced in the future in this function. Finally, made innerText a RefPtr for extra
safety since we're using it after updating the DOM tree.

* rendering/LayoutDisallowedScope.cpp: Added.
* rendering/LayoutDisallowedScope.h: Added.
(WebCore::LayoutDisallowedScope::LayoutDisallowedScope):
(WebCore::LayoutDisallowedScope::~LayoutDisallowedScope):
(WebCore::LayoutDisallowedScope::isLayoutAllowed):

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

15 files changed:
Source/WebCore/CMakeLists.txt
Source/WebCore/ChangeLog
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/accessibility/ios/AXObjectCacheIOS.mm
Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm
Source/WebCore/accessibility/mac/AXObjectCacheMac.mm
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperBase.h
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.h
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
Source/WebCore/dom/Document.cpp
Source/WebCore/html/HTMLTextFormControlElement.cpp
Source/WebCore/rendering/LayoutDisallowedScope.cpp [new file with mode: 0644]
Source/WebCore/rendering/LayoutDisallowedScope.h [new file with mode: 0644]

index 2b461b4..f2ec16d 100644 (file)
@@ -2516,6 +2516,7 @@ set(WebCore_SOURCES
     rendering/InlineFlowBox.cpp
     rendering/InlineIterator.cpp
     rendering/InlineTextBox.cpp
+    rendering/LayoutDisallowedScope.cpp
     rendering/LayoutRepainter.cpp
     rendering/LayoutState.cpp
     rendering/OrderIterator.cpp
index 408827b..8f49317 100644 (file)
@@ -1,3 +1,78 @@
+2017-06-28  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Safari's Speedometer score massively regresses when accessibility is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=173912
+
+        Reviewed by Chris Fleizach.
+
+        The bug was caused by HTMLTextFormControlElement::setInnerTextValue triggering a synchronous layout
+        via constructing VisiblePosition when the accessibility tree is present.
+
+        Added AXObjectCache::postTextReplacementNotificationForTextControl which avoids the construction of
+        VisiblePosition and other means of triggering a synchronous layout. This patch also fixes a subtle bug
+        that HTMLTextFormControlElement was creating TextMarkerData with axID set to that of the text control
+        element instead of the root editable element inside its shadow tree even though the typing command uses
+        axID of the root editable element. While I couldn't find any user-visible behavioral change from this
+        code change, new code is more self-consistent.
+
+        Also added LayoutDisallowedScope which asserts that no synchronous layout happens in setInnerTextValue
+        so that we don't introduce a new performance regression like this in the future.
+
+        No new tests. Existing tests in accessibility directory covers this.
+
+        * CMakeLists.txt: Added LayoutDisallowedScope.cpp.
+        * WebCore.xcodeproj/project.pbxproj: Ditto.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::postTextReplacementNotificationForTextControl): Added.
+        (WebCore::AXObjectCache::textMarkerDataForVisiblePosition): Modernized. Returns optional<TextMarkerData>
+        instead of taking TextMarkerData as an out-argument, and returning with axID of 0.
+        (WebCore::AXObjectCache::textMarkerDataForFirstPositionInTextControl): Added. This specialized version
+        constructs TextMarkerData for the first position inside the editable region in a text control without
+        triggering a synchronous layout.
+
+        * accessibility/AXObjectCache.h:
+        (WebCore::TextMarkerData): Initialize each member automatically.
+        (WebCore::AXObjectCache::postTextReplacementNotificationForTextControl):
+
+        * accessibility/ios/AXObjectCacheIOS.mm:
+        (WebCore::AXObjectCache::postTextReplacementPlatformNotificationForTextControl): Added.
+
+        * accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
+        (+[WebAccessibilityTextMarker textMarkerWithVisiblePosition:cache:]):
+
+        * accessibility/mac/AXObjectCacheMac.mm:
+        (WebCore::addTextMarkerFor): Extracted from textReplacementChangeDictionary. Added a new variant which
+        takes a text form control instead.
+        (WebCore::textReplacementChangeDictionary): Templatized this function to either take VisiblePosition
+        and call textMarkerForVisiblePosition or take HTMLTextFormControlElement and call
+        textMarkerForFirstPositionInTextControl.
+        (WebCore::postUserInfoForChanges): Extracted from postTextReplacementPlatformNotification.
+        (WebCore::AXObjectCache::postTextReplacementPlatformNotification): 
+        (WebCore::AXObjectCache::postTextReplacementPlatformNotificationForTextControl): Added.
+
+        * accessibility/mac/WebAccessibilityObjectWrapperBase.h:
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.h:
+
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (textMarkerForVisiblePosition):
+        (-[WebAccessibilityObjectWrapper textMarkerForFirstPositionInTextControl:]): Added.
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateLayout): Assert that LayoutDisallowedScope is not in the stack frame.
+
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::HTMLTextFormControlElement::setInnerTextValue): Call postTextReplacementNotificationForTextControl
+        to avoid triggering a synchronous layout. Also create LayoutDisallowedScope to avoid a similar performance
+        regression from being introduced in the future in this function. Finally, made innerText a RefPtr for extra
+        safety since we're using it after updating the DOM tree.
+
+        * rendering/LayoutDisallowedScope.cpp: Added.
+        * rendering/LayoutDisallowedScope.h: Added.
+        (WebCore::LayoutDisallowedScope::LayoutDisallowedScope):
+        (WebCore::LayoutDisallowedScope::~LayoutDisallowedScope):
+        (WebCore::LayoutDisallowedScope::isLayoutAllowed):
+
 2017-06-27  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [iOS] Cannot italicize or bold text rendered with text styles
index 68862e3..a543587 100644 (file)
                9BC6C21C13CCC97B008E0337 /* HTMLTextFormControlElement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9BC6C21A13CCC97B008E0337 /* HTMLTextFormControlElement.cpp */; };
                9BD0BF9312A42BF50072FD43 /* ScopedEventQueue.h in Headers */ = {isa = PBXBuildFile; fileRef = 9BD0BF9112A42BF50072FD43 /* ScopedEventQueue.h */; };
                9BD0BF9412A42BF50072FD43 /* ScopedEventQueue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9BD0BF9212A42BF50072FD43 /* ScopedEventQueue.cpp */; };
+               9BD1F6821F046310001C9CDD /* LayoutDisallowedScope.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9BD1F6811F046310001C9CDD /* LayoutDisallowedScope.cpp */; };
                9BD4E9161C462872005065BC /* JSCustomElementInterface.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9BD4E9141C462872005065BC /* JSCustomElementInterface.cpp */; };
                9BD4E9171C462872005065BC /* JSCustomElementInterface.h in Headers */ = {isa = PBXBuildFile; fileRef = 9BD4E9151C462872005065BC /* JSCustomElementInterface.h */; };
                9BD4E91A1C462CFC005065BC /* CustomElementRegistry.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9BD4E9181C462CFC005065BC /* CustomElementRegistry.cpp */; };
                9BC6C21A13CCC97B008E0337 /* HTMLTextFormControlElement.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTMLTextFormControlElement.cpp; sourceTree = "<group>"; };
                9BD0BF9112A42BF50072FD43 /* ScopedEventQueue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ScopedEventQueue.h; sourceTree = "<group>"; };
                9BD0BF9212A42BF50072FD43 /* ScopedEventQueue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ScopedEventQueue.cpp; sourceTree = "<group>"; };
+               9BD1F6801F0462B8001C9CDD /* LayoutDisallowedScope.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LayoutDisallowedScope.h; sourceTree = "<group>"; };
+               9BD1F6811F046310001C9CDD /* LayoutDisallowedScope.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LayoutDisallowedScope.cpp; sourceTree = "<group>"; };
                9BD4E9141C462872005065BC /* JSCustomElementInterface.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSCustomElementInterface.cpp; sourceTree = "<group>"; };
                9BD4E9151C462872005065BC /* JSCustomElementInterface.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSCustomElementInterface.h; sourceTree = "<group>"; };
                9BD4E9181C462CFC005065BC /* CustomElementRegistry.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CustomElementRegistry.cpp; sourceTree = "<group>"; };
                                BCEA481A097D93020094C9E4 /* InlineTextBox.cpp */,
                                BCEA481B097D93020094C9E4 /* InlineTextBox.h */,
                                580371631A66F1D300BAF519 /* LayerFragment.h */,
+                               9BD1F6811F046310001C9CDD /* LayoutDisallowedScope.cpp */,
+                               9BD1F6801F0462B8001C9CDD /* LayoutDisallowedScope.h */,
                                A120ACA113F9984600FE4AC7 /* LayoutRepainter.cpp */,
                                A120ACA013F9983700FE4AC7 /* LayoutRepainter.h */,
                                2D9066040BE141D400956998 /* LayoutState.cpp */,
                                97AABD1814FA09D5007457AE /* ThreadableWebSocketChannelClientWrapper.cpp in Sources */,
                                51DF6D800B92A18E00C2DC85 /* ThreadCheck.mm in Sources */,
                                0F6383DD18615B29003E5DB5 /* ThreadedScrollingTree.cpp in Sources */,
+                               9BD1F6821F046310001C9CDD /* LayoutDisallowedScope.cpp in Sources */,
                                E1FF57A60F01256B00891EBB /* ThreadGlobalData.cpp in Sources */,
                                185BCF280F3279CE000EA262 /* ThreadTimers.cpp in Sources */,
                                7AA3A699194A64E7001CBD24 /* TileController.cpp in Sources */,
index 155d353..2b75b45 100644 (file)
@@ -74,6 +74,7 @@
 #include "HTMLLabelElement.h"
 #include "HTMLMeterElement.h"
 #include "HTMLNames.h"
+#include "HTMLTextFormControlElement.h"
 #include "InlineElementBox.h"
 #include "MathMLElement.h"
 #include "Page.h"
@@ -93,6 +94,7 @@
 #include "SVGElement.h"
 #include "ScrollView.h"
 #include "TextBoundaries.h"
+#include "TextControlInnerElements.h"
 #include "TextIterator.h"
 #include <wtf/DataLog.h>
 
@@ -1293,6 +1295,25 @@ void AXObjectCache::postTextReplacementNotification(Node* node, AXTextEditType d
 #endif
 }
 
+void AXObjectCache::postTextReplacementNotificationForTextControl(HTMLTextFormControlElement& textControl, const String& deletedText, const String& insertedText)
+{
+    stopCachingComputedObjectAttributes();
+
+    AccessibilityObject* object = getOrCreate(&textControl);
+#if PLATFORM(COCOA)
+    if (object) {
+        if (enqueuePasswordValueChangeNotification(object))
+            return;
+        object = object->observableObject();
+    }
+
+    postTextReplacementPlatformNotificationForTextControl(object, deletedText, insertedText, textControl);
+#else
+    nodeTextChangePlatformNotification(object, textChangeForEditType(AXTextEditTypeDelete), 0, deletedText);
+    nodeTextChangePlatformNotification(object, textChangeForEditType(AXTextEditTypeInsert), 0, insertedText);
+#endif
+}
+
 bool AXObjectCache::enqueuePasswordValueChangeNotification(AccessibilityObject* object)
 {
     if (!isPasswordFieldOrContainedByPasswordField(object))
@@ -2083,45 +2104,73 @@ AccessibilityObject* AXObjectCache::accessibilityObjectForTextMarkerData(TextMar
     return this->getOrCreate(domNode);
 }
 
-void AXObjectCache::textMarkerDataForVisiblePosition(TextMarkerData& textMarkerData, const VisiblePosition& visiblePos)
+std::optional<TextMarkerData> AXObjectCache::textMarkerDataForVisiblePosition(const VisiblePosition& visiblePos)
 {
-    // This memory must be bzero'd so instances of TextMarkerData can be tested for byte-equivalence.
-    // This also allows callers to check for failure by looking at textMarkerData upon return.
-    memset(&textMarkerData, 0, sizeof(TextMarkerData));
-    
     if (visiblePos.isNull())
-        return;
-    
+        return std::nullopt;
+
     Position deepPos = visiblePos.deepEquivalent();
     Node* domNode = deepPos.deprecatedNode();
     ASSERT(domNode);
     if (!domNode)
-        return;
-    
+        return std::nullopt;
+
     if (is<HTMLInputElement>(*domNode) && downcast<HTMLInputElement>(*domNode).isPasswordField())
-        return;
-    
+        return std::nullopt;
+
     // If the visible position has an anchor type referring to a node other than the anchored node, we should
     // set the text marker data with CharacterOffset so that the offset will correspond to the node.
     CharacterOffset characterOffset = characterOffsetFromVisiblePosition(visiblePos);
     if (deepPos.anchorType() == Position::PositionIsAfterAnchor || deepPos.anchorType() == Position::PositionIsAfterChildren) {
+        TextMarkerData textMarkerData;
         textMarkerDataForCharacterOffset(textMarkerData, characterOffset);
-        return;
+        return textMarkerData;
     }
-    
+
     // find or create an accessibility object for this node
     AXObjectCache* cache = domNode->document().axObjectCache();
     RefPtr<AccessibilityObject> obj = cache->getOrCreate(domNode);
-    
+
+    TextMarkerData textMarkerData;
     textMarkerData.axID = obj.get()->axObjectID();
     textMarkerData.node = domNode;
     textMarkerData.offset = deepPos.deprecatedEditingOffset();
     textMarkerData.affinity = visiblePos.affinity();
-    
+
     textMarkerData.characterOffset = characterOffset.offset;
     textMarkerData.characterStartIndex = characterOffset.startIndex;
-    
+
     cache->setNodeInUse(domNode);
+
+    return textMarkerData;
+}
+
+// This function exits as a performance optimization to avoid a synchronous layout.
+std::optional<TextMarkerData> AXObjectCache::textMarkerDataForFirstPositionInTextControl(HTMLTextFormControlElement& textControl)
+{
+    TextControlInnerTextElement* innerTextElement = textControl.innerTextElement();
+    if (!innerTextElement)
+        return std::nullopt;
+
+    if (is<HTMLInputElement>(textControl) && downcast<HTMLInputElement>(textControl).isPasswordField())
+        return std::nullopt;
+
+    Position firstPosition = firstPositionInNode(innerTextElement);
+    Node* firstChild = innerTextElement->firstChild();
+    if (!firstChild)
+        firstChild = innerTextElement;
+    ContainerNode* editingHost = highestEditableRoot(firstPosition);
+
+    AXObjectCache* cache = textControl.document().axObjectCache();
+    RefPtr<AccessibilityObject> obj = cache->getOrCreate(editingHost);
+
+    TextMarkerData textMarkerData;
+    textMarkerData.axID = obj.get()->axObjectID();
+    textMarkerData.node = firstChild;
+
+    cache->setNodeInUse(&textControl);
+
+    return textMarkerData;
 }
 
 CharacterOffset AXObjectCache::nextCharacterOffset(const CharacterOffset& characterOffset, bool ignoreNextNodeStart)
index 1c5954a..13ff66e 100644 (file)
@@ -41,6 +41,7 @@ namespace WebCore {
 
 class Document;
 class HTMLAreaElement;
+class HTMLTextFormControlElement;
 class Node;
 class Page;
 class RenderBlock;
@@ -51,13 +52,13 @@ class VisiblePosition;
 class Widget;
 
 struct TextMarkerData {
-    AXID axID;
-    Node* node;
-    int offset;
-    int characterStartIndex;
-    int characterOffset;
-    bool ignored;
-    EAffinity affinity;
+    AXID axID { 0 };
+    Node* node { nullptr };
+    int offset { 0 };
+    int characterStartIndex { 0 };
+    int characterOffset { 0 };
+    bool ignored { false };
+    EAffinity affinity { DOWNSTREAM };
 };
 
 struct CharacterOffset {
@@ -212,7 +213,8 @@ public:
     AccessibilityObject* objectFromAXID(AXID id) const { return m_objects.get(id); }
 
     // Text marker utilities.
-    void textMarkerDataForVisiblePosition(TextMarkerData&, const VisiblePosition&);
+    std::optional<TextMarkerData> textMarkerDataForVisiblePosition(const VisiblePosition&);
+    std::optional<TextMarkerData> textMarkerDataForFirstPositionInTextControl(HTMLTextFormControlElement&);
     void textMarkerDataForCharacterOffset(TextMarkerData&, const CharacterOffset&);
     void textMarkerDataForNextCharacterOffset(TextMarkerData&, const CharacterOffset&);
     void textMarkerDataForPreviousCharacterOffset(TextMarkerData&, const CharacterOffset&);
@@ -300,6 +302,7 @@ public:
 
     void postTextStateChangeNotification(Node*, AXTextEditType, const String&, const VisiblePosition&);
     void postTextReplacementNotification(Node*, AXTextEditType deletionType, const String& deletedText, AXTextEditType insertionType, const String& insertedText, const VisiblePosition&);
+    void postTextReplacementNotificationForTextControl(HTMLTextFormControlElement&, const String& deletedText, const String& insertedText);
     void postTextStateChangeNotification(Node*, const AXTextStateChangeIntent&, const VisibleSelection&);
     void postTextStateChangeNotification(const Position&, const AXTextStateChangeIntent&, const VisibleSelection&);
     void postLiveRegionChangeNotification(AccessibilityObject*);
@@ -337,6 +340,7 @@ protected:
 #if PLATFORM(COCOA)
     void postTextStateChangePlatformNotification(AccessibilityObject*, const AXTextStateChangeIntent&, const VisibleSelection&);
     void postTextStateChangePlatformNotification(AccessibilityObject*, AXTextEditType, const String&, const VisiblePosition&);
+    void postTextReplacementPlatformNotificationForTextControl(AccessibilityObject*, const String& deletedText, const String& insertedText, HTMLTextFormControlElement&);
     void postTextReplacementPlatformNotification(AccessibilityObject*, AXTextEditType, const String&, AXTextEditType, const String&, const VisiblePosition&);
 #else
     static AXTextChange textChangeForEditType(AXTextEditType);
@@ -499,6 +503,7 @@ inline void AXObjectCache::handleScrolledToAnchor(const Node*) { }
 inline void AXObjectCache::postTextStateChangeNotification(Node*, const AXTextStateChangeIntent&, const VisibleSelection&) { }
 inline void AXObjectCache::postTextStateChangeNotification(Node*, AXTextEditType, const String&, const VisiblePosition&) { }
 inline void AXObjectCache::postTextReplacementNotification(Node*, AXTextEditType, const String&, AXTextEditType, const String&, const VisiblePosition&) { }
+inline void AXObjectCache::postTextReplacementNotificationForTextControl(HTMLTextFormControl&, const String&, const String&) { }
 inline void AXObjectCache::postNotification(AccessibilityObject*, Document*, AXNotification, PostTarget, PostType) { }
 inline void AXObjectCache::postNotification(RenderObject*, AXNotification, PostTarget, PostType) { }
 inline void AXObjectCache::postNotification(Node*, AXNotification, PostTarget, PostType) { }
index 5b35fca..da65632 100644 (file)
@@ -113,6 +113,11 @@ void AXObjectCache::postTextReplacementPlatformNotification(AccessibilityObject*
     postPlatformNotification(object, AXValueChanged);
 }
 
+void AXObjectCache::postTextReplacementPlatformNotificationForTextControl(AccessibilityObject* object, const String&, const String&, HTMLTextFormControlElement&)
+{
+    postPlatformNotification(object, AXValueChanged);
+}
+
 void AXObjectCache::frameLoadingEventPlatformNotification(AccessibilityObject* axFrameObject, AXLoadingEvent loadingEvent)
 {
     if (!axFrameObject)
index d66173e..54f5006 100644 (file)
@@ -180,10 +180,10 @@ static AccessibilityObjectWrapper* AccessibilityUnignoredAncestor(AccessibilityO
 
 + (WebAccessibilityTextMarker *)textMarkerWithVisiblePosition:(VisiblePosition&)visiblePos cache:(AXObjectCache*)cache
 {
-    TextMarkerData textMarkerData;
-    cache->textMarkerDataForVisiblePosition(textMarkerData, visiblePos);
-    
-    return [[[WebAccessibilityTextMarker alloc] initWithTextMarker:&textMarkerData cache:cache] autorelease];
+    auto textMarkerData = cache->textMarkerDataForVisiblePosition(visiblePos);
+    if (!textMarkerData)
+        return nil;
+    return [[[WebAccessibilityTextMarker alloc] initWithTextMarker:&textMarkerData.value() cache:cache] autorelease];
 }
 
 + (WebAccessibilityTextMarker *)textMarkerWithCharacterOffset:(CharacterOffset&)characterOffset cache:(AXObjectCache*)cache
index 7a2e401..2379751 100644 (file)
@@ -407,7 +407,22 @@ void AXObjectCache::postTextStateChangePlatformNotification(AccessibilityObject*
     [userInfo release];
 }
 
-static NSDictionary *textReplacementChangeDictionary(AccessibilityObject* object, AXTextEditType type, const String& string, const VisiblePosition& position)
+static void addTextMarkerFor(NSMutableDictionary* change, AccessibilityObject& object, const VisiblePosition& position)
+{
+    if (position.isNull())
+        return;
+    if (id textMarker = [object.wrapper() textMarkerForVisiblePosition:position])
+        [change setObject:textMarker forKey:NSAccessibilityTextChangeValueStartMarker];
+}
+
+static void addTextMarkerFor(NSMutableDictionary* change, AccessibilityObject& object, HTMLTextFormControlElement& textControl)
+{
+    if (id textMarker = [object.wrapper() textMarkerForFirstPositionInTextControl:textControl])
+        [change setObject:textMarker forKey:NSAccessibilityTextChangeValueStartMarker];
+}
+
+template <typename TextMarkerTargetType>
+static NSDictionary *textReplacementChangeDictionary(AccessibilityObject& object, AXTextEditType type, const String& string, TextMarkerTargetType& markerTarget)
 {
     NSString *text = (NSString *)string;
     NSUInteger length = [text length];
@@ -420,10 +435,7 @@ static NSDictionary *textReplacementChangeDictionary(AccessibilityObject* object
         text = [text substringToIndex:AXValueChangeTruncationLength];
     }
     [change setObject:text forKey:NSAccessibilityTextChangeValue];
-    if (position.isNotNull()) {
-        if (id textMarker = [object->wrapper() textMarkerForVisiblePosition:position])
-            [change setObject:textMarker forKey:NSAccessibilityTextChangeValueStartMarker];
-    }
+    addTextMarkerFor(change, object, markerTarget);
     return [change autorelease];
 }
 
@@ -435,6 +447,23 @@ void AXObjectCache::postTextStateChangePlatformNotification(AccessibilityObject*
     postTextReplacementPlatformNotification(object, AXTextEditTypeUnknown, emptyString(), type, text, position);
 }
 
+static void postUserInfoForChanges(AccessibilityObject& rootWebArea, AccessibilityObject& object, NSMutableArray* changes)
+{
+    NSMutableDictionary *userInfo = [[NSMutableDictionary alloc] initWithCapacity:4];
+    [userInfo setObject:@(platformChangeTypeForWebCoreChangeType(AXTextStateChangeTypeEdit)) forKey:NSAccessibilityTextStateChangeTypeKey];
+    if (changes.count)
+        [userInfo setObject:changes forKey:NSAccessibilityTextChangeValues];
+
+    if (id wrapper = object.wrapper())
+        [userInfo setObject:wrapper forKey:NSAccessibilityTextChangeElement];
+
+    AXPostNotificationWithUserInfo(rootWebArea.wrapper(), NSAccessibilityValueChangedNotification, userInfo);
+    if (rootWebArea.wrapper() != object.wrapper())
+        AXPostNotificationWithUserInfo(object.wrapper(), NSAccessibilityValueChangedNotification, userInfo);
+
+    [userInfo release];
+}
+
 void AXObjectCache::postTextReplacementPlatformNotification(AccessibilityObject* object, AXTextEditType deletionType, const String& deletedText, AXTextEditType insertionType, const String& insertedText, const VisiblePosition& position)
 {
     if (!object)
@@ -443,26 +472,30 @@ void AXObjectCache::postTextReplacementPlatformNotification(AccessibilityObject*
     if (!object)
         return;
 
-    NSMutableDictionary *userInfo = [[NSMutableDictionary alloc] initWithCapacity:4];
-    [userInfo setObject:@(platformChangeTypeForWebCoreChangeType(AXTextStateChangeTypeEdit)) forKey:NSAccessibilityTextStateChangeTypeKey];
-
     NSMutableArray *changes = [[NSMutableArray alloc] initWithCapacity:2];
-    if (NSDictionary *change = textReplacementChangeDictionary(object, deletionType, deletedText, position))
+    if (NSDictionary *change = textReplacementChangeDictionary(*object, deletionType, deletedText, position))
         [changes addObject:change];
-    if (NSDictionary *change = textReplacementChangeDictionary(object, insertionType, insertedText, position))
+    if (NSDictionary *change = textReplacementChangeDictionary(*object, insertionType, insertedText, position))
         [changes addObject:change];
-    if (changes.count)
-        [userInfo setObject:changes forKey:NSAccessibilityTextChangeValues];
+    postUserInfoForChanges(*rootWebArea(), *object, changes);
     [changes release];
+}
 
-    if (id wrapper = object->wrapper())
-        [userInfo setObject:wrapper forKey:NSAccessibilityTextChangeElement];
+void AXObjectCache::postTextReplacementPlatformNotificationForTextControl(AccessibilityObject* object, const String& deletedText, const String& insertedText, HTMLTextFormControlElement& textControl)
+{
+    if (!object)
+        object = rootWebArea();
 
-    AXPostNotificationWithUserInfo(rootWebArea()->wrapper(), NSAccessibilityValueChangedNotification, userInfo);
-    if (rootWebArea()->wrapper() != object->wrapper())
-        AXPostNotificationWithUserInfo(object->wrapper(), NSAccessibilityValueChangedNotification, userInfo);
+    if (!object)
+        return;
 
-    [userInfo release];
+    NSMutableArray *changes = [[NSMutableArray alloc] initWithCapacity:2];
+    if (NSDictionary *change = textReplacementChangeDictionary(*object, AXTextEditTypeDelete, deletedText, textControl))
+        [changes addObject:change];
+    if (NSDictionary *change = textReplacementChangeDictionary(*object, AXTextEditTypeInsert, insertedText, textControl))
+        [changes addObject:change];
+    postUserInfoForChanges(*rootWebArea(), *object, changes);
+    [changes release];
 }
 
 void AXObjectCache::frameLoadingEventPlatformNotification(AccessibilityObject* axFrameObject, AXLoadingEvent loadingEvent)
index 6ac2ec0..787149b 100644 (file)
@@ -37,6 +37,7 @@ class AccessibilityObject;
 struct AccessibilitySearchCriteria;
 class IntRect;
 class FloatPoint;
+class HTMLTextFormControlElement;
 class Path;
 class VisiblePosition;
 }
index 2decdd0..51fb519 100644 (file)
@@ -34,6 +34,7 @@
 
 - (id)textMarkerRangeFromVisiblePositions:(const WebCore::VisiblePosition&)startPosition endPosition:(const WebCore::VisiblePosition&)endPosition;
 - (id)textMarkerForVisiblePosition:(const WebCore::VisiblePosition&)visiblePos;
+- (id)textMarkerForFirstPositionInTextControl:(WebCore::HTMLTextFormControlElement&)textControl;
 
 // When a plugin uses a WebKit control to act as a surrogate view (e.g. PDF use WebKit to create text fields).
 - (id)associatedPluginParent;
index dd42ef9..2a3be16 100644 (file)
@@ -681,13 +681,12 @@ static CharacterOffset characterOffsetForTextMarker(AXObjectCache* cache, CFType
 static id textMarkerForVisiblePosition(AXObjectCache* cache, const VisiblePosition& visiblePos)
 {
     ASSERT(cache);
-    
-    TextMarkerData textMarkerData;
-    cache->textMarkerDataForVisiblePosition(textMarkerData, visiblePos);
-    if (!textMarkerData.axID)
+
+    auto textMarkerData = cache->textMarkerDataForVisiblePosition(visiblePos);
+    if (!textMarkerData)
         return nil;
-    
-    return CFBridgingRelease(wkCreateAXTextMarker(&textMarkerData, sizeof(textMarkerData)));
+
+    return CFBridgingRelease(wkCreateAXTextMarker(&textMarkerData.value(), sizeof(textMarkerData.value())));
 }
 
 - (id)textMarkerForVisiblePosition:(const VisiblePosition &)visiblePos
@@ -695,6 +694,19 @@ static id textMarkerForVisiblePosition(AXObjectCache* cache, const VisiblePositi
     return textMarkerForVisiblePosition(m_object->axObjectCache(), visiblePos);
 }
 
+- (id)textMarkerForFirstPositionInTextControl:(HTMLTextFormControlElement &)textControl
+{
+    auto *cache = m_object->axObjectCache();
+    if (!cache)
+        return nil;
+
+    auto textMarkerData = cache->textMarkerDataForFirstPositionInTextControl(textControl);
+    if (!textMarkerData)
+        return nil;
+
+    return CFBridgingRelease(wkCreateAXTextMarker(&textMarkerData.value(), sizeof(textMarkerData.value())));
+}
+
 static VisiblePosition visiblePositionForTextMarker(AXObjectCache* cache, CFTypeRef textMarker)
 {
     ASSERT(cache);
index c4baada..af7683e 100644 (file)
 #include "JSLazyEventListener.h"
 #include "KeyboardEvent.h"
 #include "Language.h"
+#include "LayoutDisallowedScope.h"
 #include "LoaderStrategy.h"
 #include "Logging.h"
 #include "MainFrame.h"
@@ -1914,6 +1915,7 @@ void Document::updateStyleIfNeeded()
 
 void Document::updateLayout()
 {
+    ASSERT(LayoutDisallowedScope::isLayoutAllowed());
     ASSERT(isMainThread());
 
     FrameView* frameView = view();
index d04c390..96a9153 100644 (file)
@@ -40,6 +40,7 @@
 #include "HTMLInputElement.h"
 #include "HTMLNames.h"
 #include "HTMLParserIdioms.h"
+#include "LayoutDisallowedScope.h"
 #include "Logging.h"
 #include "NoEventDispatchAssertion.h"
 #include "NodeTraversal.h"
@@ -549,7 +550,8 @@ static String innerTextValueFrom(TextControlInnerTextElement& innerText)
 
 void HTMLTextFormControlElement::setInnerTextValue(const String& value)
 {
-    TextControlInnerTextElement* innerText = innerTextElement();
+    LayoutDisallowedScope layoutDisallowedScope(LayoutDisallowedScope::Reason::PerformanceOptimization);
+    RefPtr<TextControlInnerTextElement> innerText = innerTextElement();
     if (!innerText)
         return;
 
@@ -577,7 +579,7 @@ void HTMLTextFormControlElement::setInnerTextValue(const String& value)
 #if HAVE(ACCESSIBILITY) && PLATFORM(COCOA)
         if (textIsChanged && renderer()) {
             if (AXObjectCache* cache = document().existingAXObjectCache())
-                cache->postTextReplacementNotification(this, AXTextEditTypeDelete, previousValue, AXTextEditTypeInsert, value, VisiblePosition(Position(this, Position::PositionIsBeforeAnchor)));
+                cache->postTextReplacementNotificationForTextControl(*this, previousValue, value);
         }
 #endif
     }
diff --git a/Source/WebCore/rendering/LayoutDisallowedScope.cpp b/Source/WebCore/rendering/LayoutDisallowedScope.cpp
new file mode 100644 (file)
index 0000000..f7c9b3c
--- /dev/null
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "LayoutDisallowedScope.h"
+
+namespace WebCore {
+
+#if !ASSERT_DISABLED
+
+LayoutDisallowedScope* LayoutDisallowedScope::s_currentAssertion = nullptr;
+
+#endif
+
+}
diff --git a/Source/WebCore/rendering/LayoutDisallowedScope.h b/Source/WebCore/rendering/LayoutDisallowedScope.h
new file mode 100644 (file)
index 0000000..bd162f5
--- /dev/null
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+namespace WebCore {
+
+#if !ASSERT_DISABLED
+
+class LayoutDisallowedScope {
+public:
+    enum class Reason { PerformanceOptimization };
+    LayoutDisallowedScope(Reason)
+        : m_previousAssertion(s_currentAssertion)
+    {
+        s_currentAssertion = this;
+    }
+
+    ~LayoutDisallowedScope()
+    {
+        s_currentAssertion = m_previousAssertion;
+    }
+
+    static bool isLayoutAllowed() { return !s_currentAssertion; }
+
+private:
+    LayoutDisallowedScope* m_previousAssertion;
+    static LayoutDisallowedScope* s_currentAssertion;
+};
+
+#else
+
+class LayoutDisallowedScope {
+public:
+    enum class Reason { PerformanceOptimization };
+    LayoutDisallowedScope(Reason) { }
+    static bool isLayoutAllowed() { return true; }
+};
+
+#endif
+
+}