Revert "Stablizing Mac autofill accessibility when popup show/hides"

This reverts commit 0297659d.

Reason for revert: Build failures here:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20CFI

Original change's description:
> Stablizing Mac autofill accessibility when popup show/hides
> 
> This is a follow up change of CL:1667787, which introduced
> AutofillPopupControllerImpl::FireControlsChangedEvent for autofill
> popup accessibility.
> 
> On Mac and potentially other platforms, when accessibility is
> enabled, upon invoking autofill popup and FireControlsChangedEvent
> we end up dereferencing a nullptr of AxPlatformNode due to
> Mac does not have a complete implementation of AxPlatformNode yet.
> 
> This CL fixes the above by adding a check for AxPlatformNode in
> FireControlsChangedEvent.
> 
> Changes:
>  1. Introduced a check for invalid AxPlatformNode in
>     AutofillPopupControllerImpl::FireControlsChangedEvent to return
>     early from firing event.
>  2. Added associated unit tests for FireControlsChangedEvent.
>  3. Exposed GetWebContentsPopupControllerAxId virtual to
>     AutofillPopupDelegate..
> 
> Bug: 986587
> Change-Id: I9a6ed49330a9ea9d7d4e5483c3ee06d5675919ee
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1714297
> Commit-Queue: Victor Fei <vicfei@microsoft.com>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Ian Prest <iapres@microsoft.com>
> Reviewed-by: Nektarios Paisios <nektar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#686461}

TBR=sky@chromium.org,estade@chromium.org,nektar@chromium.org,ftirelo@chromium.org,akihiroota@chromium.org,iapres@microsoft.com,vicfei@microsoft.com

Change-Id: I67ef1a599f54569e9cd9a964bc3b1b27f7c27c4f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 986587
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752863Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686552}

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

@@ -597,31 +597,24 @@ void AutofillPopupControllerImpl::FireControlsChangedEvent(bool is_show) {
 
   // Retrieve the ax node id associated with the current web contents' element
   // that has a controller relation to the current autofill popup.
-  int32_t node_id = delegate_->GetWebContentsPopupControllerAxId();
+  int32_t node_id = static_cast<AutofillExternalDelegate*>(delegate_.get())
+                        ->GetWebContentsPopupControllerAxId();
 
   // We can only raise controls changed accessibility event when we have a valid
   // ax tree and an ax node associated with the ax tree for the popup
   // controller, and a valid ax unique id for the popup controllee.
-  if (!ax_tree_manager)
-    return;
-  ui::AXPlatformNodeDelegate* ax_platform_node_delegate =
-      ax_tree_manager->GetDelegate(tree_id, node_id);
-  if (!ax_platform_node_delegate)
-    return;
-  ui::AXPlatformNode* target_node =
-      ax_platform_node_delegate->GetFromNodeID(node_id);
-  base::Optional<int32_t> popup_ax_id = view_->GetAxUniqueId();
-  if (!target_node || !popup_ax_id)
+  if (!ax_tree_manager || !ax_tree_manager->GetDelegate(tree_id, node_id) ||
+      !view_->GetAxUniqueId())
     return;
 
-  // All the conditions are valid, raise the accessibility event and set global
-  // popup ax unique id.
   if (is_show)
-    ui::SetActivePopupAxUniqueId(popup_ax_id);
+    ui::SetActivePopupAxUniqueId(view_->GetAxUniqueId());
   else
     ui::ClearActivePopupAxUniqueId();
 
-  target_node->NotifyAccessibilityEvent(ax::mojom::Event::kControlsChanged);
+  ax_tree_manager->GetDelegate(tree_id, node_id)
+      ->GetFromNodeID(node_id)
+      ->NotifyAccessibilityEvent(ax::mojom::Event::kControlsChanged);
 }
 
 }  // namespace autofill

chrome/browser/ui/autofill/autofill_popup_controller_impl.h

