Remove legacy protos from BinaryFCMService

This doesn't break tests in other classes since references to legacy
protos had already been removed, so this is just a cleanup for code
that never runs.

Bug: 1103390
Change-Id: I72ae6028002592a1b9981174334c47454b961a3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506263Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822742}

chrome/browser/safe_browsing/cloud_content_scanning/binary_fcm_service.cc

@@ -21,7 +21,6 @@
 #include "components/gcm_driver/instance_id/instance_id.h"
 #include "components/gcm_driver/instance_id/instance_id_driver.h"
 #include "components/gcm_driver/instance_id/instance_id_profile_service.h"
-#include "components/safe_browsing/core/proto/webprotect.pb.h"
 
 namespace safe_browsing {
 
@@ -111,15 +110,8 @@ void BinaryFCMService::SetCallbackForToken(const std::string& token,
   message_token_map_[token] = std::move(callback);
 }
 
-void BinaryFCMService::SetCallbackForToken(
-    const std::string& token,
-    OnConnectorMessageCallback callback) {
-  connector_message_token_map_[token] = std::move(callback);
-}
-
 void BinaryFCMService::ClearCallbackForToken(const std::string& token) {
   message_token_map_.erase(token);
-  connector_message_token_map_.erase(token);
 }
 
 void BinaryFCMService::UnregisterInstanceID(
@@ -197,48 +189,22 @@ void BinaryFCMService::OnMessage(const std::string& app_id,
   if (!parsed)
     return;
 
-  DeepScanningClientResponse response;
+  enterprise_connectors::ContentAnalysisResponse response;
   parsed = response.ParseFromString(serialized_proto);
-  if (!parsed) {
-    base::UmaHistogramBoolean(
-        "SafeBrowsingFCMService.IncomingMessageParsedProto", false);
+  base::UmaHistogramBoolean("SafeBrowsingFCMService.IncomingMessageParsedProto",
+                            parsed);
+
+  if (!parsed)
     return;
-  }
 
-  // The received proto is either a DeepScanningClientResponse or a
-  // ContentAnalysisResponse. These protos only overlap on the request token
-  // field, so having that token exist in |connector_message_token_map_| means
-  // it should be parsed as a ContentAnalysisResponse instead.
-  if (connector_message_token_map_.contains(response.token())) {
-    enterprise_connectors::ContentAnalysisResponse response;
-    bool parsed = response.ParseFromString(serialized_proto);
-    base::UmaHistogramBoolean(
-        "SafeBrowsingFCMService.IncomingMessageParsedProto", parsed);
-
-    if (!parsed)
-      return;
-
-    auto callback_it =
-        connector_message_token_map_.find(response.request_token());
-    bool has_valid_token = (callback_it != connector_message_token_map_.end());
-    base::UmaHistogramBoolean(
-        "SafeBrowsingFCMService.IncomingMessageHasValidToken", has_valid_token);
-    if (!has_valid_token)
-      return;
-
-    callback_it->second.Run(std::move(response));
-  } else {
-    base::UmaHistogramBoolean(
-        "SafeBrowsingFCMService.IncomingMessageParsedProto", true);
-    auto callback_it = message_token_map_.find(response.token());
-    bool has_valid_token = (callback_it != message_token_map_.end());
-    base::UmaHistogramBoolean(
-        "SafeBrowsingFCMService.IncomingMessageHasValidToken", has_valid_token);
-    if (!has_valid_token)
-      return;
-
-    callback_it->second.Run(std::move(response));
-  }
+  auto callback_it = message_token_map_.find(response.request_token());
+  bool has_valid_token = (callback_it != message_token_map_.end());
+  base::UmaHistogramBoolean(
+      "SafeBrowsingFCMService.IncomingMessageHasValidToken", has_valid_token);
+  if (!has_valid_token)
+    return;
+
+  callback_it->second.Run(std::move(response));
 }
 
 void BinaryFCMService::OnMessagesDeleted(const std::string& app_id) {}

chrome/browser/safe_browsing/cloud_content_scanning/binary_fcm_service.h

@@ -14,7 +14,6 @@
 #include "components/enterprise/common/proto/connectors.pb.h"
 #include "components/gcm_driver/gcm_app_handler.h"
 #include "components/gcm_driver/instance_id/instance_id.h"
-#include "components/safe_browsing/core/proto/webprotect.pb.h"
 
 class Profile;
 
@@ -47,9 +46,7 @@ class BinaryFCMService : public gcm::GCMAppHandler {
 
   using GetInstanceIDCallback =
       base::OnceCallback<void(const std::string& token)>;
-  using OnMessageCallback =
-      base::RepeatingCallback<void(DeepScanningClientResponse)>;
-  using OnConnectorMessageCallback = base::RepeatingCallback<void(
+  using OnMessageCallback = base::RepeatingCallback<void(
       enterprise_connectors::ContentAnalysisResponse)>;
   using UnregisterInstanceIDCallback = base::OnceCallback<void(bool)>;
 
@@ -64,8 +61,6 @@ class BinaryFCMService : public gcm::GCMAppHandler {
 
   void SetCallbackForToken(const std::string& token,
                            OnMessageCallback callback);
-  void SetCallbackForToken(const std::string& token,
-                           OnConnectorMessageCallback callback);
   void ClearCallbackForToken(const std::string& token);
 
   // Performs cleanup needed at shutdown.
@@ -128,8 +123,6 @@ class BinaryFCMService : public gcm::GCMAppHandler {
       base::TimeDelta::FromSeconds(1);
 
   base::flat_map<std::string, OnMessageCallback> message_token_map_;
-  base::flat_map<std::string, OnConnectorMessageCallback>
-      connector_message_token_map_;
 
   // Map from an InstanceID to the number of callers to GetInstanceID using that
   // InstanceID.

chrome/browser/safe_browsing/cloud_content_scanning/binary_fcm_service_unittest.cc

@@ -128,54 +128,6 @@ TEST_F(BinaryFCMServiceTest, GetsInstanceID) {
 }
 
 TEST_F(BinaryFCMServiceTest, RoutesMessages) {
-  DeepScanningClientResponse response1;
-  DeepScanningClientResponse response2;
-
-  binary_fcm_service_->SetCallbackForToken(
-      "token1", base::BindRepeating(
-                    [](DeepScanningClientResponse* target_response,
-                       DeepScanningClientResponse response) {
-                      *target_response = response;
-                    },
-                    &response1));
-  binary_fcm_service_->SetCallbackForToken(
-      "token2", base::BindRepeating(
-                    [](DeepScanningClientResponse* target_response,
-                       DeepScanningClientResponse response) {
-                      *target_response = response;
-                    },
-                    &response2));
-
-  DeepScanningClientResponse message;
-  std::string serialized_message;
-  gcm::IncomingMessage incoming_message;
-
-  // Test that a message with token1 is routed only to the first callback.
-  message.set_token("token1");
-  ASSERT_TRUE(message.SerializeToString(&serialized_message));
-  base::Base64Encode(serialized_message, &serialized_message);
-  incoming_message.data["proto"] = serialized_message;
-  binary_fcm_service_->OnMessage("app_id", incoming_message);
-  EXPECT_EQ(response1.token(), "token1");
-  EXPECT_EQ(response2.token(), "");
-
-  // Test that a message with token2 is routed only to the second callback.
-  message.set_token("token2");
-  ASSERT_TRUE(message.SerializeToString(&serialized_message));
-  base::Base64Encode(serialized_message, &serialized_message);
-  incoming_message.data["proto"] = serialized_message;
-  binary_fcm_service_->OnMessage("app_id", incoming_message);
-  EXPECT_EQ(response1.token(), "token1");
-  EXPECT_EQ(response2.token(), "token2");
-
-  // Test that I can clear a callback
-  response2.clear_token();
-  binary_fcm_service_->ClearCallbackForToken("token2");
-  binary_fcm_service_->OnMessage("app_id", incoming_message);
-  EXPECT_EQ(response2.token(), "");
-}
-
-TEST_F(BinaryFCMServiceTest, RoutesConnectorMessages) {
   enterprise_connectors::ContentAnalysisResponse response1;
   enterprise_connectors::ContentAnalysisResponse response2;
 
@@ -266,21 +218,6 @@ TEST_F(BinaryFCMServiceTest, EmitsMessageParsedHistogram) {
     histograms.ExpectUniqueSample(
         "SafeBrowsingFCMService.IncomingMessageParsedProto", false, 1);
   }
-  {
-    base::HistogramTester histograms;
-    gcm::IncomingMessage incoming_message;
-    DeepScanningClientResponse message;
-    std::string serialized_message;
-
-    ASSERT_TRUE(message.SerializeToString(&serialized_message));
-    base::Base64Encode(serialized_message, &serialized_message);
-    incoming_message.data["proto"] = serialized_message;
-    binary_fcm_service_->OnMessage("app_id", incoming_message);
-    histograms.ExpectUniqueSample(
-        "SafeBrowsingFCMService.IncomingMessageParsedBase64", true, 1);
-    histograms.ExpectUniqueSample(
-        "SafeBrowsingFCMService.IncomingMessageParsedProto", true, 1);
-  }
   {
     base::HistogramTester histograms;
     gcm::IncomingMessage incoming_message;
@@ -299,32 +236,6 @@ TEST_F(BinaryFCMServiceTest, EmitsMessageParsedHistogram) {
 }
 
 TEST_F(BinaryFCMServiceTest, EmitsMessageHasValidTokenHistogram) {
-  gcm::IncomingMessage incoming_message;
-  DeepScanningClientResponse message;
-
-  message.set_token("token1");
-  std::string serialized_message;
-  ASSERT_TRUE(message.SerializeToString(&serialized_message));
-  base::Base64Encode(serialized_message, &serialized_message);
-  incoming_message.data["proto"] = serialized_message;
-
-  {
-    base::HistogramTester histograms;
-    binary_fcm_service_->OnMessage("app_id", incoming_message);
-    histograms.ExpectUniqueSample(
-        "SafeBrowsingFCMService.IncomingMessageHasValidToken", false, 1);
-  }
-  {
-    BinaryFCMService::OnMessageCallback callback = base::DoNothing();
-    binary_fcm_service_->SetCallbackForToken("token1", std::move(callback));
-    base::HistogramTester histograms;
-    binary_fcm_service_->OnMessage("app_id", incoming_message);
-    histograms.ExpectUniqueSample(
-        "SafeBrowsingFCMService.IncomingMessageHasValidToken", true, 1);
-  }
-}
-
-TEST_F(BinaryFCMServiceTest, EmitsConnectorMessageHasValidTokenHistogram) {
   gcm::IncomingMessage incoming_message;
   enterprise_connectors::ContentAnalysisResponse message;
 
@@ -341,8 +252,7 @@ TEST_F(BinaryFCMServiceTest, EmitsConnectorMessageHasValidTokenHistogram) {
         "SafeBrowsingFCMService.IncomingMessageHasValidToken", false, 1);
   }
   {
-    BinaryFCMService::OnConnectorMessageCallback callback = base::DoNothing();
-    binary_fcm_service_->SetCallbackForToken("token1", std::move(callback));
+    binary_fcm_service_->SetCallbackForToken("token1", base::DoNothing());
     base::HistogramTester histograms;
     binary_fcm_service_->OnMessage("app_id", incoming_message);
     histograms.ExpectUniqueSample(