[chromedriver] Fix race where we might not wait for page load after navigating.
Change the NavigationTracker to assume the page is loading after Page.navigate.
BUG=chromedriver:347
R=chrisgao@chromium.org, craigdh@chromium.org
Review URL: https://codereview.chromium.org/15772009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202651 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index a3a7976..a436d83b 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -814,6 +814,7 @@
'test/chromedriver/chrome/devtools_client.h',
'test/chromedriver/chrome/devtools_client_impl.cc',
'test/chromedriver/chrome/devtools_client_impl.h',
+ 'test/chromedriver/chrome/devtools_event_listener.cc',
'test/chromedriver/chrome/devtools_event_listener.h',
'test/chromedriver/chrome/devtools_http_client.cc',
'test/chromedriver/chrome/devtools_http_client.h',
diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl.cc b/chrome/test/chromedriver/chrome/devtools_client_impl.cc
index 89906ef..4adedbf 100644
--- a/chrome/test/chromedriver/chrome/devtools_client_impl.cc
+++ b/chrome/test/chromedriver/chrome/devtools_client_impl.cc
@@ -178,7 +178,8 @@
return Status(kOk);
}
-DevToolsClientImpl::ResponseInfo::ResponseInfo() : state(kWaiting) {}
+DevToolsClientImpl::ResponseInfo::ResponseInfo(const std::string& method)
+ : state(kWaiting), method(method) {}
DevToolsClientImpl::ResponseInfo::~ResponseInfo() {}
@@ -200,7 +201,8 @@
if (!socket_->Send(message))
return Status(kDisconnected, "unable to send message to renderer");
- linked_ptr<ResponseInfo> response_info = make_linked_ptr(new ResponseInfo());
+ linked_ptr<ResponseInfo> response_info =
+ make_linked_ptr(new ResponseInfo(method));
response_info_map_[command_id] = response_info;
while (response_info->state == kWaiting) {
Status status = ProcessNextMessage(command_id);
@@ -231,6 +233,9 @@
status = EnsureListenersNotifiedOfEvent();
if (status.IsError())
return status;
+ status = EnsureListenersNotifiedOfCommandResponse();
+ if (status.IsError())
+ return status;
// The command response may have already been received or blocked while
// notifying listeners.
@@ -300,15 +305,25 @@
linked_ptr<ResponseInfo> response_info = response_info_map_[response.id];
if (response_info->state == kReceived)
return Status(kUnknownError, "received multiple command responses");
+
if (response_info->state == kIgnored) {
response_info_map_.erase(response.id);
- return Status(kOk);
+ } else {
+ response_info->state = kReceived;
+ response_info->response.id = response.id;
+ response_info->response.error = response.error;
+ if (response.result)
+ response_info->response.result.reset(response.result->DeepCopy());
}
- response_info->state = kReceived;
- response_info->response.id = response.id;
- response_info->response.error = response.error;
- if (response.result)
- response_info->response.result.reset(response.result->DeepCopy());
+
+ if (response.result) {
+ unnotified_cmd_response_listeners_ = listeners_;
+ unnotified_cmd_response_info_ = response_info;
+ Status status = EnsureListenersNotifiedOfCommandResponse();
+ unnotified_cmd_response_info_.reset();
+ if (status.IsError())
+ return status;
+ }
return Status(kOk);
}
@@ -333,6 +348,19 @@
return Status(kOk);
}
+Status DevToolsClientImpl::EnsureListenersNotifiedOfCommandResponse() {
+ while (unnotified_cmd_response_listeners_.size()) {
+ DevToolsEventListener* listener =
+ unnotified_cmd_response_listeners_.front();
+ unnotified_cmd_response_listeners_.pop_front();
+ Status status =
+ listener->OnCommandSuccess(this, unnotified_cmd_response_info_->method);
+ if (status.IsError())
+ return status;
+ }
+ return Status(kOk);
+}
+
namespace internal {
bool ParseInspectorMessage(
diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl.h b/chrome/test/chromedriver/chrome/devtools_client_impl.h
index f46d6b2..e30410ba 100644
--- a/chrome/test/chromedriver/chrome/devtools_client_impl.h
+++ b/chrome/test/chromedriver/chrome/devtools_client_impl.h
@@ -103,10 +103,11 @@
kReceived
};
struct ResponseInfo {
- ResponseInfo();
+ explicit ResponseInfo(const std::string& method);
~ResponseInfo();
ResponseState state;
+ std::string method;
internal::InspectorCommandResponse response;
};
typedef std::map<int, linked_ptr<ResponseInfo> > ResponseInfoMap;
@@ -121,6 +122,7 @@
const internal::InspectorCommandResponse& response);
Status EnsureListenersNotifiedOfConnect();
Status EnsureListenersNotifiedOfEvent();
+ Status EnsureListenersNotifiedOfCommandResponse();
scoped_ptr<SyncWebSocket> socket_;
GURL url_;
@@ -132,6 +134,8 @@
std::list<DevToolsEventListener*> unnotified_connect_listeners_;
std::list<DevToolsEventListener*> unnotified_event_listeners_;
const internal::InspectorEvent* unnotified_event_;
+ std::list<DevToolsEventListener*> unnotified_cmd_response_listeners_;
+ linked_ptr<ResponseInfo> unnotified_cmd_response_info_;
ResponseInfoMap response_info_map_;
int next_id_;
int stack_count_;
diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc b/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc
index 9d1f475..708ca50 100644
--- a/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc
+++ b/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc
@@ -1072,3 +1072,54 @@
msgs.push_back("{\"id\": 6, \"result\": {}}");
ASSERT_EQ(kOk, client.HandleReceivedEvents().code());
}
+
+namespace {
+
+class MockCommandListener : public DevToolsEventListener {
+ public:
+ MockCommandListener() {}
+ virtual ~MockCommandListener() {}
+
+ virtual void OnEvent(DevToolsClient* client,
+ const std::string& method,
+ const base::DictionaryValue& params) OVERRIDE {
+ msgs_.push_back(method);
+ }
+
+ virtual Status OnCommandSuccess(DevToolsClient* client,
+ const std::string& method) OVERRIDE {
+ msgs_.push_back(method);
+ if (!callback_.is_null())
+ callback_.Run(client);
+ return Status(kOk);
+ }
+
+ base::Callback<void(DevToolsClient*)> callback_;
+ std::list<std::string> msgs_;
+};
+
+void HandleReceivedEvents(DevToolsClient* client) {
+ EXPECT_EQ(kOk, client->HandleReceivedEvents().code());
+}
+
+} // namespace
+
+TEST(DevToolsClientImpl, ReceivesCommandResponse) {
+ std::list<std::string> msgs;
+ SyncWebSocketFactory factory = base::Bind(&CreateMockSyncWebSocket6, &msgs);
+ Logger logger;
+ DevToolsClientImpl client(
+ factory, "http://url", "id", base::Bind(&CloserFunc), &logger);
+ MockCommandListener listener1;
+ listener1.callback_ = base::Bind(&HandleReceivedEvents);
+ MockCommandListener listener2;
+ client.AddListener(&listener1);
+ client.AddListener(&listener2);
+ msgs.push_back("{\"id\": 1, \"result\": {}}");
+ msgs.push_back("{\"method\": \"event\", \"params\": {}}");
+ base::DictionaryValue params;
+ ASSERT_EQ(kOk, client.SendCommand("cmd", params).code());
+ ASSERT_EQ(2u, listener2.msgs_.size());
+ ASSERT_EQ("cmd", listener2.msgs_.front());
+ ASSERT_EQ("event", listener2.msgs_.back());
+}
diff --git a/chrome/test/chromedriver/chrome/devtools_event_listener.cc b/chrome/test/chromedriver/chrome/devtools_event_listener.cc
new file mode 100644
index 0000000..30a023e
--- /dev/null
+++ b/chrome/test/chromedriver/chrome/devtools_event_listener.cc
@@ -0,0 +1,23 @@
+// 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/chrome/devtools_event_listener.h"
+
+#include "chrome/test/chromedriver/chrome/status.h"
+
+DevToolsEventListener::~DevToolsEventListener() {}
+
+Status DevToolsEventListener::OnConnected(DevToolsClient* client) {
+ return Status(kOk);
+}
+
+void DevToolsEventListener::OnEvent(DevToolsClient* client,
+ const std::string& method,
+ const base::DictionaryValue& params) {}
+
+Status DevToolsEventListener::OnCommandSuccess(
+ DevToolsClient* client,
+ const std::string& method) {
+ return Status(kOk);
+}
diff --git a/chrome/test/chromedriver/chrome/devtools_event_listener.h b/chrome/test/chromedriver/chrome/devtools_event_listener.h
index 6ce0385..610b7f49 100644
--- a/chrome/test/chromedriver/chrome/devtools_event_listener.h
+++ b/chrome/test/chromedriver/chrome/devtools_event_listener.h
@@ -14,16 +14,23 @@
class DevToolsClient;
class Status;
-// Listens to WebKit Inspector events and DevTools debugger connection.
+// Receives notification of incoming Blink Inspector messages and connection
+// to the DevTools server.
class DevToolsEventListener {
public:
- virtual ~DevToolsEventListener() {}
+ virtual ~DevToolsEventListener();
- virtual Status OnConnected(DevToolsClient* client) = 0;
+ // Called when a connection is made to the DevTools server.
+ virtual Status OnConnected(DevToolsClient* client);
+ // Called when an event is received.
virtual void OnEvent(DevToolsClient* client,
const std::string& method,
- const base::DictionaryValue& params) = 0;
+ const base::DictionaryValue& params);
+
+ // Called when a command success response is received.
+ virtual Status OnCommandSuccess(DevToolsClient* client,
+ const std::string& method);
};
#endif // CHROME_TEST_CHROMEDRIVER_CHROME_DEVTOOLS_EVENT_LISTENER_H_
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.cc b/chrome/test/chromedriver/chrome/navigation_tracker.cc
index cd04f81..509b318c 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.cc
@@ -126,3 +126,10 @@
scheduled_frame_set_.clear();
}
}
+
+Status NavigationTracker::OnCommandSuccess(DevToolsClient* client,
+ const std::string& method) {
+ if (method == "Page.navigate")
+ loading_state_ = kLoading;
+ return Status(kOk);
+}
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.h b/chrome/test/chromedriver/chrome/navigation_tracker.h
index 45c8b98..cd16206 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.h
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.h
@@ -42,6 +42,8 @@
virtual void OnEvent(DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) OVERRIDE;
+ virtual Status OnCommandSuccess(DevToolsClient* client,
+ const std::string& method) OVERRIDE;
private:
DevToolsClient* client_;
diff --git a/chrome/test/chromedriver/chrome/web_view_impl.cc b/chrome/test/chromedriver/chrome/web_view_impl.cc
index 4a25e090..023e111 100644
--- a/chrome/test/chromedriver/chrome/web_view_impl.cc
+++ b/chrome/test/chromedriver/chrome/web_view_impl.cc
@@ -8,6 +8,7 @@
#include "base/files/file_path.h"
#include "base/json/json_writer.h"
#include "base/logging.h"
+#include "base/string_util.h"
#include "base/stringprintf.h"
#include "base/threading/platform_thread.h"
#include "base/time.h"
@@ -111,6 +112,10 @@
}
Status WebViewImpl::Load(const std::string& url) {
+ // Javascript URLs will cause a hang while waiting for the page to stop
+ // loading, so just disallow.
+ if (StartsWithASCII(url, "javascript:", false))
+ return Status(kUnknownError, "unsupported protocol");
base::DictionaryValue params;
params.SetString("url", url);
return client_->SendCommand("Page.navigate", params);
