Deliver errors from devtools API calls to callbacks

headless/lib/browser/headless_devtools_client_impl.cc

Index: headless/lib/browser/headless_devtools_client_impl.cc
diff --git a/headless/lib/browser/headless_devtools_client_impl.cc b/headless/lib/browser/headless_devtools_client_impl.cc
index 22f52946e4969707b50dac9e77528cb9fe6b6065..56bf5ede6d140ea26d24419f4d84f248a7d7f1cf 100644
--- a/headless/lib/browser/headless_devtools_client_impl.cc
+++ b/headless/lib/browser/headless_devtools_client_impl.cc
@@ -126,11 +126,15 @@ bool HeadlessDevToolsClientImpl::DispatchMessageReply(
   pending_messages_.erase(it);
   if (!callback.callback_with_result.is_null()) {
     const base::DictionaryValue* result_dict;
-    if (!message_dict.GetDictionary("result", &result_dict)) {
-      NOTREACHED() << "Badly formed reply result";
+    if (message_dict.GetDictionary("result", &result_dict)) {
+      callback.callback_with_result.Run(*result_dict);
+    } else if (message_dict.GetDictionary("error", &result_dict)) {
+      DLOG(ERROR) << "Error in method call result: " << *result_dict;
+      callback.callback_with_result.Run(*base::Value::CreateNullValue());

[altimin, 2016/11/29 17:43:27]

I don't really like creating a temporary unique pointer and passing a const ref
to a function, it scares me. Can we please have an explicit variable
declaration?

[tmarek, 2016/11/30 09:01:08]

Done.

+    } else {
+      NOTREACHED() << "Replay has neither result nor error";

[altimin, 2016/11/29 17:43:27]

nit: reply?

[tmarek, 2016/11/30 09:01:08]

Done.

       return false;
     }
-    callback.callback_with_result.Run(*result_dict);
   } else if (!callback.callback.is_null()) {
     callback.callback.Run();
   }