Bug 1531579 - Refresh viewport cache on tree mutations and fire android event. r=yzen
☠☠ backed out by 5242fa07735c ☠ ☠
authorEitan Isaacson <eitan@monotonous.org>
Tue, 05 Mar 2019 19:28:41 +0000
changeset 521440 583048926c12
parent 521439 e2b7138d7c24
child 521441 cc930a52098b
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1531579
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1531579 - Refresh viewport cache on tree mutations and fire android event. r=yzen Differential Revision: https://phabricator.services.mozilla.com/D21610
accessible/android/AccessibleWrap.cpp
accessible/android/DocAccessibleWrap.h
accessible/android/Platform.cpp
accessible/android/SessionAccessibility.cpp
accessible/android/SessionAccessibility.h
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
--- a/accessible/android/AccessibleWrap.cpp
+++ b/accessible/android/AccessibleWrap.cpp
@@ -63,16 +63,26 @@ nsresult AccessibleWrap::HandleAccEvent(
         if (newPosition) {
           if (DocAccessibleWrap* topContentDoc =
                   doc->GetTopLevelContentDoc(accessible)) {
             topContentDoc->CacheFocusPath(newPosition);
           }
         }
         break;
       }
+      case nsIAccessibleEvent::EVENT_SHOW:
+      case nsIAccessibleEvent::EVENT_HIDE: {
+        if (DocAccessibleWrap* topContentDoc =
+                doc->GetTopLevelContentDoc(accessible)) {
+          topContentDoc->CacheViewport();
+        }
+        break;
+      }
+      default:
+        break;
     }
   }
 
   nsresult rv = Accessible::HandleAccEvent(aEvent);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (IPCAccessibilityActive()) {
     return NS_OK;
@@ -154,23 +164,16 @@ nsresult AccessibleWrap::HandleAccEvent(
     }
     case nsIAccessibleEvent::EVENT_SCROLLING: {
       AccScrollingEvent* event = downcast_accEvent(aEvent);
       sessionAcc->SendScrollingEvent(accessible, event->ScrollX(),
                                      event->ScrollY(), event->MaxScrollX(),
                                      event->MaxScrollY());
       break;
     }
-    case nsIAccessibleEvent::EVENT_SHOW:
-    case nsIAccessibleEvent::EVENT_HIDE: {
-      AccMutationEvent* event = downcast_accEvent(aEvent);
-      auto parent = static_cast<AccessibleWrap*>(event->Parent());
-      sessionAcc->SendWindowContentChangedEvent(parent);
-      break;
-    }
     default:
       break;
   }
 
   return NS_OK;
 }
 
 void AccessibleWrap::Shutdown() {
--- a/accessible/android/DocAccessibleWrap.h
+++ b/accessible/android/DocAccessibleWrap.h
@@ -27,33 +27,33 @@ class DocAccessibleWrap : public DocAcce
   }
   void RemoveID(uint32_t aID) { mIDToAccessibleMap.Remove(aID); }
   AccessibleWrap* GetAccessibleByID(int32_t aID) const;
 
   DocAccessibleWrap* GetTopLevelContentDoc(AccessibleWrap* aAccessible);
 
   void CacheFocusPath(AccessibleWrap* aAccessible);
 
+  void CacheViewport();
+
   enum {
     eBatch_Viewport = 0,
     eBatch_FocusPath = 1,
     eBatch_BoundsUpdate = 2,
   };
 
  protected:
   /*
    * This provides a mapping from 32 bit id to accessible objects.
    */
   nsDataHashtable<nsUint32HashKey, AccessibleWrap*> mIDToAccessibleMap;
 
   virtual void DoInitialUpdate() override;
 
  private:
-  void CacheViewport();
-
   void UpdateFocusPathBounds();
 
   static void CacheViewportCallback(nsITimer* aTimer, void* aDocAccParam);
 
   nsCOMPtr<nsITimer> mCacheRefreshTimer;
 
   AccessibleHashtable mFocusPath;
 };
--- a/accessible/android/Platform.cpp
+++ b/accessible/android/Platform.cpp
@@ -102,21 +102,18 @@ void a11y::ProxyTextChangeEvent(ProxyAcc
     sessionAcc->SendTextChangedEvent(WrapperFor(aTarget), aStr, aStart, aLen,
                                      aIsInsert, aFromUser);
   }
 }
 
 void a11y::ProxyShowHideEvent(ProxyAccessible* aTarget,
                               ProxyAccessible* aParent, bool aInsert,
                               bool aFromUser) {
-  SessionAccessibility* sessionAcc =
-      SessionAccessibility::GetInstanceFor(aTarget);
-  if (sessionAcc) {
-    sessionAcc->SendWindowContentChangedEvent(WrapperFor(aParent));
-  }
+  // We rely on the window content changed events to be dispatched
+  // after the viewport cache is refreshed.
 }
 
 void a11y::ProxySelectionEvent(ProxyAccessible*, ProxyAccessible*, uint32_t) {}
 
 void a11y::ProxyVirtualCursorChangeEvent(
     ProxyAccessible* aTarget, ProxyAccessible* aOldPosition,
     int32_t aOldStartOffset, int32_t aOldEndOffset,
     ProxyAccessible* aNewPosition, int32_t aNewStartOffset,
--- a/accessible/android/SessionAccessibility.cpp
+++ b/accessible/android/SessionAccessibility.cpp
@@ -195,25 +195,23 @@ void SessionAccessibility::SendScrolling
                   java::sdk::Integer::ValueOf(aMaxScrollX));
   GECKOBUNDLE_PUT(eventInfo, "maxScrollY",
                   java::sdk::Integer::ValueOf(aMaxScrollY));
   GECKOBUNDLE_FINISH(eventInfo);
 
   mSessionAccessibility->SendEvent(
       java::sdk::AccessibilityEvent::TYPE_VIEW_SCROLLED, virtualViewId,
       aAccessible->AndroidClass(), eventInfo);
