Add a method to override the network conditions of the ChromeDriver session.

chrome/test/chromedriver/session_commands.cc

 // Copyright (c) 2013 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/chromedriver/session_commands.h"
 
 #include <list>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 20 matching lines @@
 #include "chrome/test/chromedriver/chrome_launcher.h"
 #include "chrome/test/chromedriver/command_listener.h"
 #include "chrome/test/chromedriver/logging.h"
 #include "chrome/test/chromedriver/net/url_request_context_getter.h"
 #include "chrome/test/chromedriver/session.h"
 #include "chrome/test/chromedriver/util.h"
 #include "chrome/test/chromedriver/version.h"
 
 namespace {
 
+
+

[samuong, 2016/06/27 21:31:53]

nit: delete these two blank lines

[roisinmcl, 2016/06/28 18:36:48]

Done.

+const int kAirplaneModeMask = 0x1;
+const int kWifiMask = 0x2;
+const int k4GMask = 0x8;
+const int k3GMask = 0x10;
+const int k2GMask = 0x20;
+
+const int kAirplaneModeLatency = 0;
+const int kAirplaneModeThroughput = 0;
+const int kWifiLatency = 2;
+const int kWifiThroughput = 30720 * 1024;
+const int k4GLatency = 20;
+const int k4GThroughput = 4096 * 1024;
+const int k3GLatency = 100;
+const int k3GThroughput = 750 * 1024;
+const int k2GLatency = 300;
+const int k2GThroughput = 250 * 1024;
+
+const bool kOnline = false;
+const bool kOffline = true;

[samuong, 2016/06/27 21:31:53]

i don't think these are necessary, saying "offline = true" or "offline = false"
fine

[roisinmcl, 2016/06/28 18:36:48]

Done.

+
 const char kWindowHandlePrefix[] = "CDwindow-";
 
 std::string WebViewIdToWindowHandle(const std::string& web_view_id) {
   return kWindowHandlePrefix + web_view_id;
 }
 
 bool WindowHandleToWebViewId(const std::string& window_handle,
                              std::string* web_view_id) {
   if (window_handle.find(kWindowHandlePrefix) != 0u)
     return false;
@@ skipping 447 matching lines @@
   conditions.SetInteger(
       "download_throughput",
       session->overridden_network_conditions->download_throughput);
   conditions.SetInteger(
       "upload_throughput",
       session->overridden_network_conditions->upload_throughput);
   value->reset(conditions.DeepCopy());
   return Status(kOk);
 }
 
+Status ExecuteSetNetworkConnection(Session* session,
+                                   const base::DictionaryValue& params,
+                                   std::unique_ptr<base::Value>* value) {
+  int connection_type;
+  const base::DictionaryValue* parameters = nullptr;
+
+  params.GetDictionary("parameters", &parameters);

[samuong, 2016/06/27 21:31:53]

the return value for this call is not checked, and if it fails we won't notice
and the next line will segfault

you can actually use GetInteger("parameters.type", &connection_type) instead,
which avoids the need for two separate calls

[roisinmcl, 2016/06/29 17:55:12]

Done.

+  if (!parameters->GetInteger("type", &connection_type)) {
+    return Status(kUnknownError, "invalid connection_type");
+  }
+
+  ChromeDesktopImpl* desktop = nullptr;
+  Status status = session->chrome->GetAsDesktop(&desktop);
+  if (!desktop) {

[samuong, 2016/06/27 21:31:53]

for consistency, use "if (status.IsError())"
also a nit: this if statement only has one line, so we don't need braces

[roisinmcl, 2016/06/28 18:36:48]

Done.

+    return status;
+  }
+  desktop.SetNetworkConnection(connection_type);
+
+  std::unique_ptr<NetworkConditions> network_conditions(
+      new NetworkConditions());
+
+  if (connection_type & kWifiMask) {
+    network_conditions->latency = kWifiLatency;
+    network_conditions->upload_throughput = kWifiThroughput;
+    network_conditions->download_throughput = kWifiThroughput;
+    network_conditions->offline = kOnline;
+  } else if (connection_type & k4GMask) {
+    network_conditions->latency = k4GLatency;
+    network_conditions->upload_throughput = k4GThroughput;
+    network_conditions->download_throughput = k4GThroughput;
+    network_conditions->offline = kOnline;
+  } else if (connection_type & k3GMask) {
+    network_conditions->latency = k3GLatency;
+    network_conditions->upload_throughput = k3GThroughput;
+    network_conditions->download_throughput = k3GThroughput;
+    network_conditions->offline = kOnline;
+  } else if (connection_type & k2GMask) {
+    network_conditions->latency = k2GLatency;
+    network_conditions->upload_throughput = k2GThroughput;
+    network_conditions->download_throughput = k2GThroughput;
+    network_conditions->offline = kOnline;
+  } else if (connection_type & kAirplaneModeMask) {
+    network_conditions->latency = kAirplaneModeLatency;
+    network_conditions->upload_throughput = kAirplaneModeThroughput;
+    network_conditions->download_throughput = kAirplaneModeThroughput;

[samuong, 2016/06/27 21:31:53]

do we actually need to set latency and throughput when we've already set
offline=true?

[roisinmcl, 2016/06/28 18:36:48]

I left these being set to zero in case the latency or throughput was ever
accessed in another function. However if these aren't being used when
offline=true then I can get rid of them.

[samuong, 2016/06/28 23:23:40]

It's probably fine either way. Since this if branch, and the else branch below
it, are both doing the same thing, we should probably combine them.

[roisinmcl, 2016/06/29 17:55:12]

Done.

+    network_conditions->offline = kOffline;
+  } else {
+    return Status(kUnknownError, "invalid 'connection_type'");

[samuong, 2016/06/27 21:31:53]

should this be an error? if i've got airplane mode off, and also have wifi and
data switched off, my phone won't be connected. maybe just set it to offline in
this case?

[roisinmcl, 2016/06/28 18:36:48]

Done.

+  }
+
+  session->overridden_network_conditions.reset(
+     network_conditions.release());
+
+  // Applies overridden network connection to all WebViews of the session
+  // to ensure that network emulation is applied on a per-session basis
+  // rather than the just to the current WebView.
+  std::list<std::string> web_view_ids;
+  session->chrome->GetWebViewIds(&web_view_ids);

[samuong, 2016/06/27 21:31:53]

don't forget to check the error status

[roisinmcl, 2016/06/28 18:36:48]

Done.

+  std::list<std::string>::const_iterator web_view_itr;
+  std::list<std::string>::const_iterator end;
+
+  for (web_view_itr = web_view_ids.begin(), end = web_view_ids.end();
+       web_view_itr != end;
+       ++web_view_itr) {

[samuong, 2016/06/27 21:31:53]

you can use a range-based for loop here
for an example, see http://en.cppreference.com/w/cpp/language/range-for

[roisinmcl, 2016/06/28 18:36:48]

Done.

+    std::string web_view_id = *web_view_itr;
+    WebView* web_view;
+    session->chrome->GetWebViewById(web_view_id, &web_view);
+    if (!web_view) {
+      return Status(kUnknownError, "invalid web view");
+    }

[samuong, 2016/06/27 21:31:53]

nit: remove braces for this, and any other single-line blocks

[roisinmcl, 2016/06/28 18:36:48]

Done.

+    web_view->OverrideNetworkConditions(
+        *session->overridden_network_conditions);
+  }
+  return Status(kOk);
+}
+
 Status ExecuteGetWindowPosition(Session* session,
                                 const base::DictionaryValue& params,
                                 std::unique_ptr<base::Value>* value) {
   ChromeDesktopImpl* desktop = NULL;
   Status status = session->chrome->GetAsDesktop(&desktop);
   if (status.IsError())
     return status;
 
   AutomationExtension* extension = NULL;
   status = desktop->GetAutomationExtension(&extension);
@@ skipping 169 matching lines @@
 
 Status ExecuteSetAutoReporting(Session* session,
                                const base::DictionaryValue& params,
                                std::unique_ptr<base::Value>* value) {
   bool enabled;
   if (!params.GetBoolean("enabled", &enabled))
     return Status(kUnknownError, "missing parameter 'enabled'");
   session->auto_reporting_enabled = enabled;
   return Status(kOk);
 }