[Cast MRP] Remove null fields for Cast-protocol messages.

Bug: 1129217
Change-Id: Ic9300541ba56cddcb75524af07dd8afe9a4f35c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427124
Auto-Submit: John Williams <jrw@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: John Williams <jrw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813328}

chrome/browser/media/router/providers/cast/cast_session_client_impl.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/media/router/providers/cast/cast_session_client_impl.h"
 
+#include <vector>
+
 #include "chrome/browser/media/router/data_decoder_util.h"
 #include "chrome/browser/media/router/providers/cast/app_activity.h"
 #include "components/cast_channel/enum_table.h"
@@ -23,6 +25,28 @@ void ReportClientMessageParseError(const MediaRoute::Id& route_id,
               << error;
 }
 
+// Traverses a JSON value, recursively removing any dict entries whose value is
+// null.
+void RemoveNullFields(base::Value& value) {
+  if (value.is_list()) {
+    for (auto& item : value.GetList()) {
+      RemoveNullFields(item);
+    }
+  } else if (value.is_dict()) {
+    std::vector<std::string> to_remove;
+    for (auto pair : value.DictItems()) {
+      if (pair.second.is_none()) {
+        to_remove.push_back(pair.first);
+      } else {
+        RemoveNullFields(pair.second);
+      }
+    }
+    for (const auto& key : to_remove) {
+      value.RemoveKey(key);
+    }
+  }
+}
+
 }  // namespace
 
 CastSessionClientImpl::CastSessionClientImpl(const std::string& client_id,
@@ -127,6 +151,11 @@ void CastSessionClientImpl::HandleParsedClientMessage(
     return;
   }
 
+  // NOTE(jrw): This step isn't part of the Cast protocol per se, but it's
+  // required for backward compatibility.  There is one known case
+  // (crbug.com/1129217) where not doing it breaks the Cast SDK.
+  RemoveNullFields(*result.value);
+
   std::unique_ptr<CastInternalMessage> cast_message =
       CastInternalMessage::From(std::move(*result.value));
   if (!cast_message) {

chrome/browser/media/router/providers/cast/cast_session_client_unittest.cc

@@ -159,6 +159,39 @@ TEST_F(CastSessionClientImplTest, OnMessageWrongSessionId) {
       })"));
 }
 
+TEST_F(CastSessionClientImplTest, NullFieldsAreRemoved) {
+  EXPECT_CALL(activity_, SendMediaRequestToReceiver)
+      .WillOnce([](const auto& message) {
+        // TODO(crbug.com/961081): Use IsCastInternalMessage as argument to
+        // SendMediaRequestToReceiver when bug is fixed.
+        EXPECT_THAT(message, IsCastInternalMessage(R"({
+          "type": "v2_message",
+          "clientId": "theClientId",
+          "sequenceNumber": 123,
+          "message": {
+             "sessionId": "theSessionId",
+             "type": "MEDIA_GET_STATUS",
+             "array": [{"in_array": true}]
+          }
+        })"));
+        return 0;
+      });
+
+  client_->OnMessage(
+      blink::mojom::PresentationConnectionMessage::NewMessage(R"({
+        "type": "v2_message",
+        "clientId": "theClientId",
+        "sequenceNumber": 123,
+        "message": {
+          "sessionId": "theSessionId",
+          "type": "MEDIA_GET_STATUS",
+          "array": [{"in_array": true, "is_null": null}],
+          "dummy": null
+        }
+      })"));
+  RunUntilIdle();
+}
+
 TEST_F(CastSessionClientImplTest, AppMessageFromClient) {
   EXPECT_CALL(activity_, SendAppMessageToReceiver)
       .WillOnce(Return(cast_channel::Result::kOk));