@@ -139,11 +139,6 @@ class AutofillPopupControllerImpl : public AutofillPopupController {
 
   AutofillPopupLayoutModel& LayoutModelForTesting() { return layout_model_; }
 
-  // Raise an accessibility event to indicate the controls relation of the
-  // form control of the popup and popup itself has changed based on the popup's
-  // show or hide action.
-  void FireControlsChangedEvent(bool is_show);
-
  private:
 #if !defined(OS_ANDROID)
   FRIEND_TEST_ALL_PREFIXES(AutofillPopupControllerUnitTest, ElideText);
@@ -160,8 +155,12 @@ class AutofillPopupControllerImpl : public AutofillPopupController {
   // Hides |view_| unless it is null and then deletes |this|.
   void HideViewAndDie();
 
+  // Raise an accessibility event to indicate the controls relation of the
+  // form control of the popup and popup itself has changed based on the popup's
+  // show or hide action.
+  void FireControlsChangedEvent(bool is_show);
+
   friend class AutofillPopupControllerUnitTest;
-  friend class AutofillPopupControllerAccessibilityUnitTest;
   void SetViewForTesting(AutofillPopupView* view) { view_ = view; }
 
   PopupControllerCommon controller_common_;

components/autofill/core/browser/autofill_external_delegate.cc

@@ -286,10 +286,6 @@ AutofillDriver* AutofillExternalDelegate::GetAutofillDriver() {
   return driver_;
 }
 
-int32_t AutofillExternalDelegate::GetWebContentsPopupControllerAxId() const {
-  return query_field_.form_control_ax_id;
-}
-
 void AutofillExternalDelegate::RegisterDeletionCallback(
     base::OnceClosure deletion_callback) {
   deletion_callback_ = std::move(deletion_callback);
@@ -299,6 +295,10 @@ void AutofillExternalDelegate::Reset() {
   manager_->client()->HideAutofillPopup();
 }
 
+int32_t AutofillExternalDelegate::GetWebContentsPopupControllerAxId() const {
+  return query_field_.form_control_ax_id;
+}
+
 base::WeakPtr<AutofillExternalDelegate> AutofillExternalDelegate::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }

components/autofill/core/browser/autofill_external_delegate.h

@@ -57,10 +57,6 @@ class AutofillExternalDelegate : public AutofillPopupDelegate {
   // popup type after call to |onQuery|.
   PopupType GetPopupType() const override;
   AutofillDriver* GetAutofillDriver() override;
-
-  // Returns the ax node id associated with the current web contents' element
-  // who has a controller relation to the current autofill popup.
-  int32_t GetWebContentsPopupControllerAxId() const override;
   void RegisterDeletionCallback(base::OnceClosure deletion_callback) override;
 
   // Records and associates a query_id with web form data.  Called
@@ -100,6 +96,10 @@ class AutofillExternalDelegate : public AutofillPopupDelegate {
   // values or settings.
   void Reset();
 
+  // Returns the ax node id associated with the current web contents' element
+  // who has a controller relation to the current autofill popup.
+  int32_t GetWebContentsPopupControllerAxId() const;
+
   const FormData& query_form() const { return query_form_; }
 
  protected:

components/autofill/core/browser/ui/autofill_popup_delegate.h

@@ -56,10 +56,6 @@ class AutofillPopupDelegate {
   // Returns the associated AutofillDriver.
   virtual AutofillDriver* GetAutofillDriver() = 0;
 
-  // Returns the ax node id associated with the current web contents' element
-  // who has a controller relation to the current autofill popup.
-  virtual int32_t GetWebContentsPopupControllerAxId() const = 0;
-
   // Sets |deletion_callback| to be called from the delegate's destructor.
   // Useful for deleting objects which cannot be owned by the delegate but
   // should not outlive it.

components/password_manager/core/browser/password_autofill_manager.cc

@@ -260,12 +260,6 @@ autofill::AutofillDriver* PasswordAutofillManager::GetAutofillDriver() {
   return password_manager_driver_->GetAutofillDriver();
 }
 
-int32_t PasswordAutofillManager::GetWebContentsPopupControllerAxId() const {
-  // TODO: Needs to be implemented once we step up accessibility features later.
-  NOTIMPLEMENTED_LOG_ONCE() << "See http://crbug.com/991253";
-  return 0;
-}
-
 void PasswordAutofillManager::RegisterDeletionCallback(
     base::OnceClosure deletion_callback) {
   deletion_callback_ = std::move(deletion_callback);

components/password_manager/core/browser/password_autofill_manager.h

@@ -55,7 +55,6 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
   void ClearPreviewedForm() override;
   autofill::PopupType GetPopupType() const override;
   autofill::AutofillDriver* GetAutofillDriver() override;
-  int32_t GetWebContentsPopupControllerAxId() const override;
   void RegisterDeletionCallback(base::OnceClosure deletion_callback) override;
 
   // Invoked when a password mapping is added.