Remove legacy protos support from BinaryUploadService

This should not change deep scanning behaviour with Connectors.

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

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.h

@@ -22,7 +22,6 @@
 #include "components/enterprise/common/proto/connectors.pb.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/safe_browsing/core/proto/csd.pb.h"
-#include "components/safe_browsing/core/proto/webprotect.pb.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 class Profile;
@@ -97,7 +96,6 @@ class BinaryUploadService : public KeyedService {
   class Request {
    public:
     // |callback| will run on the UI thread.
-    Request(Callback callback, GURL url);
     Request(ContentAnalysisCallback, GURL url);
     virtual ~Request();
     Request(const Request&) = delete;
@@ -132,11 +130,6 @@ class BinaryUploadService : public KeyedService {
     using DataCallback = base::OnceCallback<void(Result, const Data&)>;
     virtual void GetRequestData(DataCallback callback) = 0;
 
-    // Returns the metadata to upload, as a DeepScanningClientRequest.
-    const DeepScanningClientRequest& deep_scanning_request() const {
-      return deep_scanning_request_;
-    }
-
     // Returns the URL to send the request to.
     GURL GetUrlWithParams() const;
 
@@ -146,17 +139,9 @@ class BinaryUploadService : public KeyedService {
       return content_analysis_request_;
     }
 
-    bool use_legacy_proto() const { return use_legacy_proto_; }
-
     void set_tab_url(const GURL& tab_url);
     const GURL& tab_url() const;
 
-    // Methods for modifying the DeepScanningClientRequest.
-    void set_request_dlp_scan(DlpDeepScanningClientRequest dlp_request);
-    void set_request_malware_scan(
-        MalwareDeepScanningClientRequest malware_request);
-    void set_request_token(const std::string& token);
-
     // Methods for modifying the ContentAnalysisRequest.
     void set_analysis_connector(
         enterprise_connectors::AnalysisConnector connector);
@@ -164,15 +149,14 @@ class BinaryUploadService : public KeyedService {
     void set_csd(ClientDownloadRequest csd);
     void add_tag(const std::string& tag);
     void set_email(const std::string& email);
-
-    // Methods for modifying either internal proto requests.
+    void set_request_token(const std::string& token);
     void set_fcm_token(const std::string& token);
     void set_device_token(const std::string& token);
     void set_filename(const std::string& filename);
     void set_digest(const std::string& digest);
     void clear_dlp_scan_request();
 
-    // Methods for accessing either internal proto requests.
+    // Methods for accessing the ContentAnalysisRequest.
     const std::string& device_token() const;
     const std::string& request_token() const;
     const std::string& fcm_notification_token() const;
@@ -181,26 +165,19 @@ class BinaryUploadService : public KeyedService {
 
     // Finish the request, with the given |result| and |response| from the
     // server.
-    void FinishRequest(Result result);
-    void FinishConnectorRequest(
-        Result result,
-        enterprise_connectors::ContentAnalysisResponse response);
-    void FinishLegacyRequest(Result result,
-                             DeepScanningClientResponse response);
+    void FinishRequest(Result result,
+                       enterprise_connectors::ContentAnalysisResponse response);
 
     // Calls SerializeToString on the appropriate proto request.
     void SerializeToString(std::string* destination) const;
 
    private:
-    const bool use_legacy_proto_;
-
-    DeepScanningClientRequest deep_scanning_request_;
-    Callback callback_;
-
     enterprise_connectors::ContentAnalysisRequest content_analysis_request_;
     ContentAnalysisCallback content_analysis_callback_;
 
+    // The URL to send the data to for scanning.
     GURL url_;
+
     // The URL of the page that initially triggered the scan.
     GURL tab_url_;
   };
@@ -230,14 +207,9 @@ class BinaryUploadService : public KeyedService {
   static GURL GetUploadUrl(bool is_advanced_protection_request);
 
  protected:
-  void FinishRequest(Request* request, Result result);
-  void FinishConnectorRequest(
-      Request* request,
-      Result result,
-      enterprise_connectors::ContentAnalysisResponse response);
-  void FinishLegacyRequest(Request* request,
-                           Result result,
-                           DeepScanningClientResponse response);
+  void FinishRequest(Request* request,
+                     Result result,
+                     enterprise_connectors::ContentAnalysisResponse response);
 
  private:
   friend class BinaryUploadServiceTest;
@@ -257,14 +229,10 @@ class BinaryUploadService : public KeyedService {
                         bool success,
                         const std::string& response_data);
 
-  void OnGetConnectorResponse(
-      Request* request,
-      enterprise_connectors::ContentAnalysisResponse response);
-  void OnGetLegacyResponse(Request* request,
-                           DeepScanningClientResponse response);
+  void OnGetResponse(Request* request,
+                     enterprise_connectors::ContentAnalysisResponse response);
 
-  void MaybeFinishConnectorRequest(Request* request);
-  void MaybeFinishLegacyRequest(Request* request);
+  void MaybeFinishRequest(Request* request);
 
   void OnTimeout(Request* request);
 
@@ -292,7 +260,7 @@ class BinaryUploadService : public KeyedService {
                             Result result,
                             const DeepScanningClientResponse& response);
 
-  // Called at the end of either Finish{Connector|Legacy}Request methods.
+  // Called at the end of the FinishRequest method.
   void FinishRequestCleanup(Request* request, const std::string& instance_id);
 
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.cc

@@ -71,9 +71,6 @@ DeepScanningDialogDelegate::Factory* GetFactoryStorage() {
 // from a string.
 class StringSourceRequest : public BinaryUploadService::Request {
  public:
-  StringSourceRequest(GURL analysis_url,
-                      std::string text,
-                      BinaryUploadService::Callback callback);
   StringSourceRequest(GURL analysis_url,
                       std::string text,
                       BinaryUploadService::ContentAnalysisCallback callback);
@@ -91,17 +88,6 @@ class StringSourceRequest : public BinaryUploadService::Request {
       BinaryUploadService::Result::FILE_TOO_LARGE;
 };
 
-StringSourceRequest::StringSourceRequest(GURL analysis_url,
-                                         std::string text,
-                                         BinaryUploadService::Callback callback)
-    : Request(std::move(callback), analysis_url) {
-  // Only remember strings less than the maximum allowed.
-  if (text.size() < BinaryUploadService::kMaxUploadSizeBytes) {
-    data_.contents = std::move(text);
-    result_ = BinaryUploadService::Result::SUCCESS;
-  }
-}
-
 StringSourceRequest::StringSourceRequest(
     GURL analysis_url,
     std::string text,
@@ -632,7 +618,8 @@ void DeepScanningDialogDelegate::OnGotFileInfo(
   // property (too large, unsupported file type, encrypted, ...) that make its
   // upload pointless, so the request should finish early.
   if (result != BinaryUploadService::Result::SUCCESS) {
-    request->FinishRequest(result);
+    request->FinishRequest(result,
+                           enterprise_connectors::ContentAnalysisResponse());
     return;
   }
 

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate_browsertest.cc

@@ -52,7 +52,8 @@ class FakeBinaryUploadService : public BinaryUploadService {
   // Finish the authentication request. Called after ShowForWebContents to
   // simulate an async callback.
   void ReturnAuthorizedResponse() {
-    FinishRequest(authorization_request_.get(), authorization_result_);
+    FinishRequest(authorization_request_.get(), authorization_result_,
+                  enterprise_connectors::ContentAnalysisResponse());
   }
 
   void SetResponseForText(
@@ -87,13 +88,12 @@ class FakeBinaryUploadService : public BinaryUploadService {
     } else {
       std::string file = request->filename();
       if (file.empty()) {
-        request->FinishConnectorRequest(prepared_text_result_,
-                                        prepared_text_response_);
+        request->FinishRequest(prepared_text_result_, prepared_text_response_);
       } else {
         ASSERT_TRUE(prepared_file_results_.count(file));
         ASSERT_TRUE(prepared_file_responses_.count(file));
-        request->FinishConnectorRequest(prepared_file_results_[file],
-                                        prepared_file_responses_[file]);
+        request->FinishRequest(prepared_file_results_[file],
+                               prepared_file_responses_[file]);
       }
     }
   }

chrome/browser/safe_browsing/cloud_content_scanning/test_binary_upload_service.cc

@@ -18,17 +18,9 @@ TestBinaryUploadService::TestBinaryUploadService()
 void TestBinaryUploadService::MaybeUploadForDeepScanning(
     std::unique_ptr<Request> request) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (request->use_legacy_proto()) {
-    content::GetUIThreadTaskRunner({})->PostTask(
-        FROM_HERE,
-        base::BindOnce(&Request::FinishLegacyRequest, std::move(request),
-                       saved_result_, saved_response_));
-  } else {
-    content::GetUIThreadTaskRunner({})->PostTask(
-        FROM_HERE,
-        base::BindOnce(&Request::FinishConnectorRequest, std::move(request),
-                       saved_result_, saved_content_analysis_response_));
-  }
+  content::GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE, base::BindOnce(&Request::FinishRequest, std::move(request),
+                                saved_result_, saved_response_));
   was_called_ = true;
 }
 
@@ -36,12 +28,6 @@ void TestBinaryUploadService::SetResponse(
     Result result,
     enterprise_connectors::ContentAnalysisResponse response) {
   saved_result_ = result;
-  saved_content_analysis_response_ = response;
-}
-
-void TestBinaryUploadService::SetResponse(Result result,
-                                          DeepScanningClientResponse response) {
-  saved_result_ = result;
   saved_response_ = response;
 }
 

chrome/browser/safe_browsing/cloud_content_scanning/test_binary_upload_service.h

@@ -21,16 +21,13 @@ class TestBinaryUploadService : public BinaryUploadService {
   void MaybeUploadForDeepScanning(std::unique_ptr<Request> request) override;
   void SetResponse(Result result,
                    enterprise_connectors::ContentAnalysisResponse response);
-  void SetResponse(Result result, DeepScanningClientResponse response);
 
   bool was_called() { return was_called_; }
   void ClearWasCalled();
 
  private:
   Result saved_result_ = Result::UNKNOWN;
-  DeepScanningClientResponse saved_response_ = DeepScanningClientResponse();
-  enterprise_connectors::ContentAnalysisResponse
-      saved_content_analysis_response_;
+  enterprise_connectors::ContentAnalysisResponse saved_response_;
   bool was_called_ = false;
 };
 

chrome/browser/safe_browsing/download_protection/deep_scanning_request_unittest.cc

@@ -68,7 +68,7 @@ class FakeBinaryUploadService : public BinaryUploadService {
 
   void MaybeUploadForDeepScanning(std::unique_ptr<Request> request) override {
     last_request_ = request->content_analysis_request();
-    request->FinishConnectorRequest(saved_result_, saved_response_);
+    request->FinishRequest(saved_result_, saved_response_);
   }
 
   void SetResponse(BinaryUploadService::Result result,