AX: PSON: Going back from apple.com to search results, cannot interact with HTML...
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Feb 2019 17:17:46 +0000 (17:17 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Feb 2019 17:17:46 +0000 (17:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194742

Reviewed by Chris Dumez.

Source/WebCore:

With the new process model, WebProcess hits a case where it tries to send the "page loaded" notification before VoiceOver
had a chance to register for any notifications. This leads to those notifications being dropped (and thus this bug).

This change instead asks the UIProcess to send the notification, which we know VoiceOver has registered for, and can reliably
receive notifications.

It also sends the notification for "load failures," which to the VO users' perspective amounts to the same thing as a successful
page load.

* accessibility/mac/AXObjectCacheMac.mm:
(WebCore::AXObjectCache::frameLoadingEventPlatformNotification):

Source/WebKit:

Re-initialize the accessibility web process tokens when swapping processes.
Send page load notifications from the UIProcess instead of the WebProcess to improve reliability.

* UIProcess/mac/PageClientImplMac.mm:
(WebKit::PageClientImpl::didFinishLoadForMainFrame):
(WebKit::PageClientImpl::didFailLoadForMainFrame):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::reinitializeWebPage):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/gtk/WebPageGtk.cpp:
(WebKit::WebPage::platformReinitialize):
(WebKit::WebPage::platformDetach): Deleted.
(WebKit::WebPage::platformEditorState const): Deleted.
(WebKit::WebPage::updateAccessibilityTree): Deleted.
(WebKit::WebPage::performDefaultBehaviorForKeyEvent): Deleted.
(WebKit::WebPage::platformCanHandleRequest): Deleted.
(WebKit::WebPage::platformUserAgent const): Deleted.
(WebKit::WebPage::getCenterForZoomGesture): Deleted.
(WebKit::WebPage::setInputMethodState): Deleted.
(WebKit::WebPage::collapseSelectionInFrame): Deleted.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformReinitialize):
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::platformReinitialize):
* WebProcess/WebPage/win/WebPageWin.cpp:
(WebKit::WebPage::platformReinitialize):
* WebProcess/WebPage/wpe/WebPageWPE.cpp:
(WebKit::WebPage::platformReinitialize):

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

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/accessibility/mac/AXObjectCacheMac.mm
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/mac/PageClientImplMac.mm
Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/gtk/WebPageGtk.cpp
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp
Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp

index 8da12a9..bec0fd4 100644 (file)
@@ -1,3 +1,22 @@
+2019-02-18  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: PSON: Going back from apple.com to search results, cannot interact with HTML content. Disabling Swap Processes on Cross-Site Navigation resolves the issue.
+        https://bugs.webkit.org/show_bug.cgi?id=194742
+
+        Reviewed by Chris Dumez.
+
+        With the new process model, WebProcess hits a case where it tries to send the "page loaded" notification before VoiceOver
+        had a chance to register for any notifications. This leads to those notifications being dropped (and thus this bug).
+
+        This change instead asks the UIProcess to send the notification, which we know VoiceOver has registered for, and can reliably
+        receive notifications.
+
+        It also sends the notification for "load failures," which to the VO users' perspective amounts to the same thing as a successful
+        page load.
+
+        * accessibility/mac/AXObjectCacheMac.mm:
+        (WebCore::AXObjectCache::frameLoadingEventPlatformNotification):
+
 2019-02-18  Megan Gardner  <megan_gardner@apple.com>
 
         Turn On Smart Delete
index 2d7f13d..1bb5898 100644 (file)
@@ -261,22 +261,27 @@ void AXObjectCache::setShouldRepostNotificationsForTests(bool value)
     axShouldRepostNotificationsForTests = value;
 }
 
