[chromedriver] Apply page load timeout to slow cross-process navigations

chrome/test/chromedriver/chrome/devtools_client.h

Index: chrome/test/chromedriver/chrome/devtools_client.h
diff --git a/chrome/test/chromedriver/chrome/devtools_client.h b/chrome/test/chromedriver/chrome/devtools_client.h
index 884ec9e420fcd12d1f0300d17994ab2cdd725e8d..277405d8a3ae82a86f8ca02dfbfca509f3450204 100644
--- a/chrome/test/chromedriver/chrome/devtools_client.h
+++ b/chrome/test/chromedriver/chrome/devtools_client.h
@@ -12,10 +12,10 @@
 
 namespace base {
 class DictionaryValue;
-class TimeDelta;
 }
 
 class DevToolsEventListener;
+class Timeout;
 class Status;
 
 // A DevTools client of a single DevTools debugger.
@@ -36,6 +36,11 @@ class DevToolsClient {
       const std::string& method,
       const base::DictionaryValue& params) = 0;
 
+  virtual Status SendCommandWithTimeout(
+      const std::string& method,
+      const base::DictionaryValue& params,
+      const Timeout* timeout);
+

[samuong, 2016/04/13 03:59:30]

Once we're done with the subsequent CLs, will every call to SendCommand()
replaced with SendCommandWithTimeout()?

[Alexander Semashko, 2016/04/13 15:51:35]

Most likely no. Of course we can go forward and introduce timeouts everywhere,
but it needs separate discussions/decisions. E.g. what timeout should we use for
Runtime.enable?
It is unclear to me what were the reasons for current design and what would be
desired to have.

[samuong, 2016/04/15 04:23:55]

Ok, that makes sense. I'm not sure which part of the design you're unclear on
but we can discuss this later outside of the CL if you like.

Can we keep this class pure virtual? Just define the *WithTimeout() functions
like this in StubDevToolsClient and delete devtools_client.cc from this CL.

[Alexander Semashko, 2016/04/15 15:11:48]

I'm thinking of the 10 minute timeout in SendCommandInternal. Each message
coming from the browser restarts the timeout, so if there are periodically
coming events (is it possible? smth like "DOM changed"), we may never return. Is
it intended to be so? Or did I misread something?
Done.

   virtual Status SendAsyncCommand(
       const std::string& method,
       const base::DictionaryValue& params) = 0;
@@ -45,6 +50,12 @@ class DevToolsClient {
       const base::DictionaryValue& params,
       scoped_ptr<base::DictionaryValue>* result) = 0;
 
+  virtual Status SendCommandAndGetResultWithTimeout(
+      const std::string& method,
+      const base::DictionaryValue& params,
+      const Timeout* timeout,
+      scoped_ptr<base::DictionaryValue>* result);
+
   // Adds a listener. This must only be done when the client is disconnected.
   virtual void AddListener(DevToolsEventListener* listener) = 0;
 
@@ -54,7 +65,7 @@ class DevToolsClient {
   // If the condition is not met within |timeout|, kTimeout status
   // is returned eventually. If |timeout| is 0, this function will not block.
   virtual Status HandleEventsUntil(const ConditionalFunc& conditional_func,
-                                   const base::TimeDelta& timeout) = 0;
+                                   const Timeout& timeout) = 0;
 
   // Handles events that have been received but not yet handled.
   virtual Status HandleReceivedEvents() = 0;