[chromedriver] Fix passing deferencing nullptr value

DevTools event listener is passing a dereferenced pointer value
to navigation. However, sometimes it's dereferencing a nullptr value
(from an uninitialized unique_ptr)
causing navigation tracker to crash when try to use the reference.
See bug list 3578 for a case when that pointer is nullptr.
When navigating to a blank page and an alert is triggered,
ChromeDriver will not wait for the original navigation to
finish and set the original Page.navigate response info state
to ignore; when the response finally come back, ChromeDriver is
not copying the result in the response causing the result uninitialized.

Bug: chromedriver:3578,chromedriver:3488
Change-Id: I7fd6159d724036a7354a47844da9d14d76c70b3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2370091Reviewed-by: John Chen <johnchen@chromium.org>
Commit-Queue: Shengfa Lin <shengfa@google.com>
Cr-Commit-Position: refs/heads/master@{#801149}

chrome/test/chromedriver/chrome/devtools_client_impl.cc

@@ -629,9 +629,8 @@ Status DevToolsClientImpl::EnsureListenersNotifiedOfCommandResponse() {
         unnotified_cmd_response_listeners_.front();
     unnotified_cmd_response_listeners_.pop_front();
     Status status = listener->OnCommandSuccess(
-        this,
-        unnotified_cmd_response_info_->method,
-        *unnotified_cmd_response_info_->response.result.get(),
+        this, unnotified_cmd_response_info_->method,
+        unnotified_cmd_response_info_->response.result.get(),
         unnotified_cmd_response_info_->command_timeout);
     if (status.IsError())
       return status;

chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc

@@ -1182,7 +1182,7 @@ class MockCommandListener : public DevToolsEventListener {
 
   Status OnCommandSuccess(DevToolsClient* client,
                           const std::string& method,
-                          const base::DictionaryValue& result,
+                          const base::DictionaryValue* result,
                           const Timeout& command_timeout) override {
     msgs_.push_back(method);
     if (!callback_.is_null())

chrome/test/chromedriver/chrome/devtools_event_listener.cc

@@ -21,7 +21,7 @@ Status DevToolsEventListener::OnEvent(DevToolsClient* client,
 Status DevToolsEventListener::OnCommandSuccess(
     DevToolsClient* client,
     const std::string& method,
-    const base::DictionaryValue& result,
+    const base::DictionaryValue* result,
     const Timeout& command_timeout) {
   return Status(kOk);
 }

chrome/test/chromedriver/chrome/devtools_event_listener.h

@@ -32,7 +32,7 @@ class DevToolsEventListener {
   // Called when a command success response is received.
   virtual Status OnCommandSuccess(DevToolsClient* client,
                                   const std::string& method,
-                                  const base::DictionaryValue& result,
+                                  const base::DictionaryValue* result,
                                   const Timeout& command_timeout);
 
   // True if the listener should be added to the browser-wide |DevToolsClient|

chrome/test/chromedriver/chrome/navigation_tracker.cc

@@ -320,15 +320,14 @@ void NavigationTracker::clearFrameStates() {
   setCurrentFrameInvalid();
 }
 
-Status NavigationTracker::OnCommandSuccess(
-    DevToolsClient* client,
-    const std::string& method,
-    const base::DictionaryValue& result,
-    const Timeout& command_timeout) {
+Status NavigationTracker::OnCommandSuccess(DevToolsClient* client,
+                                           const std::string& method,
+                                           const base::DictionaryValue* result,
+                                           const Timeout& command_timeout) {
   // Check if Page.navigate has any error from top frame
   std::string error_text;
-  if (method == "Page.navigate" && result.GetString("errorText", &error_text) &&
-      isNetworkError(error_text))
+  if (method == "Page.navigate" && result &&
+      result->GetString("errorText", &error_text) && isNetworkError(error_text))
     return Status(kUnknownError, error_text);
 
   // Check for start of navigation. In some case response to navigate is delayed

chrome/test/chromedriver/chrome/navigation_tracker.h

@@ -62,7 +62,7 @@ class NavigationTracker : public DevToolsEventListener,
                  const base::DictionaryValue& params) override;
   Status OnCommandSuccess(DevToolsClient* client,
                           const std::string& method,
-                          const base::DictionaryValue& result,
+                          const base::DictionaryValue* result,
                           const Timeout& command_timeout) override;
 
  private:

chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc

@@ -506,7 +506,7 @@ TEST(NavigationTracker, OnSuccessfulNavigate) {
   base::DictionaryValue result;
   result.SetString("frameId", client_ptr->GetId());
   web_view.nextEvaluateScript("loading", kOk);
-  tracker.OnCommandSuccess(client_ptr, "Page.navigate", result, Timeout());
+  tracker.OnCommandSuccess(client_ptr, "Page.navigate", &result, Timeout());
   ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, true));
   web_view.nextEvaluateScript("complete", kOk);
   tracker.OnEvent(client_ptr, "Page.loadEventFired", params);
@@ -537,7 +537,7 @@ TEST(NavigationTracker, OnNetworkErroredNavigate) {
   web_view.nextEvaluateScript("loading", kOk);
   ASSERT_NE(
       kOk,
-      tracker.OnCommandSuccess(client_ptr, "Page.navigate", result, Timeout())
+      tracker.OnCommandSuccess(client_ptr, "Page.navigate", &result, Timeout())
           .code());
   ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, false));
 }
@@ -564,7 +564,7 @@ TEST(NavigationTracker, OnNonNetworkErroredNavigate) {
   result.SetString("frameId", client_ptr->GetId());
   result.SetString("errorText", "net::ERR_CERT_COMMON_NAME_INVALID");
   web_view.nextEvaluateScript("loading", kOk);
-  tracker.OnCommandSuccess(client_ptr, "Page.navigate", result, Timeout());
+  tracker.OnCommandSuccess(client_ptr, "Page.navigate", &result, Timeout());
   ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, true));
   web_view.nextEvaluateScript("complete", kOk);
   tracker.OnEvent(client_ptr, "Page.loadEventFired", params);