Android Autofill Accessibility, Phase I

content/browser/accessibility/browser_accessibility_manager_android.cc

Index: content/browser/accessibility/browser_accessibility_manager_android.cc
diff --git a/content/browser/accessibility/browser_accessibility_manager_android.cc b/content/browser/accessibility/browser_accessibility_manager_android.cc
index 1c4057918678276fecda2308cc0e8ec4aeb3fbb9..d275051419013f279a524faf1c4fe2bd00355169 100644
--- a/content/browser/accessibility/browser_accessibility_manager_android.cc
+++ b/content/browser/accessibility/browser_accessibility_manager_android.cc
@@ -10,13 +10,16 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/feature_list.h"
 #include "base/i18n/char_iterator.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "content/app/strings/grit/content_strings.h"
 #include "content/browser/accessibility/browser_accessibility_android.h"
 #include "content/browser/accessibility/one_shot_accessibility_tree_search.h"
 #include "content/common/accessibility_messages.h"
+#include "content/public/common/content_client.h"
 #include "jni/BrowserAccessibilityManager_jni.h"
 #include "ui/accessibility/ax_text_utils.h"
 
@@ -117,6 +120,18 @@ AccessibilityMatchPredicate PredicateForSearchKey(
   return AllInterestingNodesPredicate;
 }
 
+// The element in the document for which we may be displaying an autofill popup.
+int32_t element_hosting_autofill_popup_id_ = -1;

[dmazzoni, 2017/03/28 16:47:35]

I think globals like this are typically named something
like g_element_hosting_autofill_popup_id

[csashi, 2017/03/28 17:48:10]

Done.

+
+// The element that currently has focus. Will be used to set
+// |element_hosting_autofill_popup_id_| when we display an autofill popup.
+int32_t current_focused_id_ = -1;

[dmazzoni, 2017/03/28 16:47:35]

You don't need to keep track of this. Just call GetFocus()
and get the ID of the return value.

