Implement the target locator commands for ChromeDriver.

chrome/test/webdriver/automation.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/test/webdriver/automation.h"
 
 #include "base/command_line.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
@@ skipping 19 matching lines @@
     return;
   }
   // TODO(kkania): See if these are still needed.
   test_launcher_utils::PrepareBrowserCommandLineForTests(&launch_arguments_);
   launch_arguments_.AppendSwitch(switches::kDomAutomationController);
   launch_arguments_.AppendSwitch(switches::kFullMemoryCrashReport);
 
   launch_arguments_.AppendSwitchPath(switches::kUserDataDir,
                                      profile_dir_.path());
   UITestBase::SetUp();
-  browser_ = automation()->GetBrowserWindow(0);
-  if (!browser_.get()) {
-    Terminate();
-    return;
-  }
-  tab_ = browser_->GetActiveTab();
-  if (!tab_.get()) {
-    Terminate();
-    return;
-  }
   *success = true;
 }
 
 void Automation::Terminate() {
   QuitBrowser();
 }
 
-void Automation::ExecuteScript(const std::string& frame_xpath,
+void Automation::ExecuteScript(int tab_id,
+                               const std::string& frame_xpath,
                                const std::string& script,
                                std::string* result,
                                bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
   std::wstring wide_xpath = UTF8ToWide(frame_xpath);
   std::wstring wide_script = UTF8ToWide(script);
   std::wstring wide_result;
-  *success = tab_->ExecuteAndExtractString(
+  *success = tab->ExecuteAndExtractString(
       wide_xpath, wide_script, &wide_result);
   if (*success)
     *result = WideToUTF8(wide_result);
 }
 
-void Automation::SendWebKeyEvent(const WebKeyEvent& key_event,
+void Automation::SendWebKeyEvent(int tab_id,
+                                 const WebKeyEvent& key_event,
                                  bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
+  int tab_index = 0;
+  int browser_count = 0;
+  if (!tab->GetTabIndex(&tab_index) ||
+      !automation()->GetBrowserWindowCount(&browser_count)) {
+    *success = false;
+    return;
+  }
+  scoped_refptr<BrowserProxy> browser_with_tab;
+  for (int i = 0; i < browser_count; ++i) {

[Paweł Hajdan Jr., 2011/02/14 09:29:43]

This loop looks worrying. So far no automation code required that kind of loop.
Can you keep a TabProxy around instead of doing a linear lookup every time? If
that's not possible, have you considered alternative solutions that wouldn't
require looping over all browsers?

[kkania, 2011/02/14 17:42:12]

Yes, I do not like this code either. I am keeping the tab proxy around, but that
is not what I need. I need a BrowserProxy, and there is no way to get that from
the TabProxy currently. Perhaps adding such a method is the best way to solve
this problem.

The other way would be to keep track of all BrowserProxies the user knows about
along with the TabProxies, and know which are associated with which.

I think I prefer the first way better. I'll add it.

+    scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(i);
+    if (browser.get()) {
+      scoped_refptr<TabProxy> tab_at_index = browser->GetTab(tab_index);
+      if (tab_at_index->handle() == tab->handle()) {
+        browser_with_tab = browser;
+        break;
+      }
+    }
+  }
+  if (!browser_with_tab.get() || !browser_with_tab->ActivateTab(tab_index)) {
+    LOG(ERROR) << "Could not activate tab to send keys";
+    *success = false;
+    return;
+  }
   scoped_ptr<DictionaryValue> dict(new DictionaryValue);
   dict->SetString("command", "SendKeyEventToActiveTab");
   dict->SetInteger("type", key_event.type);
   dict->SetInteger("nativeKeyCode", key_event.key_code);
   dict->SetInteger("windowsKeyCode", key_event.key_code);
   dict->SetString("unmodifiedText", key_event.unmodified_text);
   dict->SetString("text", key_event.modified_text);
   dict->SetInteger("modifiers", key_event.modifiers);
   dict->SetBoolean("isSystemKey", false);
   std::string request;
   base::JSONWriter::Write(dict.get(), false, &request);
   std::string reply;
-  *success = browser_->SendJSONRequest(request, &reply);
+  *success = browser_with_tab->SendJSONRequest(request, &reply);
   if (!*success) {
     LOG(ERROR) << "Could not send web key event. Reply: " << reply;
   }
 }
 
-void Automation::NavigateToURL(const std::string& url,
+void Automation::NavigateToURL(int tab_id,
+                               const std::string& url,
                                bool* success) {
-  *success = tab_->NavigateToURL(GURL(url));
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
+  *success = tab->NavigateToURL(GURL(url));
 }
 
-void Automation::GoForward(bool* success) {
-  *success = tab_->GoForward();
+void Automation::GoForward(int tab_id, bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
+  *success = tab->GoForward();
 }
 
-void Automation::GoBack(bool* success) {
-  *success = tab_->GoBack();
+void Automation::GoBack(int tab_id, bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
+  *success = tab->GoBack();
 }
 
-void Automation::Reload(bool* success) {
-  *success = tab_->Reload();
+void Automation::Reload(int tab_id, bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
+  *success = tab->Reload();
 }
 
-void Automation::GetURL(std::string* url,
+void Automation::GetURL(int tab_id,
+                        std::string* url,
                         bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
   GURL gurl;
-  *success = tab_->GetCurrentURL(&gurl);
+  *success = tab->GetCurrentURL(&gurl);
   if (*success)
     *url = gurl.possibly_invalid_spec();
 }
 
-void Automation::GetTabTitle(std::string* tab_title,
+void Automation::GetTabTitle(int tab_id,
+                             std::string* tab_title,
                              bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
   std::wstring wide_title;
-  *success = tab_->GetTabTitle(&wide_title);
+  *success = tab->GetTabTitle(&wide_title);
   if (*success)
     *tab_title = WideToUTF8(wide_title);
 }
 
+void Automation::GetTabIds(std::vector<int>* tab_ids,

[Paweł Hajdan Jr., 2011/02/14 09:29:43]

What is this method trying to do, and why? I can infer the first from code, but
the second is still puzzling me a bit (especially why is this _needed_). There
is no comment for this method, neither here or in the header.

[kkania, 2011/02/14 17:42:12]

Will add appropriate comments. This method implements the WebDriver method for
retrieving handles for all the known tabs currently available to the user. The
client can then use these handles to switch between what tab/window they want to
work with.

+                           bool* success) {
+  *success = false;
+  std::vector<scoped_refptr<TabProxy> > tabs;
+  int browser_count = 0;
+  if (!automation()->GetBrowserWindowCount(&browser_count)) {
+    LOG(ERROR) << "Failed to get browser window count";
+    return;
+  }
+  for (int browser_index = 0; browser_index < browser_count; ++browser_index) {
+    scoped_refptr<BrowserProxy> browser =
+        automation()->GetBrowserWindow(browser_index);
+    if (!browser.get())
+      continue;
+    int tab_count = 0;
+    if (!browser->GetTabCount(&tab_count))
+      continue;
+
+    for (int tab_index = 0; tab_index < tab_count; ++tab_index) {
+      scoped_refptr<TabProxy> tab = browser->GetTab(tab_index);
+      if (!tab.get())
+        continue;
+      tabs.push_back(tab);
+    }
+  }
+
+  tabs_ = tabs;

[Paweł Hajdan Jr., 2011/02/14 09:29:43]

I guess bad things happen if a tab gets opened or closed and |tabs_| is not
updated. Have you considered such condition?

[kkania, 2011/02/14 17:42:12]

It is ok if tabs_ does  not get updated. We need to give back handles for all
the tabs in the system at this point in time to the client. If a tab closes, and
the user tells us to do something with that tab, we will simply return an error.
If a new tab is created after we return these, the user has no way of
identifying that tab to us without calling this method again.

+  for (size_t i = 0; i < tabs_.size(); ++i)
+    tab_ids->push_back(tabs_[i]->handle());
+  *success = true;
+}
+
+void Automation::CloseTab(int tab_id, bool* success) {
+  TabProxy* tab = NULL;
+  if (!GetTab(tab_id, &tab, success))
+    return;
+  *success = tab->Close(true);
+}
+
+bool Automation::GetTab(int tab_id, TabProxy** tab, bool* success) {
+  for (size_t i = 0; i < tabs_.size(); ++i) {

[Paweł Hajdan Jr., 2011/02/14 09:29:43]

A linear scan doesn't seem to be an optimal solution here. Can you do better?

[kkania, 2011/02/14 17:42:12]

Will use a map

+    if (tabs_[i]->handle() == tab_id) {
+      *tab = tabs_[i];
+      return true;

[Paweł Hajdan Jr., 2011/02/14 09:29:43]

Now this is weird. We have _both_ a return value and a boolean out parameter,
and sometimes they might not be consistent. I think we should just stick to one
of them, but not both. I generally prefer return values.

[kkania, 2011/02/14 17:42:12]

Yes, this is a bit of a shortcut and can be confusing. I'll just keep to the
return value here.

+    }
+  }
+  *success = false;
+  return false;
+}
+
 }  // namespace webdriver