Send an error message when we fail to fetch DIAL app info

The error message format corresponds to chrome.cast.Error in the Cast
SDK, which is also used by the Cast MRP.

Bug: 1153895, b/171829399
Change-Id: Ia4c3565472e70d19869bdcf30a7f8cb2897e4691
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585748
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840496}

chrome/browser/media/router/discovery/dial/dial_app_discovery_service.cc

@@ -115,6 +115,8 @@ void DialAppDiscoveryService::PendingRequest::OnDialAppInfoFetchError(
     int response_code,
     const std::string& error_message) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // FIXME: |response_code| uses internal codes from net/base/net_error_list.h
+  // instead of HTTP status codes, so |kNotFound| is never used.
   if (response_code == net::HTTP_NOT_FOUND ||
       response_code >= net::HTTP_INTERNAL_SERVER_ERROR ||
       response_code == net::HTTP_OK) {

chrome/browser/media/router/providers/dial/dial_internal_message_util.cc

@@ -11,7 +11,6 @@
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/strings/string_number_conversions.h"
-#include "chrome/browser/media/router/providers/dial/dial_activity_manager.h"
 #include "components/media_router/browser/route_message_util.h"
 #include "components/media_router/common/discovery/media_sink_internal.h"
 #include "net/base/escape.h"
@@ -49,6 +48,8 @@ std::string DialInternalMessageTypeToString(DialInternalMessageType type) {
       return "custom_dial_launch";
     case DialInternalMessageType::kDialAppInfo:
       return "dial_app_info";
+    case DialInternalMessageType::kError:
+      return "error";
     case DialInternalMessageType::kOther:
       break;
   }
@@ -76,6 +77,9 @@ DialInternalMessageType StringToDialInternalMessageType(
   if (str_type == "dial_app_info")
     return DialInternalMessageType::kDialAppInfo;
 
+  if (str_type == "error")
+    return DialInternalMessageType::kError;
+
   return DialInternalMessageType::kOther;
 }
 
@@ -90,6 +94,22 @@ std::string DialReceiverActionToString(DialReceiverAction action) {
   return "";
 }
 
+std::string DialAppInfoErrorToString(DialAppInfoResultCode error) {
+  switch (error) {
+    case DialAppInfoResultCode::kNotFound:
+      return "not_found";
+    case DialAppInfoResultCode::kNetworkError:
+      return "network_error";
+    case DialAppInfoResultCode::kParsingError:
+      return "parsing_error";
+    case DialAppInfoResultCode::kOk:
+    case DialAppInfoResultCode::kCount:
+      NOTREACHED() << "Unexpected DialAppInfoResultCode: "
+                   << static_cast<int>(error);
+      return "";
+  }
+}
+
 }  // namespace
 
 // static