+
+// Autofill popup will not be part of the |AXTree| that is sent by renderer.
+// Hence, we need a proxy |AXNode| to represent the autofill popup.
+BrowserAccessibility* autofill_popup_proxy_node_ = nullptr;
+ui::AXNode* autofill_popup_proxy_node_ax_node_ = nullptr;
+
 }  // anonymous namespace
 
 namespace aria_strings {
@@ -124,6 +139,9 @@ namespace aria_strings {
   const char kAriaLiveAssertive[] = "assertive";
 }
 
+const base::Feature kAutofillAccessibility{"AndroidAutofillAccessibility",
+                                           base::FEATURE_DISABLED_BY_DEFAULT};
+
 // static
 BrowserAccessibilityManager* BrowserAccessibilityManager::Create(
     const ui::AXTreeUpdate& initial_tree,
@@ -617,6 +635,7 @@ void BrowserAccessibilityManagerAndroid::Focus(JNIEnv* env,
   BrowserAccessibilityAndroid* node = GetFromUniqueID(id);
   if (node)
     node->manager()->SetFocus(*node);
+  current_focused_id_ = id;
 }
 
 void BrowserAccessibilityManagerAndroid::Blur(
@@ -743,6 +762,14 @@ jint BrowserAccessibilityManagerAndroid::FindElementType(
     jint start_id,
     const JavaParamRef<jstring>& element_type_str,
     jboolean forwards) {
+  // Navigate forwards to the autofill popup's proxy node if focus is currently
+  // on the element hosting the autofill popup. Once within the popup, a back
+  // press will navigate back to the element hosting the popup.
+  if (forwards && start_id == element_hosting_autofill_popup_id_ &&
+      autofill_popup_proxy_node_) {
+    return autofill_popup_proxy_node_->unique_id();
+  }
+
   BrowserAccessibilityAndroid* start_node = GetFromUniqueID(start_id);
   if (!start_node)
     return 0;
@@ -918,10 +945,8 @@ void BrowserAccessibilityManagerAndroid::SetAccessibilityFocus(
     const JavaParamRef<jobject>& obj,
     jint id) {
   BrowserAccessibilityAndroid* node = GetFromUniqueID(id);
-  if (!node)
-    return;
-
-  node->manager()->SetAccessibilityFocus(*node);
+  if (node)
+    node->manager()->SetAccessibilityFocus(*node);
 }
 
 bool BrowserAccessibilityManagerAndroid::IsSlider(
@@ -935,6 +960,61 @@ bool BrowserAccessibilityManagerAndroid::IsSlider(
   return node->GetRole() == ui::AX_ROLE_SLIDER;
 }
 
+void BrowserAccessibilityManagerAndroid::OnAutofillPopupDisplayed(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj) {
+  if (!base::FeatureList::IsEnabled(kAutofillAccessibility))
+    return;
+
+  if (autofill_popup_proxy_node_) {
+    LOG(ERROR) << "AutofillPopup proxy node exists, unique_id="

[dmazzoni, 2017/03/28 16:47:35]

Remove this debugging

[csashi, 2017/03/28 17:48:10]

Done.

+               << autofill_popup_proxy_node_->unique_id();
+    OnAutofillPopupDismissed(env, obj);
+  }
+
+  autofill_popup_proxy_node_ = BrowserAccessibility::Create();

[dmazzoni, 2017/03/28 16:47:35]

Do we need a check that this doesn't already exist?

There's no chance OnAutofillPopupDisplayed could be
called twice?

[csashi, 2017/03/28 17:48:10]

The if statement above checks that.
Seems that would be the wrong flow, but it seems better to assume that
OnAutofillPopupDisplayed could be called in succession without an intervening
dismiss.

+  autofill_popup_proxy_node_ax_node_ = new ui::AXNode(nullptr, -1, -1);
+  ui::AXNodeData ax_node_data;
+  ax_node_data.role = ui::AX_ROLE_MENU;
+  content::ContentClient* content_client = content::GetContentClient();
+  ax_node_data.SetName(content_client->GetLocalizedString(
+      IDS_AX_AUTOFILL_POPUP_ACCESSIBLE_NODE_DATA));
+  ax_node_data.state = 1 << ui::AX_STATE_READ_ONLY;
+  ax_node_data.state |= 1 << ui::AX_STATE_FOCUSABLE;
+  ax_node_data.state |= 1 << ui::AX_STATE_SELECTABLE;
+  autofill_popup_proxy_node_ax_node_->SetData(ax_node_data);
+  autofill_popup_proxy_node_->Init(this, autofill_popup_proxy_node_ax_node_);
+
+  element_hosting_autofill_popup_id_ = current_focused_id_;

[dmazzoni, 2017/03/28 16:47:35]

Use GetFocus()->GetId() here

[csashi, 2017/03/28 17:48:10]

Done.

+}
+
+void BrowserAccessibilityManagerAndroid::OnAutofillPopupDismissed(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj) {
+  element_hosting_autofill_popup_id_ = -1;
+
+  if (autofill_popup_proxy_node_) {

[dmazzoni, 2017/03/28 16:47:35]

Do we also need to clean this up if
BrowserAccessibilityManagerAndroid is destroyed
before OnAutofillPopupDismissed is called, like if a
tab is closed?

[csashi, 2017/03/28 17:48:10]

Done.

+    autofill_popup_proxy_node_->Destroy();
+    delete autofill_popup_proxy_node_ax_node_;
+    autofill_popup_proxy_node_ = nullptr;
+  }
+}
+
+jboolean BrowserAccessibilityManagerAndroid::IsAutofillPopupNode(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& obj,
+    jint id) {
+  return autofill_popup_proxy_node_ &&
+         autofill_popup_proxy_node_->unique_id() == id;
+}
+
+jint BrowserAccessibilityManagerAndroid::GetElementHostingAutofillPopupId(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>& obj) {
+  return autofill_popup_proxy_node_ ? autofill_popup_proxy_node_->unique_id()
+                                    : -1;
+}
+
 bool BrowserAccessibilityManagerAndroid::Scroll(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,