diff options
25 files changed, 244 insertions, 172 deletions
diff --git a/chrome/test/chromedriver/chrome/console_logger.cc b/chrome/test/chromedriver/chrome/console_logger.cc index 71b1a79..5f7dabb 100644 --- a/chrome/test/chromedriver/chrome/console_logger.cc +++ b/chrome/test/chromedriver/chrome/console_logger.cc @@ -40,12 +40,11 @@ Status ConsoleLogger::OnConnected(DevToolsClient* client) { return client->SendCommand("Console.enable", params); } -void ConsoleLogger::OnEvent( - DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) { +Status ConsoleLogger::OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { if (!StartsWithASCII(method, "Console.messageAdded", true)) - return; + return Status(kOk); // If the event has proper structure and fields, log formatted. // Else it's a weird message that we don't know how to format, log full JSON. @@ -83,7 +82,7 @@ void ConsoleLogger::OnEvent( if (message_dict->GetString("level", &level_name)) { if (ConsoleLevelToLogLevel(level_name, &level)) { log_->AddEntry(level, message.str()); // Found all expected fields. - return; + return Status(kOk); } } } @@ -93,4 +92,5 @@ void ConsoleLogger::OnEvent( std::string message_json; base::JSONWriter::Write(¶ms, &message_json); log_->AddEntry(Log::kWarning, message_json); + return Status(kOk); } diff --git a/chrome/test/chromedriver/chrome/console_logger.h b/chrome/test/chromedriver/chrome/console_logger.h index de09268..4d0bdcb 100644 --- a/chrome/test/chromedriver/chrome/console_logger.h +++ b/chrome/test/chromedriver/chrome/console_logger.h @@ -22,9 +22,9 @@ class ConsoleLogger : public DevToolsEventListener { explicit ConsoleLogger(Log* log); virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; private: Log* log_; diff --git a/chrome/test/chromedriver/chrome/console_logger_unittest.cc b/chrome/test/chromedriver/chrome/console_logger_unittest.cc index deae61b..eb8e9ac 100644 --- a/chrome/test/chromedriver/chrome/console_logger_unittest.cc +++ b/chrome/test/chromedriver/chrome/console_logger_unittest.cc @@ -30,9 +30,9 @@ class FakeDevToolsClient : public StubDevToolsClient { return command; } - void TriggerEvent(const std::string& method, + Status TriggerEvent(const std::string& method, const base::DictionaryValue& params) { - listener_->OnEvent(this, method, params); + return listener_->OnEvent(this, method, params); } // Overridden from DevToolsClient: @@ -130,36 +130,37 @@ TEST(ConsoleLogger, ConsoleMessages) { base::DictionaryValue params1; // All fields are set. ConsoleLogParams(¶ms1, "source1", "url1", "debug", 10, 1, "text1"); - client.TriggerEvent("Console.messageAdded", params1); - client.TriggerEvent("Console.gaga", params1); // Ignored -- wrong method. + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params1).code()); + // Ignored -- wrong method. + ASSERT_EQ(kOk, client.TriggerEvent("Console.gaga", params1).code()); base::DictionaryValue params2; // All optionals are not set. ConsoleLogParams(¶ms2, "source2", NULL, "log", -1, -1, "text2"); - client.TriggerEvent("Console.messageAdded", params2); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params2).code()); base::DictionaryValue params3; // Line without column, no source. ConsoleLogParams(¶ms3, NULL, "url3", "warning", 30, -1, "text3"); - client.TriggerEvent("Console.messageAdded", params3); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params3).code()); base::DictionaryValue params4; // Column without line. ConsoleLogParams(¶ms4, "source4", "url4", "error", -1, 4, "text4"); - client.TriggerEvent("Console.messageAdded", params4); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params4).code()); base::DictionaryValue params5; // Bad level name. ConsoleLogParams(¶ms5, "source5", "url5", "gaga", 50, 5, "ulala"); - client.TriggerEvent("Console.messageAdded", params5); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params5).code()); base::DictionaryValue params6; // Unset level. ConsoleLogParams(¶ms6, "source6", "url6", NULL, 60, 6, NULL); - client.TriggerEvent("Console.messageAdded", params6); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params6).code()); base::DictionaryValue params7; // No text. ConsoleLogParams(¶ms7, "source7", "url7", "log", -1, -1, NULL); - client.TriggerEvent("Console.messageAdded", params7); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params7).code()); base::DictionaryValue params8; // No message object. params8.SetInteger("gaga", 8); - client.TriggerEvent("Console.messageAdded", params8); + ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params8).code()); EXPECT_STREQ("", client.PopSentCommand().c_str()); // No other commands sent. diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl.cc b/chrome/test/chromedriver/chrome/devtools_client_impl.cc index 4adedbf..ff18dfd 100644 --- a/chrome/test/chromedriver/chrome/devtools_client_impl.cc +++ b/chrome/test/chromedriver/chrome/devtools_client_impl.cc @@ -152,7 +152,7 @@ Status DevToolsClientImpl::SendCommandAndGetResult( } void DevToolsClientImpl::AddListener(DevToolsEventListener* listener) { - DCHECK(listener); + CHECK(listener); listeners_.push_back(listener); } @@ -342,8 +342,10 @@ Status DevToolsClientImpl::EnsureListenersNotifiedOfEvent() { while (unnotified_event_listeners_.size()) { DevToolsEventListener* listener = unnotified_event_listeners_.front(); unnotified_event_listeners_.pop_front(); - listener->OnEvent(this, - unnotified_event_->method, *unnotified_event_->params); + Status status = listener->OnEvent( + this, unnotified_event_->method, *unnotified_event_->params); + if (status.IsError()) + return status; } return Status(kOk); } diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc b/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc index 708ca50..f11490a 100644 --- a/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc +++ b/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc @@ -344,12 +344,13 @@ class MockListener : public DevToolsEventListener { return Status(kOk); } - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE { + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE { called_ = true; EXPECT_STREQ("method", method.c_str()); EXPECT_TRUE(params.HasKey("key")); + return Status(kOk); } private: @@ -688,13 +689,14 @@ class OnConnectedListener : public DevToolsEventListener { return client_->SendCommand(method_, params); } - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE { + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE { EXPECT_EQ(client_, client); EXPECT_STREQ("onconnected-id", client->GetId().c_str()); EXPECT_TRUE(on_connected_called_); on_event_called_ = true; + return Status(kOk); } private: @@ -846,10 +848,11 @@ class OtherEventListener : public DevToolsEventListener { virtual Status OnConnected(DevToolsClient* client) OVERRIDE { return Status(kOk); } - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE { + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE { received_event_ = true; + return Status(kOk); } bool received_event_; @@ -868,12 +871,13 @@ class OnEventListener : public DevToolsEventListener { return Status(kOk); } - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE { - ASSERT_EQ(client_, client); + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE { + EXPECT_EQ(client_, client); client_->SendCommand("method", params); EXPECT_TRUE(other_listener_->received_event_); + return Status(kOk); } private: @@ -995,9 +999,9 @@ class MockDevToolsEventListener : public DevToolsEventListener { return Status(kOk); } - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE { + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE { id_++; Status status = client->SendCommand("hello", params); id_--; @@ -1006,6 +1010,7 @@ class MockDevToolsEventListener : public DevToolsEventListener { } else { EXPECT_EQ(kOk, status.code()); } + return Status(kOk); } private: @@ -1080,10 +1085,11 @@ class MockCommandListener : public DevToolsEventListener { MockCommandListener() {} virtual ~MockCommandListener() {} - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE { + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE { msgs_.push_back(method); + return Status(kOk); } virtual Status OnCommandSuccess(DevToolsClient* client, diff --git a/chrome/test/chromedriver/chrome/devtools_event_listener.cc b/chrome/test/chromedriver/chrome/devtools_event_listener.cc index 30a023e..00d4487 100644 --- a/chrome/test/chromedriver/chrome/devtools_event_listener.cc +++ b/chrome/test/chromedriver/chrome/devtools_event_listener.cc @@ -12,9 +12,11 @@ Status DevToolsEventListener::OnConnected(DevToolsClient* client) { return Status(kOk); } -void DevToolsEventListener::OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) {} +Status DevToolsEventListener::OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { + return Status(kOk); +} Status DevToolsEventListener::OnCommandSuccess( DevToolsClient* client, diff --git a/chrome/test/chromedriver/chrome/devtools_event_listener.h b/chrome/test/chromedriver/chrome/devtools_event_listener.h index 610b7f4..8060fb8 100644 --- a/chrome/test/chromedriver/chrome/devtools_event_listener.h +++ b/chrome/test/chromedriver/chrome/devtools_event_listener.h @@ -24,9 +24,9 @@ class DevToolsEventListener { virtual Status OnConnected(DevToolsClient* client); // Called when an event is received. - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params); + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params); // Called when a command success response is received. virtual Status OnCommandSuccess(DevToolsClient* client, diff --git a/chrome/test/chromedriver/chrome/dom_tracker.cc b/chrome/test/chromedriver/chrome/dom_tracker.cc index 31eb046..40f2f88 100644 --- a/chrome/test/chromedriver/chrome/dom_tracker.cc +++ b/chrome/test/chromedriver/chrome/dom_tracker.cc @@ -7,13 +7,11 @@ #include <utility> #include "base/json/json_writer.h" -#include "base/logging.h" #include "base/values.h" #include "chrome/test/chromedriver/chrome/devtools_client.h" #include "chrome/test/chromedriver/chrome/status.h" DomTracker::DomTracker(DevToolsClient* client) { - DCHECK(client); client->AddListener(this); } @@ -35,36 +33,37 @@ Status DomTracker::OnConnected(DevToolsClient* client) { return client->SendCommand("DOM.getDocument", params); } -void DomTracker::OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) { +Status DomTracker::OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { if (method == "DOM.setChildNodes") { const base::Value* nodes; - if (!params.Get("nodes", &nodes)) { - LOG(ERROR) << "DOM.setChildNodes missing 'nodes'"; - return; - } + if (!params.Get("nodes", &nodes)) + return Status(kUnknownError, "DOM.setChildNodes missing 'nodes'"); + if (!ProcessNodeList(nodes)) { std::string json; base::JSONWriter::Write(nodes, &json); - LOG(ERROR) << "DOM.setChildNodes has invalid 'nodes': " << json; + return Status(kUnknownError, + "DOM.setChildNodes has invalid 'nodes': " + json); } } else if (method == "DOM.childNodeInserted") { const base::Value* node; - if (!params.Get("node", &node)) { - LOG(ERROR) << "DOM.childNodeInserted missing 'node'"; - return; - } + if (!params.Get("node", &node)) + return Status(kUnknownError, "DOM.childNodeInserted missing 'node'"); + if (!ProcessNode(node)) { std::string json; base::JSONWriter::Write(node, &json); - LOG(ERROR) << "DOM.childNodeInserted has invalid 'node': " << json; + return Status(kUnknownError, + "DOM.childNodeInserted has invalid 'node': " + json); } } else if (method == "DOM.documentUpdated") { node_to_frame_map_.clear(); base::DictionaryValue params; client->SendCommand("DOM.getDocument", params); } + return Status(kOk); } bool DomTracker::ProcessNodeList(const base::Value* nodes) { diff --git a/chrome/test/chromedriver/chrome/dom_tracker.h b/chrome/test/chromedriver/chrome/dom_tracker.h index fdccb55..4518f4e 100644 --- a/chrome/test/chromedriver/chrome/dom_tracker.h +++ b/chrome/test/chromedriver/chrome/dom_tracker.h @@ -30,9 +30,9 @@ class DomTracker : public DevToolsEventListener { // Overridden from DevToolsEventListener: virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; private: bool ProcessNodeList(const base::Value* nodes); diff --git a/chrome/test/chromedriver/chrome/dom_tracker_unittest.cc b/chrome/test/chromedriver/chrome/dom_tracker_unittest.cc index d7684924..ca6e319 100644 --- a/chrome/test/chromedriver/chrome/dom_tracker_unittest.cc +++ b/chrome/test/chromedriver/chrome/dom_tracker_unittest.cc @@ -61,13 +61,14 @@ TEST(DomTracker, GetFrameIdForNode) { "}]"; base::DictionaryValue params; params.Set("nodes", base::JSONReader::Read(nodes)); - tracker.OnEvent(&client, "DOM.setChildNodes", params); + ASSERT_EQ(kOk, tracker.OnEvent(&client, "DOM.setChildNodes", params).code()); ASSERT_TRUE(tracker.GetFrameIdForNode(101, &frame_id).IsError()); ASSERT_TRUE(frame_id.empty()); ASSERT_TRUE(tracker.GetFrameIdForNode(102, &frame_id).IsOk()); ASSERT_STREQ("f", frame_id.c_str()); - tracker.OnEvent(&client, "DOM.documentUpdated", params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "DOM.documentUpdated", params).code()); ASSERT_TRUE(tracker.GetFrameIdForNode(102, &frame_id).IsError()); ASSERT_STREQ("DOM.getDocument", client.PopSentCommand().c_str()); } @@ -79,14 +80,16 @@ TEST(DomTracker, ChildNodeInserted) { base::DictionaryValue params; params.Set("node", base::JSONReader::Read("{\"nodeId\":1}")); - tracker.OnEvent(&client, "DOM.childNodeInserted", params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "DOM.childNodeInserted", params).code()); ASSERT_TRUE(tracker.GetFrameIdForNode(1, &frame_id).IsError()); ASSERT_TRUE(frame_id.empty()); params.Clear(); params.Set("node", base::JSONReader::Read( "{\"nodeId\":2,\"frameId\":\"f\"}")); - tracker.OnEvent(&client, "DOM.childNodeInserted", params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "DOM.childNodeInserted", params).code()); ASSERT_TRUE(tracker.GetFrameIdForNode(2, &frame_id).IsOk()); ASSERT_STREQ("f", frame_id.c_str()); } diff --git a/chrome/test/chromedriver/chrome/frame_tracker.cc b/chrome/test/chromedriver/chrome/frame_tracker.cc index 3e7535f..c18b948 100644 --- a/chrome/test/chromedriver/chrome/frame_tracker.cc +++ b/chrome/test/chromedriver/chrome/frame_tracker.cc @@ -7,13 +7,11 @@ #include <utility> #include "base/json/json_writer.h" -#include "base/logging.h" #include "base/values.h" #include "chrome/test/chromedriver/chrome/devtools_client.h" #include "chrome/test/chromedriver/chrome/status.h" FrameTracker::FrameTracker(DevToolsClient* client) { - DCHECK(client); client->AddListener(this); } @@ -39,24 +37,25 @@ Status FrameTracker::OnConnected(DevToolsClient* client) { return client->SendCommand("Page.enable", params); } -void FrameTracker::OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) { +Status FrameTracker::OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { if (method == "Runtime.executionContextCreated") { const base::DictionaryValue* context; if (!params.GetDictionary("context", &context)) { - LOG(ERROR) << "Runtime.executionContextCreated missing dict 'context'"; - return; + return Status(kUnknownError, + "Runtime.executionContextCreated missing dict 'context'"); } + int context_id; std::string frame_id; if (!context->GetInteger("id", &context_id) || !context->GetString("frameId", &frame_id)) { std::string json; base::JSONWriter::Write(context, &json); - LOG(ERROR) << "Runtime.executionContextCreated has invalid 'context': " - << json; - return; + return Status( + kUnknownError, + "Runtime.executionContextCreated has invalid 'context': " + json); } frame_to_context_map_[frame_id] = context_id; } else if (method == "Page.frameNavigated") { @@ -64,4 +63,5 @@ void FrameTracker::OnEvent(DevToolsClient* client, if (!params.Get("frame.parentId", &unused_value)) frame_to_context_map_.clear(); } + return Status(kOk); } diff --git a/chrome/test/chromedriver/chrome/frame_tracker.h b/chrome/test/chromedriver/chrome/frame_tracker.h index 41415e6..436c006 100644 --- a/chrome/test/chromedriver/chrome/frame_tracker.h +++ b/chrome/test/chromedriver/chrome/frame_tracker.h @@ -31,9 +31,9 @@ class FrameTracker : public DevToolsEventListener { // Overridden from DevToolsEventListener: virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; private: std::map<std::string, int> frame_to_context_map_; diff --git a/chrome/test/chromedriver/chrome/frame_tracker_unittest.cc b/chrome/test/chromedriver/chrome/frame_tracker_unittest.cc index df3302e..f79dcc4 100644 --- a/chrome/test/chromedriver/chrome/frame_tracker_unittest.cc +++ b/chrome/test/chromedriver/chrome/frame_tracker_unittest.cc @@ -21,7 +21,9 @@ TEST(FrameTracker, GetContextIdForFrame) { const char context[] = "{\"id\":100,\"frameId\":\"f\"}"; base::DictionaryValue params; params.Set("context", base::JSONReader::Read(context)); - tracker.OnEvent(&client, "Runtime.executionContextCreated", params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "Runtime.executionContextCreated", params) + .code()); ASSERT_EQ(kNoSuchExecutionContext, tracker.GetContextIdForFrame("foo", &context_id).code()); ASSERT_EQ(-1, context_id); @@ -30,10 +32,12 @@ TEST(FrameTracker, GetContextIdForFrame) { base::DictionaryValue nav_params; nav_params.SetString("frame.parentId", "1"); - tracker.OnEvent(&client, "Page.frameNavigated", nav_params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "Page.frameNavigated", nav_params).code()); ASSERT_TRUE(tracker.GetContextIdForFrame("f", &context_id).IsOk()); nav_params.Clear(); - tracker.OnEvent(&client, "Page.frameNavigated", nav_params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "Page.frameNavigated", nav_params).code()); ASSERT_EQ(kNoSuchExecutionContext, tracker.GetContextIdForFrame("f", &context_id).code()); } @@ -45,13 +49,17 @@ TEST(FrameTracker, CanUpdateFrameContextId) { const char context[] = "{\"id\":1,\"frameId\":\"f\"}"; base::DictionaryValue params; params.Set("context", base::JSONReader::Read(context)); - tracker.OnEvent(&client, "Runtime.executionContextCreated", params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "Runtime.executionContextCreated", params) + .code()); int context_id = -1; ASSERT_TRUE(tracker.GetContextIdForFrame("f", &context_id).IsOk()); ASSERT_EQ(1, context_id); params.SetInteger("context.id", 2); - tracker.OnEvent(&client, "Runtime.executionContextCreated", params); + ASSERT_EQ(kOk, + tracker.OnEvent(&client, "Runtime.executionContextCreated", params) + .code()); ASSERT_TRUE(tracker.GetContextIdForFrame("f", &context_id).IsOk()); ASSERT_EQ(2, context_id); } diff --git a/chrome/test/chromedriver/chrome/geolocation_override_manager.cc b/chrome/test/chromedriver/chrome/geolocation_override_manager.cc index 45de85d..408ad17 100644 --- a/chrome/test/chromedriver/chrome/geolocation_override_manager.cc +++ b/chrome/test/chromedriver/chrome/geolocation_override_manager.cc @@ -4,7 +4,6 @@ #include "chrome/test/chromedriver/chrome/geolocation_override_manager.h" -#include "base/logging.h" #include "base/values.h" #include "chrome/test/chromedriver/chrome/devtools_client.h" #include "chrome/test/chromedriver/chrome/geoposition.h" @@ -28,18 +27,16 @@ Status GeolocationOverrideManager::OnConnected(DevToolsClient* client) { return ApplyOverrideIfNeeded(); } -void GeolocationOverrideManager::OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) { +Status GeolocationOverrideManager::OnEvent( + DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { if (method == "Page.frameNavigated") { const base::Value* unused_value; - if (!params.Get("frame.parentId", &unused_value)) { - Status status = ApplyOverrideIfNeeded(); - if (status.IsError()) { - LOG(ERROR) << "Unable to apply override: " << status.message(); - } - } + if (!params.Get("frame.parentId", &unused_value)) + return ApplyOverrideIfNeeded(); } + return Status(kOk); } Status GeolocationOverrideManager::ApplyOverrideIfNeeded() { diff --git a/chrome/test/chromedriver/chrome/geolocation_override_manager.h b/chrome/test/chromedriver/chrome/geolocation_override_manager.h index 4778266..0ce6353 100644 --- a/chrome/test/chromedriver/chrome/geolocation_override_manager.h +++ b/chrome/test/chromedriver/chrome/geolocation_override_manager.h @@ -31,9 +31,9 @@ class GeolocationOverrideManager : public DevToolsEventListener { // Overridden from DevToolsEventListener: virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; private: Status ApplyOverrideIfNeeded(); diff --git a/chrome/test/chromedriver/chrome/geolocation_override_manager_unittest.cc b/chrome/test/chromedriver/chrome/geolocation_override_manager_unittest.cc index 371eb94..b5e668d 100644 --- a/chrome/test/chromedriver/chrome/geolocation_override_manager_unittest.cc +++ b/chrome/test/chromedriver/chrome/geolocation_override_manager_unittest.cc @@ -102,19 +102,25 @@ TEST(GeolocationOverrideManager, SendsCommandOnNavigation) { RecorderDevToolsClient client; GeolocationOverrideManager manager(&client); base::DictionaryValue main_frame_params; - manager.OnEvent(&client, "Page.frameNavigated", main_frame_params); + ASSERT_EQ(kOk, + manager.OnEvent(&client, "Page.frameNavigated", main_frame_params) + .code()); ASSERT_EQ(0u, client.commands_.size()); Geoposition geoposition = {1, 2, 3}; manager.OverrideGeolocation(geoposition); ASSERT_EQ(1u, client.commands_.size()); - manager.OnEvent(&client, "Page.frameNavigated", main_frame_params); + ASSERT_EQ(kOk, + manager.OnEvent(&client, "Page.frameNavigated", main_frame_params) + .code()); ASSERT_EQ(2u, client.commands_.size()); ASSERT_NO_FATAL_FAILURE( AssertGeolocationCommand(client.commands_[1], geoposition)); base::DictionaryValue sub_frame_params; sub_frame_params.SetString("frame.parentId", "id"); - manager.OnEvent(&client, "Page.frameNavigated", sub_frame_params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.frameNavigated", sub_frame_params).code()); ASSERT_EQ(2u, client.commands_.size()); } diff --git a/chrome/test/chromedriver/chrome/javascript_dialog_manager.cc b/chrome/test/chromedriver/chrome/javascript_dialog_manager.cc index 34d02bc..a0fcd93 100644 --- a/chrome/test/chromedriver/chrome/javascript_dialog_manager.cc +++ b/chrome/test/chromedriver/chrome/javascript_dialog_manager.cc @@ -4,7 +4,6 @@ #include "chrome/test/chromedriver/chrome/javascript_dialog_manager.h" -#include "base/logging.h" #include "base/values.h" #include "chrome/test/chromedriver/chrome/devtools_client.h" #include "chrome/test/chromedriver/chrome/status.h" @@ -55,18 +54,19 @@ Status JavaScriptDialogManager::OnConnected(DevToolsClient* client) { return client_->SendCommand("Page.enable", params); } -void JavaScriptDialogManager::OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) { +Status JavaScriptDialogManager::OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { if (method == "Page.javascriptDialogOpening") { std::string message; - if (!params.GetString("message", &message)) { - LOG(ERROR) << "dialog event missing or invalid 'message'"; - } + if (!params.GetString("message", &message)) + return Status(kUnknownError, "dialog event missing or invalid 'message'"); + unhandled_dialog_queue_.push_back(message); } else if (method == "Page.javascriptDialogClosing") { // Inspector only sends this event when all dialogs have been closed. // Clear the unhandled queue in case the user closed a dialog manually. unhandled_dialog_queue_.clear(); } + return Status(kOk); } diff --git a/chrome/test/chromedriver/chrome/javascript_dialog_manager.h b/chrome/test/chromedriver/chrome/javascript_dialog_manager.h index 1caee31..f5cf389 100644 --- a/chrome/test/chromedriver/chrome/javascript_dialog_manager.h +++ b/chrome/test/chromedriver/chrome/javascript_dialog_manager.h @@ -33,9 +33,9 @@ class JavaScriptDialogManager : public DevToolsEventListener { // Overridden from DevToolsEventListener: virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; private: DevToolsClient* client_; diff --git a/chrome/test/chromedriver/chrome/javascript_dialog_manager_unittest.cc b/chrome/test/chromedriver/chrome/javascript_dialog_manager_unittest.cc index 894bcfa..4874784 100644 --- a/chrome/test/chromedriver/chrome/javascript_dialog_manager_unittest.cc +++ b/chrome/test/chromedriver/chrome/javascript_dialog_manager_unittest.cc @@ -51,7 +51,9 @@ TEST(JavaScriptDialogManager, HandleDialogPassesParams) { JavaScriptDialogManager manager(&client); base::DictionaryValue params; params.SetString("message", "hi"); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); std::string given_text("text"); ASSERT_EQ(kOk, manager.HandleDialog(false, &given_text).code()); std::string text; @@ -65,7 +67,9 @@ TEST(JavaScriptDialogManager, HandleDialogNullPrompt) { JavaScriptDialogManager manager(&client); base::DictionaryValue params; params.SetString("message", "hi"); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); ASSERT_EQ(kOk, manager.HandleDialog(false, NULL).code()); ASSERT_FALSE(client.params_.HasKey("promptText")); ASSERT_TRUE(client.params_.HasKey("accept")); @@ -76,7 +80,9 @@ TEST(JavaScriptDialogManager, ReconnectClearsStateAndSendsEnable) { JavaScriptDialogManager manager(&client); base::DictionaryValue params; params.SetString("message", "hi"); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); ASSERT_TRUE(manager.IsDialogOpen()); std::string message; ASSERT_EQ(kOk, manager.GetDialogMessage(&message).code()); @@ -106,7 +112,10 @@ class FakeDevToolsClient : public StubDevToolsClient { scoped_ptr<base::DictionaryValue>* result) OVERRIDE { while (closing_count_ > 0) { base::DictionaryValue empty; - listener_->OnEvent(this, "Page.javascriptDialogClosing", empty); + Status status = + listener_->OnEvent(this, "Page.javascriptDialogClosing", empty); + if (status.IsError()) + return status; closing_count_--; } return Status(kOk); @@ -131,7 +140,9 @@ TEST(JavaScriptDialogManager, OneDialog) { std::string message; ASSERT_EQ(kNoAlertOpen, manager.GetDialogMessage(&message).code()); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); ASSERT_TRUE(manager.IsDialogOpen()); ASSERT_EQ(kOk, manager.GetDialogMessage(&message).code()); ASSERT_EQ("hi", message); @@ -148,9 +159,13 @@ TEST(JavaScriptDialogManager, TwoDialogs) { JavaScriptDialogManager manager(&client); base::DictionaryValue params; params.SetString("message", "1"); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); params.SetString("message", "2"); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); std::string message; ASSERT_EQ(kOk, manager.GetDialogMessage(&message).code()); @@ -178,12 +193,16 @@ TEST(JavaScriptDialogManager, OneDialogManualClose) { std::string message; ASSERT_EQ(kNoAlertOpen, manager.GetDialogMessage(&message).code()); - manager.OnEvent(&client, "Page.javascriptDialogOpening", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogOpening", params).code()); ASSERT_TRUE(manager.IsDialogOpen()); ASSERT_EQ(kOk, manager.GetDialogMessage(&message).code()); ASSERT_EQ("hi", message); - manager.OnEvent(&client, "Page.javascriptDialogClosing", params); + ASSERT_EQ( + kOk, + manager.OnEvent(&client, "Page.javascriptDialogClosing", params).code()); ASSERT_FALSE(manager.IsDialogOpen()); ASSERT_EQ(kNoAlertOpen, manager.GetDialogMessage(&message).code()); ASSERT_EQ(kNoAlertOpen, manager.HandleDialog(false, NULL).code()); diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.cc b/chrome/test/chromedriver/chrome/navigation_tracker.cc index 509b318..ed1d1df 100644 --- a/chrome/test/chromedriver/chrome/navigation_tracker.cc +++ b/chrome/test/chromedriver/chrome/navigation_tracker.cc @@ -4,7 +4,6 @@ #include "chrome/test/chromedriver/chrome/navigation_tracker.h" -#include "base/logging.h" #include "base/stringprintf.h" #include "base/values.h" #include "chrome/test/chromedriver/chrome/devtools_client.h" @@ -78,9 +77,9 @@ Status NavigationTracker::OnConnected(DevToolsClient* client) { return client_->SendCommand("Page.enable", empty_params); } -void NavigationTracker::OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) { +Status NavigationTracker::OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) { // Chrome does not send Page.frameStoppedLoading until all frames have // run their onLoad handlers (including frames created during the handlers). // When it does, it only sends one stopped event for all frames. @@ -90,25 +89,22 @@ void NavigationTracker::OnEvent(DevToolsClient* client, loading_state_ = kNotLoading; } else if (method == "Page.frameScheduledNavigation") { double delay; - if (!params.GetDouble("delay", &delay)) { - LOG(ERROR) << "missing or invalid 'delay'"; - return; - } + if (!params.GetDouble("delay", &delay)) + return Status(kUnknownError, "missing or invalid 'delay'"); + std::string frame_id; - if (!params.GetString("frameId", &frame_id)) { - LOG(ERROR) << "missing or invalid 'frameId'"; - return; - } + if (!params.GetString("frameId", &frame_id)) + return Status(kUnknownError, "missing or invalid 'frameId'"); + // WebDriver spec says to ignore redirects over 1s. if (delay > 1) - return; + return Status(kOk); scheduled_frame_set_.insert(frame_id); } else if (method == "Page.frameClearedScheduledNavigation") { std::string frame_id; - if (!params.GetString("frameId", &frame_id)) { - LOG(ERROR) << "missing or invalid 'frameId'"; - return; - } + if (!params.GetString("frameId", &frame_id)) + return Status(kUnknownError, "missing or invalid 'frameId'"); + scheduled_frame_set_.erase(frame_id); } else if (method == "Page.frameNavigated") { // Note: in some cases Page.frameNavigated may be received for subframes @@ -125,6 +121,7 @@ void NavigationTracker::OnEvent(DevToolsClient* client, loading_state_ = kNotLoading; scheduled_frame_set_.clear(); } + return Status(kOk); } Status NavigationTracker::OnCommandSuccess(DevToolsClient* client, diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.h b/chrome/test/chromedriver/chrome/navigation_tracker.h index cd16206..08d73bc 100644 --- a/chrome/test/chromedriver/chrome/navigation_tracker.h +++ b/chrome/test/chromedriver/chrome/navigation_tracker.h @@ -39,9 +39,9 @@ class NavigationTracker : public DevToolsEventListener { // Overridden from DevToolsEventListener: virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; virtual Status OnCommandSuccess(DevToolsClient* client, const std::string& method) OVERRIDE; diff --git a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc index 08a64ca..8b7f5f2 100644 --- a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc +++ b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc @@ -29,9 +29,11 @@ TEST(NavigationTracker, FrameLoadStartStop) { NavigationTracker tracker(&client); base::DictionaryValue params; - tracker.OnEvent(&client, "Page.frameStartedLoading", params); + ASSERT_EQ( + kOk, tracker.OnEvent(&client, "Page.frameStartedLoading", params).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", true)); - tracker.OnEvent(&client, "Page.frameStoppedLoading", params); + ASSERT_EQ( + kOk, tracker.OnEvent(&client, "Page.frameStoppedLoading", params).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false)); } @@ -44,13 +46,21 @@ TEST(NavigationTracker, NavigationScheduledThenLoaded) { params_scheduled.SetInteger("delay", 0); params_scheduled.SetString("frameId", "f"); - tracker.OnEvent(&client, "Page.frameScheduledNavigation", params_scheduled); + ASSERT_EQ( + kOk, + tracker.OnEvent( + &client, "Page.frameScheduledNavigation", params_scheduled).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", true)); - tracker.OnEvent(&client, "Page.frameStartedLoading", params); + ASSERT_EQ( + kOk, tracker.OnEvent(&client, "Page.frameStartedLoading", params).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", true)); - tracker.OnEvent(&client, "Page.frameClearedScheduledNavigation", params); + ASSERT_EQ( + kOk, + tracker.OnEvent(&client, "Page.frameClearedScheduledNavigation", params) + .code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", true)); - tracker.OnEvent(&client, "Page.frameStoppedLoading", params); + ASSERT_EQ( + kOk, tracker.OnEvent(&client, "Page.frameStoppedLoading", params).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false)); } @@ -61,7 +71,10 @@ TEST(NavigationTracker, NavigationScheduledForOtherFrame) { params_scheduled.SetInteger("delay", 0); params_scheduled.SetString("frameId", "other"); - tracker.OnEvent(&client, "Page.frameScheduledNavigation", params_scheduled); + ASSERT_EQ( + kOk, + tracker.OnEvent( + &client, "Page.frameScheduledNavigation", params_scheduled).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false)); } @@ -74,9 +87,15 @@ TEST(NavigationTracker, NavigationScheduledThenCancelled) { params_scheduled.SetInteger("delay", 0); params_scheduled.SetString("frameId", "f"); - tracker.OnEvent(&client, "Page.frameScheduledNavigation", params_scheduled); + ASSERT_EQ( + kOk, + tracker.OnEvent( + &client, "Page.frameScheduledNavigation", params_scheduled).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", true)); - tracker.OnEvent(&client, "Page.frameClearedScheduledNavigation", params); + ASSERT_EQ( + kOk, + tracker.OnEvent(&client, "Page.frameClearedScheduledNavigation", params) + .code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false)); } @@ -87,7 +106,10 @@ TEST(NavigationTracker, NavigationScheduledTooFarAway) { base::DictionaryValue params_scheduled; params_scheduled.SetInteger("delay", 10); params_scheduled.SetString("frameId", "f"); - tracker.OnEvent(&client, "Page.frameScheduledNavigation", params_scheduled); + ASSERT_EQ( + kOk, + tracker.OnEvent( + &client, "Page.frameScheduledNavigation", params_scheduled).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false)); } @@ -98,15 +120,22 @@ TEST(NavigationTracker, DiscardScheduledNavigationsOnMainFrameCommit) { base::DictionaryValue params_scheduled; params_scheduled.SetString("frameId", "subframe"); params_scheduled.SetInteger("delay", 0); - tracker.OnEvent(&client, "Page.frameScheduledNavigation", params_scheduled); + ASSERT_EQ( + kOk, + tracker.OnEvent( + &client, "Page.frameScheduledNavigation", params_scheduled).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "subframe", true)); base::DictionaryValue params_navigated; params_navigated.SetString("frame.parentId", "something"); - tracker.OnEvent(&client, "Page.frameNavigated", params_navigated); + ASSERT_EQ( + kOk, + tracker.OnEvent(&client, "Page.frameNavigated", params_navigated).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "subframe", true)); params_navigated.Clear(); - tracker.OnEvent(&client, "Page.frameNavigated", params_navigated); + ASSERT_EQ( + kOk, + tracker.OnEvent(&client, "Page.frameNavigated", params_navigated).code()); ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "subframe", false)); } @@ -154,8 +183,10 @@ class DeterminingLoadStateDevToolsClient : public StubDevToolsClient { const base::DictionaryValue& params, scoped_ptr<base::DictionaryValue>* result) OVERRIDE { if (send_event_first_.length()) { - listeners_.front()->OnEvent(this, - send_event_first_, *send_event_first_params_); + Status status = listeners_.front() + ->OnEvent(this, send_event_first_, *send_event_first_params_); + if (status.IsError()) + return status; } base::DictionaryValue result_dict; diff --git a/chrome/test/chromedriver/chrome/performance_logger.cc b/chrome/test/chromedriver/chrome/performance_logger.cc index d144870..d2d5d12 100644 --- a/chrome/test/chromedriver/chrome/performance_logger.cc +++ b/chrome/test/chromedriver/chrome/performance_logger.cc @@ -44,13 +44,12 @@ Status PerformanceLogger::OnConnected(DevToolsClient* client) { return Status(kOk); } -void PerformanceLogger::OnEvent( +Status PerformanceLogger::OnEvent( DevToolsClient* client, const std::string& method, const base::DictionaryValue& params) { - if (!ShouldLogEvent(method)) { - return; - } + if (!ShouldLogEvent(method)) + return Status(kOk); base::DictionaryValue log_message_dict; log_message_dict.SetString("webview", client->GetId()); @@ -62,4 +61,5 @@ void PerformanceLogger::OnEvent( base::JSONWriter::Write(&log_message_dict, &log_message_json); log_->AddEntry(Log::kLog, log_message_json); + return Status(kOk); } diff --git a/chrome/test/chromedriver/chrome/performance_logger.h b/chrome/test/chromedriver/chrome/performance_logger.h index 64ee85a..93306b8c 100644 --- a/chrome/test/chromedriver/chrome/performance_logger.h +++ b/chrome/test/chromedriver/chrome/performance_logger.h @@ -23,9 +23,9 @@ class PerformanceLogger : public DevToolsEventListener { explicit PerformanceLogger(Log* log); virtual Status OnConnected(DevToolsClient* client) OVERRIDE; - virtual void OnEvent(DevToolsClient* client, - const std::string& method, - const base::DictionaryValue& params) OVERRIDE; + virtual Status OnEvent(DevToolsClient* client, + const std::string& method, + const base::DictionaryValue& params) OVERRIDE; private: Log* log_; diff --git a/chrome/test/chromedriver/chrome/performance_logger_unittest.cc b/chrome/test/chromedriver/chrome/performance_logger_unittest.cc index 6206e19..e6b66dc 100644 --- a/chrome/test/chromedriver/chrome/performance_logger_unittest.cc +++ b/chrome/test/chromedriver/chrome/performance_logger_unittest.cc @@ -31,9 +31,9 @@ class FakeDevToolsClient : public StubDevToolsClient { return command; } - void TriggerEvent(const std::string& method) { + Status TriggerEvent(const std::string& method) { base::DictionaryValue empty_params; - listener_->OnEvent(this, method, empty_params); + return listener_->OnEvent(this, method, empty_params); } // Overridden from DevToolsClient: @@ -142,9 +142,10 @@ TEST(PerformanceLogger, OneWebView) { client.AddListener(&logger); logger.OnConnected(&client); ExpectEnableDomains(client); - client.TriggerEvent("Network.gaga"); - client.TriggerEvent("Page.ulala"); - client.TriggerEvent("Console.bad"); // Ignore -- different domain. + ASSERT_EQ(kOk, client.TriggerEvent("Network.gaga").code()); + ASSERT_EQ(kOk, client.TriggerEvent("Page.ulala").code()); + // Ignore -- different domain. + ASSERT_EQ(kOk, client.TriggerEvent("Console.bad").code()); ASSERT_EQ(2u, log.entries.size()); ValidateLogEntry(log.entries[0], "webview-1", "Network.gaga"); @@ -168,8 +169,8 @@ TEST(PerformanceLogger, TwoWebViews) { ExpectEnableDomains(client1); EXPECT_STREQ("", client2.PopSentCommand().c_str()); - client1.TriggerEvent("Page.gaga1"); - client2.TriggerEvent("Timeline.gaga2"); + ASSERT_EQ(kOk, client1.TriggerEvent("Page.gaga1").code()); + ASSERT_EQ(kOk, client2.TriggerEvent("Timeline.gaga2").code()); ASSERT_EQ(2u, log.entries.size()); ValidateLogEntry(log.entries[0], "webview-1", "Page.gaga1"); |