-static void AXPostNotificationWithUserInfo(AccessibilityObjectWrapper *object, NSString *notification, id userInfo)
+static void AXPostNotificationWithUserInfo(AccessibilityObjectWrapper *object, NSString *notification, id userInfo, bool skipSystemNotification = false)
 {
     if (id associatedPluginParent = [object associatedPluginParent])
         object = associatedPluginParent;
-    
-    NSAccessibilityPostNotificationWithUserInfo(object, notification, userInfo);
+
     // To simplify monitoring for notifications in tests, repost as a simple NSNotification instead of forcing test infrastucture to setup an IPC client and do all the translation between WebCore types and platform specific IPC types and back
     if (UNLIKELY(axShouldRepostNotificationsForTests))
         [object accessibilityPostedNotification:notification userInfo:userInfo];
+
+    if (skipSystemNotification)
+        return;
+
+    NSAccessibilityPostNotificationWithUserInfo(object, notification, userInfo);
 }
 
 void AXObjectCache::postPlatformNotification(AccessibilityObject* obj, AXNotification notification)
 {
     if (!obj)
         return;
-    
+
+    bool skipSystemNotification = false;
     // Some notifications are unique to Safari and do not have NSAccessibility equivalents.
     NSString *macNotification;
     switch (notification) {
@@ -304,6 +309,11 @@ void AXObjectCache::postPlatformNotification(AccessibilityObject* obj, AXNotific
             break;
         case AXLoadComplete:
             macNotification = @"AXLoadComplete";
+            // Frame loading events are handled by the UIProcess on macOS to improve reliability.
+            // On macOS, before notifications are allowed by AppKit to be sent to clients, you need to have a client (e.g. VoiceOver)
+            // register for that notification. Because these new processes appear before VO has a chance to register, it will often
+            // miss AXLoadComplete notifications. By moving them to the UIProcess, we can eliminate that issue.
+            skipSystemNotification = true;
             break;
         case AXInvalidStatusChanged:
             macNotification = @"AXInvalidStatusChanged";
@@ -367,7 +377,7 @@ void AXObjectCache::postPlatformNotification(AccessibilityObject* obj, AXNotific
     ASSERT([obj->wrapper() accessibilityIsIgnored] || true);
     ALLOW_DEPRECATED_DECLARATIONS_END
 
-    AXPostNotificationWithUserInfo(obj->wrapper(), macNotification, nil);
+    AXPostNotificationWithUserInfo(obj->wrapper(), macNotification, nil, skipSystemNotification);
 }
 
 void AXObjectCache::postTextStateChangePlatformNotification(AccessibilityObject* object, const AXTextStateChangeIntent& intent, const VisibleSelection& selection)
index d0b91b0..6d2c570 100644 (file)
@@ -1,3 +1,39 @@
+2019-02-18  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: PSON: Going back from apple.com to search results, cannot interact with HTML content. Disabling Swap Processes on Cross-Site Navigation resolves the issue.
+        https://bugs.webkit.org/show_bug.cgi?id=194742
+
+        Reviewed by Chris Dumez.
+
+        Re-initialize the accessibility web process tokens when swapping processes.
+        Send page load notifications from the UIProcess instead of the WebProcess to improve reliability.
+
+        * UIProcess/mac/PageClientImplMac.mm:
+        (WebKit::PageClientImpl::didFinishLoadForMainFrame):
+        (WebKit::PageClientImpl::didFailLoadForMainFrame):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::reinitializeWebPage):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/gtk/WebPageGtk.cpp:
+        (WebKit::WebPage::platformReinitialize):
+        (WebKit::WebPage::platformDetach): Deleted.
+        (WebKit::WebPage::platformEditorState const): Deleted.
+        (WebKit::WebPage::updateAccessibilityTree): Deleted.
+        (WebKit::WebPage::performDefaultBehaviorForKeyEvent): Deleted.
+        (WebKit::WebPage::platformCanHandleRequest): Deleted.
+        (WebKit::WebPage::platformUserAgent const): Deleted.
+        (WebKit::WebPage::getCenterForZoomGesture): Deleted.
+        (WebKit::WebPage::setInputMethodState): Deleted.
+        (WebKit::WebPage::collapseSelectionInFrame): Deleted.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::platformReinitialize):
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::platformReinitialize):
+        * WebProcess/WebPage/win/WebPageWin.cpp:
+        (WebKit::WebPage::platformReinitialize):
+        * WebProcess/WebPage/wpe/WebPageWPE.cpp:
+        (WebKit::WebPage::platformReinitialize):
+
 2019-02-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Crash while filling selection data during drag and drop
index de6f2d5..38573df 100644 (file)
@@ -89,6 +89,8 @@
 #include <WebCore/WebMediaSessionManager.h>
 #endif
 
+static NSString * const kAXLoadCompleteNotification = @"AXLoadComplete";
+
 @interface NSApplication (WebNSApplicationDetails)
 - (NSCursor *)_cursorRectCursor;
 @end
@@ -840,12 +842,16 @@ void PageClientImpl::didFinishLoadForMainFrame()
 {
     if (auto gestureController = m_impl->gestureController())
         gestureController->didFinishLoadForMainFrame();
+
+    NSAccessibilityPostNotification(NSAccessibilityUnignoredAncestor(m_view), kAXLoadCompleteNotification);
 }
 
 void PageClientImpl::didFailLoadForMainFrame()
 {
     if (auto gestureController = m_impl->gestureController())
         gestureController->didFailLoadForMainFrame();
+
+    NSAccessibilityPostNotification(NSAccessibilityUnignoredAncestor(m_view), kAXLoadCompleteNotification);
 }
 
 void PageClientImpl::didSameDocumentNavigationForMainFrame(SameDocumentNavigationType type)
index 36c9b35..163c5ca 100644 (file)
@@ -183,6 +183,12 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame& frame, Range& ra
     return dictionaryPopupInfo;
 }
 
+void WebPage::accessibilityTransferRemoteToken(RetainPtr<NSData> remoteToken)
+{
+    IPC::DataReference dataToken = IPC::DataReference(reinterpret_cast<const uint8_t*>([remoteToken bytes]), [remoteToken length]);
+    send(Messages::WebPageProxy::RegisterWebProcessAccessibilityToken(dataToken));
+}
+
 } // namespace WebKit
 
 #endif // PLATFORM(COCOA)