@@ -203,69 +223,67 @@ bool DialInternalMessageUtil::IsStopSessionMessage(
 }
 
 mojom::RouteMessagePtr DialInternalMessageUtil::CreateNewSessionMessage(
-    const DialLaunchInfo& launch_info,
+    const std::string& app_name,
+    const std::string& client_id,
     const MediaSinkInternal& sink) const {
-  base::Value message = CreateDialMessageCommon(
-      DialInternalMessageType::kNewSession,
-      CreateNewSessionBody(launch_info, sink), launch_info.client_id);
-
-  std::string str;
-  CHECK(base::JSONWriter::Write(message, &str));
-  return message_util::RouteMessageFromString(std::move(str));
+  base::Value message =
+      CreateDialMessageCommon(DialInternalMessageType::kNewSession,
+                              CreateNewSessionBody(app_name, sink), client_id);
+  return message_util::RouteMessageFromValue(std::move(message));
 }
 
 mojom::RouteMessagePtr DialInternalMessageUtil::CreateReceiverActionCastMessage(
-    const DialLaunchInfo& launch_info,
+    const std::string& client_id,
     const MediaSinkInternal& sink) const {
   base::Value message = CreateDialMessageCommon(
       DialInternalMessageType::kReceiverAction,
-      CreateReceiverActionBody(sink, DialReceiverAction::kCast),
-      launch_info.client_id);
-
-  std::string str;
-  CHECK(base::JSONWriter::Write(message, &str));
-  return message_util::RouteMessageFromString(std::move(str));
+      CreateReceiverActionBody(sink, DialReceiverAction::kCast), client_id);
+  return message_util::RouteMessageFromValue(std::move(message));
 }
 
 mojom::RouteMessagePtr DialInternalMessageUtil::CreateReceiverActionStopMessage(
-    const DialLaunchInfo& launch_info,
+    const std::string& client_id,
     const MediaSinkInternal& sink) const {
   base::Value message = CreateDialMessageCommon(
       DialInternalMessageType::kReceiverAction,
-      CreateReceiverActionBody(sink, DialReceiverAction::kStop),
-      launch_info.client_id);
-
-  std::string str;
-  CHECK(base::JSONWriter::Write(message, &str));
-  return message_util::RouteMessageFromString(std::move(str));
+      CreateReceiverActionBody(sink, DialReceiverAction::kStop), client_id);
+  return message_util::RouteMessageFromValue(std::move(message));
 }
 
 std::pair<mojom::RouteMessagePtr, int>
 DialInternalMessageUtil::CreateCustomDialLaunchMessage(
-    const DialLaunchInfo& launch_info,
+    const std::string& client_id,
     const MediaSinkInternal& sink,
     const ParsedDialAppInfo& app_info) const {
   int seq_number = GetNextDialLaunchSequenceNumber();
   // A CUSTOM_DIAL_LAUNCH message is the same as A DIAL_APP_INFO response except
   // for the message type.
-  return {CreateDialAppInfoMessage(launch_info, sink, app_info, seq_number,
+  return {CreateDialAppInfoMessage(client_id, sink, app_info, seq_number,
                                    DialInternalMessageType::kCustomDialLaunch),
           seq_number};
 }
 
 mojom::RouteMessagePtr DialInternalMessageUtil::CreateDialAppInfoMessage(
-    const DialLaunchInfo& launch_info,
+    const std::string& client_id,
     const MediaSinkInternal& sink,
     const ParsedDialAppInfo& app_info,
     int sequence_number,
     DialInternalMessageType type) const {
   base::Value message = CreateDialMessageCommon(
-      type, CreateDialAppInfoBody(sink, app_info), launch_info.client_id);
-  message.SetKey("sequenceNumber", base::Value(sequence_number));
+      type, CreateDialAppInfoBody(sink, app_info), client_id, sequence_number);
+  return message_util::RouteMessageFromValue(std::move(message));
+}
 
-  std::string str;
-  CHECK(base::JSONWriter::Write(message, &str));
-  return message_util::RouteMessageFromString(std::move(str));
+mojom::RouteMessagePtr DialInternalMessageUtil::CreateDialAppInfoErrorMessage(
+    DialAppInfoResultCode result_code,
+    const std::string& client_id,
+    int sequence_number) const {
+  base::Value body(base::Value::Type::DICTIONARY);
+  body.SetKey("code", base::Value(DialAppInfoErrorToString(result_code)));
+  base::Value message =
+      CreateDialMessageCommon(DialInternalMessageType::kError, std::move(body),
+                              client_id, sequence_number);
+  return message_util::RouteMessageFromValue(std::move(message));
 }
 
 base::Value DialInternalMessageUtil::CreateReceiver(
@@ -301,12 +319,12 @@ base::Value DialInternalMessageUtil::CreateReceiverActionBody(
 }
 
 base::Value DialInternalMessageUtil::CreateNewSessionBody(
-    const DialLaunchInfo& launch_info,
+    const std::string& app_name,
     const MediaSinkInternal& sink) const {
   base::Value message_body(base::Value::Type::DICTIONARY);
   message_body.SetKey("sessionId", base::Value(GetNextSessionId()));
   message_body.SetKey("appId", base::Value(""));
-  message_body.SetKey("displayName", base::Value(launch_info.app_name));
+  message_body.SetKey("displayName", base::Value(app_name));
   message_body.SetKey("statusText", base::Value(""));
   message_body.SetKey("appImages", base::ListValue());
   message_body.SetKey("receiver", CreateReceiver(sink));
@@ -337,12 +355,13 @@ base::Value DialInternalMessageUtil::CreateDialAppInfoBody(
 base::Value DialInternalMessageUtil::CreateDialMessageCommon(
     DialInternalMessageType type,
     base::Value body,
-    const std::string& client_id) const {
+    const std::string& client_id,
+    int sequence_number) const {
   base::Value message(base::Value::Type::DICTIONARY);
   message.SetKey("type", base::Value(DialInternalMessageTypeToString(type)));
   message.SetKey("message", std::move(body));
   message.SetKey("clientId", base::Value(client_id));
-  message.SetKey("sequenceNumber", base::Value(-1));
+  message.SetKey("sequenceNumber", base::Value(sequence_number));
   message.SetKey("timeoutMillis", base::Value(0));
   return message;
 }

chrome/browser/media/router/providers/dial/dial_internal_message_util.h

@@ -10,12 +10,12 @@
 
 #include "base/macros.h"
 #include "base/values.h"
+#include "chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h"
 #include "chrome/browser/media/router/discovery/dial/parsed_dial_app_info.h"
 #include "components/media_router/common/mojom/media_router.mojom.h"
 
 namespace media_router {
 
-struct DialLaunchInfo;
 class MediaSinkInternal;
 
 // Types of internal messages that are used in a custom DIAL launch workflow.
@@ -27,6 +27,7 @@ enum class DialInternalMessageType {
   // MR -> Cast SDK
   kNewSession,
   kReceiverAction,
+  kError,
 
   // MR <-> Cast SDK
   kCustomDialLaunch,
@@ -101,26 +102,27 @@ class DialInternalMessageUtil final {
   // Returns a NEW_SESSION message to be sent to the page when the user requests
   // an app launch.
   mojom::RouteMessagePtr CreateNewSessionMessage(
-      const DialLaunchInfo& launch_info,
+      const std::string& app_name,
+      const std::string& client_id,
       const MediaSinkInternal& sink) const;
 
   // Returns a RECEIVER_ACTION / CAST message to be sent to the page when the
   // user requests an app launch.
   mojom::RouteMessagePtr CreateReceiverActionCastMessage(
-      const DialLaunchInfo& launch_info,
+      const std::string& client_id,
       const MediaSinkInternal& sink) const;
 
   // Returns a RECEIVER_ACTION / STOP message to be sent to the page when an app
   // is stopped by DialMediaRouteProvider.
   mojom::RouteMessagePtr CreateReceiverActionStopMessage(
-      const DialLaunchInfo& launch_info,
+      const std::string& client_id,
       const MediaSinkInternal& sink) const;
 
   // Returns a CUSTOM_DIAL_LAUNCH request message to be sent to the page.
   // Generates and returns the next number to associate a DIAL launch sequence
   // with.
   std::pair<mojom::RouteMessagePtr, int> CreateCustomDialLaunchMessage(
-      const DialLaunchInfo& launch_info,
+      const std::string& client_id,
       const MediaSinkInternal& sink,
       const ParsedDialAppInfo& app_info) const;
 
@@ -128,23 +130,34 @@ class DialInternalMessageUtil final {
   // CUSTOM_DIAL_LAUNCH (called via CreateCustomDialLaunchMessage() above)
   // message.
   mojom::RouteMessagePtr CreateDialAppInfoMessage(
-      const DialLaunchInfo& launch_info,
+      const std::string& client_id,
       const MediaSinkInternal& sink,
       const ParsedDialAppInfo& app_info,
       int sequence_number,
       DialInternalMessageType type) const;
 
+  mojom::RouteMessagePtr CreateDialAppInfoErrorMessage(
+      DialAppInfoResultCode result_code,
+      const std::string& client_id,
+      int sequence_number) const;
+
  private:
   base::Value CreateReceiver(const MediaSinkInternal& sink) const;
   base::Value CreateReceiverActionBody(const MediaSinkInternal& sink,
                                        DialReceiverAction action) const;
-  base::Value CreateNewSessionBody(const DialLaunchInfo& launch_info,
+  base::Value CreateNewSessionBody(const std::string& app_name,
                                    const MediaSinkInternal& sink) const;
   base::Value CreateDialAppInfoBody(const MediaSinkInternal& sink,
                                     const ParsedDialAppInfo& app_info) const;
+
+  // |sequence_number| is used by the Cast SDK to match up requests from the SDK
+  // to Chrome with responses from Chrome. If a message from Chrome has no
+  // corresponding request, then its |sequence_number| is an invalid value of
+  // -1.
   base::Value CreateDialMessageCommon(DialInternalMessageType type,
                                       base::Value body,
-                                      const std::string& client_id) const;
+                                      const std::string& client_id,
+                                      int sequence_number = -1) const;
 
   std::string hash_token_;
   DISALLOW_COPY_AND_ASSIGN(DialInternalMessageUtil);

chrome/browser/media/router/providers/dial/dial_internal_message_util_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/json/json_reader.h"
 #include "base/strings/stringprintf.h"
+#include "chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h"
 #include "chrome/browser/media/router/providers/dial/dial_activity_manager.h"
 #include "chrome/browser/media/router/test/provider_test_helpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -47,7 +48,7 @@ class DialInternalMessageUtilTest : public ::testing::Test {
 };
 
 TEST_F(DialInternalMessageUtilTest, ParseClientConnectMessage) {
-  const char kClientConnectMessage[] = R"(
+  constexpr char kClientConnectMessage[] = R"(
         {
           "type":"client_connect",
           "message":"15212681945883010",
@@ -65,7 +66,7 @@ TEST_F(DialInternalMessageUtilTest, ParseClientConnectMessage) {
 }
 
 TEST_F(DialInternalMessageUtilTest, ParseCustomDialLaunchMessage) {
-  const char kCustomDialLaunchMessage[] = R"(
+  constexpr char kCustomDialLaunchMessage[] = R"(
   {
     "type":"custom_dial_launch",
     "message": {
@@ -90,7 +91,7 @@ TEST_F(DialInternalMessageUtilTest, ParseCustomDialLaunchMessage) {
 }
 
 TEST_F(DialInternalMessageUtilTest, ParseV2StopSessionMessage) {
-  const char kV2StopSessionMessage[] = R"(
+  constexpr char kV2StopSessionMessage[] = R"(
   {
     "type":"v2_message",
     "message": {
@@ -111,7 +112,7 @@ TEST_F(DialInternalMessageUtilTest, ParseV2StopSessionMessage) {
 }
 
 TEST_F(DialInternalMessageUtilTest, CreateReceiverActionCastMessage) {
-  const char kReceiverActionCastMessage[] = R"(
+  constexpr char kReceiverActionCastMessage[] = R"(
     {
       "clientId":"152127444812943594",
       "message": {
@@ -132,13 +133,14 @@ TEST_F(DialInternalMessageUtilTest, CreateReceiverActionCastMessage) {
       "type":"receiver_action"
     })";
 
-  auto message = util_.CreateReceiverActionCastMessage(launch_info_, sink_);
+  auto message =
+      util_.CreateReceiverActionCastMessage(launch_info_.client_id, sink_);
   ASSERT_TRUE(message->message);
   ExpectMessagesEqual(kReceiverActionCastMessage, message->message.value());
 }
 
 TEST_F(DialInternalMessageUtilTest, CreateReceiverActionStopMessage) {
-  const char kReceiverActionStopMessage[] = R"(
+  constexpr char kReceiverActionStopMessage[] = R"(
     {
       "clientId":"152127444812943594",
       "message": {
@@ -159,13 +161,14 @@ TEST_F(DialInternalMessageUtilTest, CreateReceiverActionStopMessage) {
       "type":"receiver_action"
     })";
 
-  auto message = util_.CreateReceiverActionStopMessage(launch_info_, sink_);
+  auto message =
+      util_.CreateReceiverActionStopMessage(launch_info_.client_id, sink_);
   ASSERT_TRUE(message->message);
   ExpectMessagesEqual(kReceiverActionStopMessage, message->message.value());
 }
 
 TEST_F(DialInternalMessageUtilTest, CreateNewSessionMessage) {
-  const char kNewSessionMessage[] = R"(
+  constexpr char kNewSessionMessage[] = R"(
   {
     "clientId":"152127444812943594",
     "message": {
@@ -195,13 +198,14 @@ TEST_F(DialInternalMessageUtilTest, CreateNewSessionMessage) {
     "type":"new_session"
   })";
 
-  auto message = util_.CreateNewSessionMessage(launch_info_, sink_);
+  auto message = util_.CreateNewSessionMessage(launch_info_.app_name,
+                                               launch_info_.client_id, sink_);
   ASSERT_TRUE(message->message);
   ExpectMessagesEqual(kNewSessionMessage, message->message.value());
 }
 
 TEST_F(DialInternalMessageUtilTest, CreateCustomDialLaunchMessage) {
-  const char kCustomDialLaunchMessage[] = R"(
+  constexpr char kCustomDialLaunchMessage[] = R"(
   {
     "clientId":"152127444812943594",
     "message": {
@@ -225,8 +229,8 @@ TEST_F(DialInternalMessageUtilTest, CreateCustomDialLaunchMessage) {
 
   ParsedDialAppInfo app_info =
       CreateParsedDialAppInfo("YouTube", DialAppState::kStopped);
-  auto message_and_seq_num =
-      util_.CreateCustomDialLaunchMessage(launch_info_, sink_, app_info);
+  auto message_and_seq_num = util_.CreateCustomDialLaunchMessage(
+      launch_info_.client_id, sink_, app_info);
   const auto& message = message_and_seq_num.first;
   int seq_num = message_and_seq_num.second;
   ASSERT_TRUE(message->message);
@@ -234,4 +238,25 @@ TEST_F(DialInternalMessageUtilTest, CreateCustomDialLaunchMessage) {
                       message->message.value());
 }
 
+TEST_F(DialInternalMessageUtilTest, CreateDialAppInfoErrorMessage) {
+  constexpr char kClientId[] = "152127444812943594";
+  constexpr char kErrorMessage[] = R"(
+  {
+    "clientId": "%s",
+    "type": "error",
+    "message": {
+      "code": "parsing_error"
+    },
+    "sequenceNumber": %d,
+    "timeoutMillis": 0
+  })";
+  const int kSequenceNumber = 42;
+
+  auto message = util_.CreateDialAppInfoErrorMessage(
+      DialAppInfoResultCode::kParsingError, kClientId, kSequenceNumber);
+  ExpectMessagesEqual(
+      base::StringPrintf(kErrorMessage, kClientId, kSequenceNumber),
+      message->message.value());
+}
+
 }  // namespace media_router

chrome/browser/media/router/providers/dial/dial_media_route_provider.cc

@@ -147,10 +147,10 @@ void DialMediaRouteProvider::CreateRoute(const std::string& media_source,
   // the Cast SDK to complete the launch sequence. The first messages that the
   // MRP needs to send are the RECEIVER_ACTION and NEW_SESSION.
   std::vector<mojom::RouteMessagePtr> messages;
-  messages.emplace_back(internal_message_util_.CreateReceiverActionCastMessage(
-      activity->launch_info, *sink));
-  messages.emplace_back(internal_message_util_.CreateNewSessionMessage(
-      activity->launch_info, *sink));
+  messages.push_back(internal_message_util_.CreateReceiverActionCastMessage(
+      activity->launch_info.client_id, *sink));
+  messages.push_back(internal_message_util_.CreateNewSessionMessage(
+      activity->launch_info.app_name, activity->launch_info.client_id, *sink));
   message_sender_->SendMessages(route_id, std::move(messages));
 }
 
@@ -358,14 +358,14 @@ void DialMediaRouteProvider::SendCustomDialLaunchMessage(
 
   auto message_and_seq_number =
       internal_message_util_.CreateCustomDialLaunchMessage(
-          activity->launch_info, *sink, *result.app_info);
+          activity->launch_info.client_id, *sink, *result.app_info);
   pending_dial_launches_.insert(message_and_seq_number.second);
   if (pending_dial_launches_.size() > kMaxPendingDialLaunches) {
     pending_dial_launches_.erase(pending_dial_launches_.begin());
   }
 
   std::vector<mojom::RouteMessagePtr> messages;
-  messages.emplace_back(std::move(message_and_seq_number.first));
+  messages.push_back(std::move(message_and_seq_number.first));
   message_sender_->SendMessages(route_id, std::move(messages));
 }
 
@@ -379,17 +379,22 @@ void DialMediaRouteProvider::SendDialAppInfoResponse(
 
   auto* activity = activity_manager_->GetActivity(route_id);
   auto* sink = media_sink_service_->GetSinkById(sink_id);
-  // TODO(crbug.com/1153895): If |result.app_info| is null, we may want to send
-  // an error message to the sender client.
-  if (!result.app_info || !activity || !sink) {
+  // If the activity no longer exists, there is no need to inform the sender
+  // client of the activity status.
+  if (!activity || !sink) {
     return;
   }
-  mojom::RouteMessagePtr message =
-      internal_message_util_.CreateDialAppInfoMessage(
-          activity->launch_info, *sink, *result.app_info, sequence_number,
-          DialInternalMessageType::kDialAppInfo);
+  mojom::RouteMessagePtr message;
+  if (result.app_info) {
+    message = internal_message_util_.CreateDialAppInfoMessage(
+        activity->launch_info.client_id, *sink, *result.app_info,
+        sequence_number, DialInternalMessageType::kDialAppInfo);
+  } else {
+    message = internal_message_util_.CreateDialAppInfoErrorMessage(
+        result.result_code, activity->launch_info.client_id, sequence_number);
+  }
   std::vector<mojom::RouteMessagePtr> messages;
-  messages.emplace_back(std::move(message));
+  messages.push_back(std::move(message));
   message_sender_->SendMessages(route_id, std::move(messages));
 }
 
@@ -447,9 +452,8 @@ void DialMediaRouteProvider::DoTerminateRoute(const DialActivity& activity,
       can_stop_app = activity_manager_->CanStopApp(route_id);
   if (can_stop_app.second == RouteRequestResult::OK) {
     std::vector<mojom::RouteMessagePtr> messages;
-    messages.emplace_back(
-        internal_message_util_.CreateReceiverActionStopMessage(
-            activity.launch_info, sink));
+    messages.push_back(internal_message_util_.CreateReceiverActionStopMessage(
+        activity.launch_info.client_id, sink));
     message_sender_->SendMessages(route_id, std::move(messages));
     activity_manager_->StopApp(
         route_id,

components/media_router/browser/route_message_util.cc

@@ -4,7 +4,9 @@
 
 #include "components/media_router/browser/route_message_util.h"
 
+#include "base/json/json_writer.h"
 #include "base/macros.h"
+#include "base/values.h"
 
 using media_router::mojom::RouteMessage;
 using media_router::mojom::RouteMessagePtr;
@@ -12,6 +14,13 @@ using media_router::mojom::RouteMessagePtr;
 namespace media_router {
 namespace message_util {
 
+media_router::mojom::RouteMessagePtr RouteMessageFromValue(
+    base::Value message) {
+  std::string str;
+  CHECK(base::JSONWriter::Write(message, &str));
+  return RouteMessageFromString(std::move(str));
+}
+
 RouteMessagePtr RouteMessageFromString(std::string message) {
   auto route_message = RouteMessage::New();
   route_message->type = RouteMessage::Type::TEXT;

components/media_router/browser/route_message_util.h

@@ -12,9 +12,15 @@
 #include "components/media_router/common/mojom/media_router.mojom.h"
 #include "third_party/blink/public/mojom/presentation/presentation.mojom.h"
 
+namespace base {
+class Value;
+}  // namespace base
+
 namespace media_router {
 namespace message_util {
 
+media_router::mojom::RouteMessagePtr RouteMessageFromValue(base::Value value);
+
 media_router::mojom::RouteMessagePtr RouteMessageFromString(
     std::string message);