Attachment 362247 Details for Bug 194742 – patch

[patch] patch

patch (text/plain), 13.42 KB, created by chris fleizach on 2019-02-17 13:23:00 PST

(hide)

Description:

Filename:

MIME Type:

Creator: chris fleizach

Created: 2019-02-17 13:23:00 PST

Size: 13.42 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 241652)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2019-02-16  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 NOBODY (OOPS!).
>+
>+        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-16  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] RenderImage's default intrinsic size is 0.
>Index: Source/WebCore/accessibility/mac/AXObjectCacheMac.mm
>===================================================================
>--- Source/WebCore/accessibility/mac/AXObjectCacheMac.mm	(revision 241652)
>+++ Source/WebCore/accessibility/mac/AXObjectCacheMac.mm	(working copy)
>@@ -261,15 +261,19 @@
>     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)
>@@ -276,7 +280,8 @@
> {
>     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 @@
>             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 @@
>     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: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 241652)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,39 @@
>+2019-02-16  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 NOBODY (OOPS!).
>+
>+        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-15  Youenn Fablet  <youenn@apple.com>
> 
>         NetworkDataTask should check its client before calling shouldCaptureExtraNetworkLoadMetrics
>Index: Source/WebKit/UIProcess/mac/PageClientImplMac.mm
>===================================================================
>--- Source/WebKit/UIProcess/mac/PageClientImplMac.mm	(revision 241652)
>+++ Source/WebKit/UIProcess/mac/PageClientImplMac.mm	(working copy)
>@@ -89,6 +89,8 @@
> #include <WebCore/WebMediaSessionManager.h>
> #endif
> 
>+static NSString * const kAXLoadCompleteNotification = @"AXLoadComplete";
>+
> @interface NSApplication (WebNSApplicationDetails)
> - (NSCursor *)_cursorRectCursor;
> @end
>@@ -840,6 +842,8 @@
> {
>     if (auto gestureController = m_impl->gestureController())
>         gestureController->didFinishLoadForMainFrame();
>+
>+    NSAccessibilityPostNotification(NSAccessibilityUnignoredAncestor(m_view), kAXLoadCompleteNotification);
> }
> 
> void PageClientImpl::didFailLoadForMainFrame()
>@@ -846,6 +850,8 @@
> {
>     if (auto gestureController = m_impl->gestureController())
>         gestureController->didFailLoadForMainFrame();
>+
>+    NSAccessibilityPostNotification(NSAccessibilityUnignoredAncestor(m_view), kAXLoadCompleteNotification);
> }
> 
> void PageClientImpl::didSameDocumentNavigationForMainFrame(SameDocumentNavigationType type)
>Index: Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm	(working copy)
>@@ -183,6 +183,12 @@
>     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: Source/WebKit/WebProcess/WebPage/WebPage.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.cpp	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.cpp	(working copy)
>@@ -714,6 +714,8 @@
>         setActivityState(parameters.activityState, ActivityStateChangeAsynchronous, Vector<CallbackID>());
>     if (m_layerHostingMode != parameters.layerHostingMode)
>         setLayerHostingMode(parameters.layerHostingMode);
>+
>+    platformReinitialize();
> }
> 
> void WebPage::updateThrottleState()
>Index: Source/WebKit/WebProcess/WebPage/WebPage.h
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.h	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.h	(working copy)
>@@ -1159,6 +1159,7 @@
>     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 @@
> 
> #if PLATFORM(COCOA)
>     void requestActiveNowPlayingSessionInfo(CallbackID);
>+    RetainPtr<NSData> accessibilityRemoteTokenData() const;
>+    void accessibilityTransferRemoteToken(RetainPtr<NSData>);
> #endif
> 
>     void setShouldDispatchFakeMouseMoveEvents(bool dispatch) { m_shouldDispatchFakeMouseMoveEvents = dispatch; }
>Index: Source/WebKit/WebProcess/WebPage/gtk/WebPageGtk.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/gtk/WebPageGtk.cpp	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/gtk/WebPageGtk.cpp	(working copy)
>@@ -67,6 +67,10 @@
> #endif
> }
> 
>+void WebPage::platformReinitialize()
>+{
>+}
>+
> void WebPage::platformDetach()
> {
> }
>Index: Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	(working copy)
>@@ -142,12 +142,20 @@
> {
>     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)
> {
>     data.hasContent = false;
>Index: Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm	(working copy)
>@@ -111,14 +111,22 @@
>     if ([mockAccessibilityElement respondsToSelector:@selector(accessibilitySetPresenterProcessIdentifier:)])
>         [(id)mockAccessibilityElement accessibilitySetPresenterProcessIdentifier:pid];
>     [mockAccessibilityElement setWebPage:this];
>+    m_mockAccessibilityElement = mockAccessibilityElement;
> 
>-    // 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()
> {
>     [m_mockAccessibilityElement setWebPage:nullptr];
>Index: Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/win/WebPageWin.cpp	(working copy)
>@@ -55,6 +55,10 @@
> {
> }
> 
>+void WebPage::platformReinitialize()
>+{
>+}
>+
> void WebPage::platformDetach()
> {
> }
>Index: Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp	(revision 241652)
>+++ Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp	(working copy)
>@@ -39,6 +39,10 @@
> {
> }
> 
>+void WebPage::platformReinitialize()
>+{
>+}
>+
> void WebPage::platformDetach()
> {
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 194742: 362212 | 362213 | 362215 | 362216 | 362217 | 362246 | 362247