Allow requesting Automation tree by tabId

chrome/browser/extensions/api/automation_internal/automation_internal_api.cc

Index: chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
diff --git a/chrome/browser/extensions/api/automation_internal/automation_internal_api.cc b/chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
index ebc7ac1293402cb5bd40a2d53350136c4114fb6c..dbb706f2a599d76c71b6286e4658bb5d21e182b0 100644
--- a/chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
+++ b/chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
@@ -6,8 +6,11 @@
 
 #include <vector>
 
+#include "base/strings/string_number_conversions.h"
 #include "chrome/browser/extensions/api/automation_internal/automation_action_adapter.h"
 #include "chrome/browser/extensions/api/automation_internal/automation_util.h"
+#include "chrome/browser/extensions/api/tabs/tabs_constants.h"
+#include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/extensions/api/automation_internal.h"
@@ -18,6 +21,7 @@
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/common/error_utils.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/ui/ash/accessibility/automation_manager_ash.h"
@@ -95,18 +99,32 @@ class RenderWidgetHostActionAdapter : public AutomationActionAdapter {
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostActionAdapter);
 };
 
-// TODO(aboxhall/dtseng): ensure that the initial data is sent down for the tab
-// if this doesn't turn accessibility on for the first time (e.g. if a
-// RendererAccessibility object existed already because a screenreader has been
-// run at some point).
 ExtensionFunction::ResponseAction
-AutomationInternalEnableCurrentTabFunction::Run() {
-  Browser* current_browser = GetCurrentBrowser();
-  TabStripModel* tab_strip = current_browser->tab_strip_model();
-  content::WebContents* contents =
-      tab_strip->GetWebContentsAt(tab_strip->active_index());
-  if (!contents)
-    return RespondNow(Error("No active tab"));
+AutomationInternalEnableTabFunction::Run() {
+  using api::automation_internal::EnableTab::Params;
+  scoped_ptr<Params> params(Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+  content::WebContents* contents = NULL;
+  if (params->tab_id.get()) {
+    TabStripModel* tab_strip = NULL;
+    int tab_index = -1;
+    int tab_id = *params->tab_id;
+    if (!ExtensionTabUtil::GetTabById(tab_id,
+                                      GetProfile(),
+                                      include_incognito(),
+                                      NULL,

[David Tseng, 2014/05/29 22:41:31]

What is this (in-line comments would help out)?

[aboxhall, 2014/05/30 15:37:11]

Added inline comments.

+                                      &tab_strip,
+                                      &contents,
+                                      &tab_index)) {
+      return RespondNow(Error(
+          ErrorUtils::FormatErrorMessage(tabs_constants::kTabNotFoundError,
+                                         base::IntToString(tab_id))));
+    }
+  } else {
+    contents = GetCurrentBrowser()->tab_strip_model()->GetActiveWebContents();
+    if (!contents)
+      return RespondNow(Error("No active tab"));
+  }
   content::RenderWidgetHost* rwh =
       contents->GetRenderWidgetHostView()->GetRenderWidgetHost();
   if (!rwh)
@@ -114,9 +132,9 @@ AutomationInternalEnableCurrentTabFunction::Run() {
   AutomationWebContentsObserver::CreateForWebContents(contents);
   rwh->EnableTreeOnlyAccessibilityMode();
   return RespondNow(
-      ArgumentList(api::automation_internal::EnableCurrentTab::Results::Create(
+      ArgumentList(api::automation_internal::EnableTab::Results::Create(
           rwh->GetProcess()->GetID(), rwh->GetRoutingID())));
-}
+  }

[David Tseng, 2014/05/29 22:41:31]

While we're here and I don't forget to ask, are you planning on handling the
"destroyed" code path to help us cleanup extension-side?

[aboxhall, 2014/05/30 15:37:11]

Yes, sorry - that is next on my list :)

 
 ExtensionFunction::ResponseAction
 AutomationInternalPerformActionFunction::Run() {