-
-  SendWindowContentChangedEvent(aAccessible);
 }
 
-void SessionAccessibility::SendWindowContentChangedEvent(
-    AccessibleWrap* aAccessible) {
+void SessionAccessibility::SendWindowContentChangedEvent() {
   mSessionAccessibility->SendEvent(
       java::sdk::AccessibilityEvent::TYPE_WINDOW_CONTENT_CHANGED,
-      aAccessible->VirtualViewID(), aAccessible->AndroidClass(), nullptr);
+      AccessibleWrap::kNoID, java::SessionAccessibility::CLASSNAME_WEBVIEW,
+      nullptr);
 }
 
 void SessionAccessibility::SendWindowStateChangedEvent(
     AccessibleWrap* aAccessible) {
   // Suppress window state changed events from about:blank pages.
   // This is important for tests.
   if (aAccessible->IsDoc() && aAccessible->ChildCount() == 0) {
     return;
@@ -337,16 +335,17 @@ void SessionAccessibility::ReplaceViewpo
           data.TextValue(), data.DOMNodeID(), data.Description());
       infos->SetElement(i, bundle);
     } else {
       infos->SetElement(i, acc->ToBundle(true));
     }
   }
 
   mSessionAccessibility->ReplaceViewportCache(infos);
+  SendWindowContentChangedEvent();
 }
 
 void SessionAccessibility::ReplaceFocusPathCache(
     const nsTArray<AccessibleWrap*>& aAccessibles,
     const nsTArray<BatchData>& aData) {
   auto infos = jni::ObjectArray::New<java::GeckoBundle>(aAccessibles.Length());
   for (size_t i = 0; i < aAccessibles.Length(); i++) {
     AccessibleWrap* acc = aAccessibles.ElementAt(i);
@@ -386,9 +385,10 @@ void SessionAccessibility::UpdateCachedB
           data.TextValue(), data.DOMNodeID(), data.Description());
       infos->SetElement(i, bundle);
     } else {
       infos->SetElement(i, acc->ToBundle(true));
     }
   }
 
   mSessionAccessibility->UpdateCachedBounds(infos);
+  SendWindowContentChangedEvent();
 }
--- a/accessible/android/SessionAccessibility.h
+++ b/accessible/android/SessionAccessibility.h
@@ -89,17 +89,17 @@ class SessionAccessibility final
                                      int32_t aCaretOffset);
   void SendTextTraversedEvent(AccessibleWrap* aAccessible, int32_t aStartOffset,
                               int32_t aEndOffset);
   void SendTextChangedEvent(AccessibleWrap* aAccessible, const nsString& aStr,
                             int32_t aStart, uint32_t aLen, bool aIsInsert,
                             bool aFromUser);
   void SendSelectedEvent(AccessibleWrap* aAccessible, bool aSelected);
   void SendClickedEvent(AccessibleWrap* aAccessible, bool aChecked);
-  void SendWindowContentChangedEvent(AccessibleWrap* aAccessible);
+  void SendWindowContentChangedEvent();
   void SendWindowStateChangedEvent(AccessibleWrap* aAccessible);
 
   // Cache methods
   void ReplaceViewportCache(
       const nsTArray<AccessibleWrap*>& aAccessibles,
       const nsTArray<BatchData>& aData = nsTArray<BatchData>());
 
   void ReplaceFocusPathCache(
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
@@ -152,16 +152,19 @@ class AccessibilityTest : BaseSessionTes
         // XXX: Sometimes we get the window state change of the initial
         // about:blank page loading. Need to figure out how to ignore that.
         sessionRule.waitUntilCalled(object : EventDelegate {
             @AssertCalled(count = 1)
             override fun onFocused(event: AccessibilityEvent) { }
 
             @AssertCalled
             override fun onWinStateChanged(event: AccessibilityEvent) { }
+
+            @AssertCalled
+            override fun onWinContentChanged(event: AccessibilityEvent) { }
         })
 
         if (moveToFirstChild) {
             provider.performAction(View.NO_ID,
                 AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, null)
         }
     }
 
@@ -571,16 +574,36 @@ class AccessibilityTest : BaseSessionTes
 
         provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_SELECT, null)
         waitUntilSelect(true)
 
         provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_SELECT, null)
         waitUntilSelect(false)
     }
 
+    @Test fun testMutation() {
+        sessionRule.session.loadString(
+                "<div><p id='to_show'>I will be shown</p></div>","text/html")
+        waitForInitialFocus()
+
+        val rootNode = createNodeInfo(View.NO_ID)
+        assertThat("Document has 1 child", rootNode.childCount, equalTo(1))
+
+        assertThat("Section has 1 child",
+                createNodeInfo(rootNode.getChildId(0)).childCount, equalTo(1))
+        mainSession.evaluateJS("$('#to_show').style.display = 'none';")
+        sessionRule.waitUntilCalled(object : EventDelegate {
+            @AssertCalled(count = 1)
+            override fun onWinContentChanged(event: AccessibilityEvent) { }
+        })
+
+        assertThat("Section has no children",
+                createNodeInfo(rootNode.getChildId(0)).childCount, equalTo(0))
+    }
+
     private fun screenContainsNode(nodeId: Int): Boolean {
         var node = createNodeInfo(nodeId)
         var nodeBounds = Rect()
         node.getBoundsInScreen(nodeBounds)
         return screenRect.contains(nodeBounds)
     }
 
     @Ignore // Bug 1506276 - We need to reliably wait for APZC here, and it's not trivial.