Avoid raw pointer in WebAXObjectProxyList

Bug: None
Change-Id: I847fcc2a0abf890c86932f0520262470233eff1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3970885
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1063437}

content/web_test/renderer/accessibility_controller.cc

@@ -163,7 +163,9 @@ AccessibilityController::~AccessibilityController() {
 }
 
 void AccessibilityController::Reset() {
-  elements_.Clear();
+  if (!IsInstalled())
+    return;
+  elements_->Clear();
   notification_callback_.Reset();
   log_accessibility_events_ = false;
   ax_context_.reset();
@@ -172,7 +174,7 @@ void AccessibilityController::Reset() {
 void AccessibilityController::Install(blink::WebLocalFrame* frame) {
   ax_context_ = std::make_unique<blink::WebAXContext>(frame->GetDocument(),
                                                       ui::kAXModeComplete);
-  elements_.SetAXContext(ax_context_.get());
+  elements_ = std::make_unique<WebAXObjectProxyList>(*ax_context_);
   frame->View()->GetSettings()->SetInlineTextBoxAccessibilityEnabled(true);
 
   AccessibilityControllerBindings::Install(weak_factory_.GetWeakPtr(), frame);
@@ -198,6 +200,9 @@ void AccessibilityController::PostNotification(
     const blink::WebAXObject& target,
     const std::string& notification_name,
     const std::vector<ui::AXEventIntent>& event_intents) {
+  if (!IsInstalled())
+    return;
+
   v8::Isolate* isolate = blink::MainThreadIsolate();
   v8::HandleScope handle_scope(isolate);
 
@@ -213,7 +218,7 @@ void AccessibilityController::PostNotification(
   v8::Context::Scope context_scope(context);
 
   // Call notification listeners on the element.
-  v8::Local<v8::Object> element_handle = elements_.GetOrCreate(target);
+  v8::Local<v8::Object> element_handle = elements_->GetOrCreate(target);
   if (element_handle.IsEmpty())
     return;
 
@@ -254,7 +259,7 @@ void AccessibilityController::UnsetNotificationListener() {
 
 v8::Local<v8::Object> AccessibilityController::FocusedElement() {
   blink::WebFrame* frame = web_view()->MainFrame();
-  if (!frame)
+  if (!frame || !IsInstalled())
     return v8::Local<v8::Object>();
 
   // TODO(lukasza): Finish adding OOPIF support to the web tests harness.
@@ -267,11 +272,13 @@ v8::Local<v8::Object> AccessibilityController::FocusedElement() {
           frame->ToWebLocalFrame()->GetDocument());
   if (focused_element.IsNull())
     focused_element = GetAccessibilityObjectForMainFrame();
-  return elements_.GetOrCreate(focused_element);
+  return elements_->GetOrCreate(focused_element);
 }
 
 v8::Local<v8::Object> AccessibilityController::RootElement() {
-  return elements_.GetOrCreate(GetAccessibilityObjectForMainFrame());
+  if (!IsInstalled())
+    return v8::Local<v8::Object>();
+  return elements_->GetOrCreate(GetAccessibilityObjectForMainFrame());
 }
 
 v8::Local<v8::Object> AccessibilityController::AccessibleElementById(
@@ -299,7 +306,7 @@ AccessibilityController::FindAccessibleElementByIdRecursive(
   if (!node.IsNull() && node.IsElementNode()) {
     blink::WebElement element = node.To<blink::WebElement>();
     if (element.GetAttribute("id") == id)
-      return elements_.GetOrCreate(obj);
+      return elements_->GetOrCreate(obj);
   }
 
   unsigned childCount = obj.ChildCount();

content/web_test/renderer/accessibility_controller.h

@@ -61,6 +61,7 @@ class AccessibilityController {
   v8::Local<v8::Object> RootElement();
   v8::Local<v8::Object> AccessibleElementById(const std::string& id);
   bool CanCallAOMEventListeners() const;
+  bool IsInstalled() { return elements_ != nullptr; }
 
   v8::Local<v8::Object> FindAccessibleElementByIdRecursive(
       const blink::WebAXObject&,
@@ -71,7 +72,7 @@ class AccessibilityController {
   // If true, will log all accessibility notifications.
   bool log_accessibility_events_;
 
-  WebAXObjectProxyList elements_;
+  std::unique_ptr<WebAXObjectProxyList> elements_;
 
   v8::Persistent<v8::Function> notification_callback_;
 

content/web_test/renderer/web_ax_object_proxy.cc

@@ -1771,7 +1771,8 @@ bool RootWebAXObjectProxy::IsRoot() const {
   return true;
 }
 
-WebAXObjectProxyList::WebAXObjectProxyList() = default;
+WebAXObjectProxyList::WebAXObjectProxyList(blink::WebAXContext& ax_context)
+    : ax_context_(&ax_context) {}
 
 WebAXObjectProxyList::~WebAXObjectProxyList() {
   Clear();
@@ -1831,10 +1832,6 @@ v8::Local<v8::Object> WebAXObjectProxyList::GetOrCreate(
   return handle;
 }
 
-void WebAXObjectProxyList::SetAXContext(blink::WebAXContext* ax_context) {
-  ax_context_ = ax_context;
-}
-
 blink::WebAXContext* WebAXObjectProxyList::GetAXContext() {
   return ax_context_;
 }

content/web_test/renderer/web_ax_object_proxy.h

@@ -30,7 +30,6 @@ class WebAXObjectProxy : public gin::Wrappable<WebAXObjectProxy> {
     virtual ~Factory() {}
     virtual v8::Local<v8::Object> GetOrCreate(
         const blink::WebAXObject& object) = 0;
-    virtual void SetAXContext(blink::WebAXContext* ax_context) = 0;
     virtual blink::WebAXContext* GetAXContext() = 0;
   };
 
@@ -257,30 +256,18 @@ class RootWebAXObjectProxy : public WebAXObjectProxy {
   bool IsRoot() const override;
 };
 
-// Provides simple lifetime management of the WebAXObjectProxy instances: all
-// WebAXObjectProxys ever created from the controller are stored in a list and
-// cleared explicitly.
-// TODO(aleventhal) Don't store  ax_context_ as a non-const raw pointer.
-// Refactor as the following:
-// - The constructor takes a blink::WebAXContext& argument.
-// - ax_context_ is declared as 'blink::WebAXContext* const ax_context_;'
-// - Get rid of SetAXContext()
-// - std::unique_ptr<WebAXObjectProxyList> AccessibilityController::elements_;
-// - AccessibilityController::elements_ is populated from
-// AccessibilityController::Install().
 class WebAXObjectProxyList : public WebAXObjectProxy::Factory {
  public:
-  WebAXObjectProxyList();
+  explicit WebAXObjectProxyList(blink::WebAXContext&);
   ~WebAXObjectProxyList() override;
 
   void Clear();
   v8::Local<v8::Object> GetOrCreate(const blink::WebAXObject&) override;
-  void SetAXContext(blink::WebAXContext* ax_context) override;
   blink::WebAXContext* GetAXContext() override;
 
  private:
   std::vector<v8::Global<v8::Object>> elements_;
-  blink::WebAXContext* ax_context_ = nullptr;
+  blink::WebAXContext* const ax_context_;
 };
 
 }  // namespace content