index c932b51..085d82a 100644 (file)
@@ -714,6 +714,8 @@ void WebPage::reinitializeWebPage(WebPageCreationParameters&& parameters)
         setActivityState(parameters.activityState, ActivityStateChangeAsynchronous, Vector<CallbackID>());
     if (m_layerHostingMode != parameters.layerHostingMode)
         setLayerHostingMode(parameters.layerHostingMode);
+
+    platformReinitialize();
 }
 
 void WebPage::updateThrottleState()
index ba949ba..1dd2bf7 100644 (file)
@@ -1159,6 +1159,7 @@ private:
     uint64_t messageSenderDestinationID() const override;
 
     void platformInitialize();
+    void platformReinitialize();
     void platformDetach();
     void platformEditorState(WebCore::Frame&, EditorState& result, IncludePostLayoutDataHint) const;
     void sendEditorStateUpdate();
@@ -1455,6 +1456,8 @@ private:
 
 #if PLATFORM(COCOA)
     void requestActiveNowPlayingSessionInfo(CallbackID);
+    RetainPtr<NSData> accessibilityRemoteTokenData() const;
+    void accessibilityTransferRemoteToken(RetainPtr<NSData>);
 #endif
 
     void setShouldDispatchFakeMouseMoveEvents(bool dispatch) { m_shouldDispatchFakeMouseMoveEvents = dispatch; }
index 67212f8..7dc5557 100644 (file)
@@ -67,6 +67,10 @@ void WebPage::platformInitialize()
 #endif
 }
 
+void WebPage::platformReinitialize()
+{
+}
+
 void WebPage::platformDetach()
 {
 }
index f18f19a..e15e79b 100644 (file)
@@ -142,10 +142,18 @@ void WebPage::platformInitializeAccessibility()
 {
     m_mockAccessibilityElement = adoptNS([[WKAccessibilityWebPageObject alloc] init]);
     [m_mockAccessibilityElement setWebPage:this];
-    
-    NSData *remoteToken = newAccessibilityRemoteToken([NSUUID UUID]);
-    IPC::DataReference dataToken = IPC::DataReference(reinterpret_cast<const uint8_t*>([remoteToken bytes]), [remoteToken length]);
-    send(Messages::WebPageProxy::RegisterWebProcessAccessibilityToken(dataToken));
+
+    accessibilityTransferRemoteToken(accessibilityRemoteTokenData());
+}
+
+void WebPage::platformReinitialize()
+{
+    accessibilityTransferRemoteToken(accessibilityRemoteTokenData());
+}
+
+RetainPtr<NSData> WebPage::accessibilityRemoteTokenData() const
+{
+    return newAccessibilityRemoteToken([NSUUID UUID]);
 }
 
 static void computeEditableRootHasContentAndPlainText(const VisibleSelection& selection, EditorState::PostLayoutData& data)
index daa5349..b32ca15 100644 (file)
@@ -111,12 +111,20 @@ void WebPage::platformInitialize()
     if ([mockAccessibilityElement respondsToSelector:@selector(accessibilitySetPresenterProcessIdentifier:)])
         [(id)mockAccessibilityElement accessibilitySetPresenterProcessIdentifier:pid];
     [mockAccessibilityElement setWebPage:this];
-
-    // send data back over
-    NSData* remoteToken = [NSAccessibilityRemoteUIElement remoteTokenForLocalUIElement:mockAccessibilityElement];
-    IPC::DataReference dataToken = IPC::DataReference(reinterpret_cast<const uint8_t*>([remoteToken bytes]), [remoteToken length]);
-    send(Messages::WebPageProxy::RegisterWebProcessAccessibilityToken(dataToken));
     m_mockAccessibilityElement = mockAccessibilityElement;
+
+    accessibilityTransferRemoteToken(accessibilityRemoteTokenData());
+}
+
+void WebPage::platformReinitialize()
+{
+    accessibilityTransferRemoteToken(accessibilityRemoteTokenData());
+}
+
+RetainPtr<NSData> WebPage::accessibilityRemoteTokenData() const
+{
+    ASSERT(m_mockAccessibilityElement);
+    return [NSAccessibilityRemoteUIElement remoteTokenForLocalUIElement:m_mockAccessibilityElement.get()];
 }
 
 void WebPage::platformDetach()
index a1d0cfe..12d6de7 100644 (file)
@@ -55,6 +55,10 @@ void WebPage::platformInitialize()
 {
 }
 
+void WebPage::platformReinitialize()
+{
+}
+
 void WebPage::platformDetach()
 {
 }
index 0ffb7a0..8f79614 100644 (file)
@@ -39,6 +39,10 @@ void WebPage::platformInitialize()
 {
 }
 
+void WebPage::platformReinitialize()
+{
+}
+
 void WebPage::platformDetach()
 {
 }