Show scan requests immediately on chrome://safe-browsing/#tab-deep-scan

If a scan is hanging, it's useful to see information about it early.
Previously we were only adding the request after the response had
came back.

Change-Id: Iecda3038e46941356d908427266b6b9847e2500a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2101703
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750646}

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.cc

@@ -242,6 +242,9 @@ void BinaryUploadService::OnGetRequestData(Request* request,
                      weakptr_factory_.GetWeakPtr(), request));
   upload_request->Start();
   active_uploads_[request] = std::move(upload_request);
+
+  WebUIInfoSingleton::GetInstance()->AddToDeepScanRequests(
+      request->deep_scanning_request());
 }
 
 void BinaryUploadService::OnUploadComplete(Request* request,
@@ -329,6 +332,9 @@ void BinaryUploadService::FinishRequest(Request* request,
                                         Result result,
                                         DeepScanningClientResponse response) {
   RecordRequestMetrics(request, result, response);
+
+  // We add the request here in case we never actually uploaded anything, so it
+  // wasn't added in OnGetRequestData
   WebUIInfoSingleton::GetInstance()->AddToDeepScanRequests(
       request->deep_scanning_request());
   WebUIInfoSingleton::GetInstance()->AddToDeepScanResponses(

components/safe_browsing/content/web_ui/safe_browsing_ui.cc

@@ -250,17 +250,18 @@ void WebUIInfoSingleton::ClearReportingEvents() {
 }
 
 #if BUILDFLAG(FULL_SAFE_BROWSING)
-int WebUIInfoSingleton::AddToDeepScanRequests(
+void WebUIInfoSingleton::AddToDeepScanRequests(
     const DeepScanningClientRequest& request) {
   if (!HasListener())
-    return -1;
-
-  for (auto* webui_listener : webui_instances_)
-    webui_listener->NotifyDeepScanRequestJsListener(request);
+    return;
 
-  deep_scan_requests_.push_back(request);
+  if (deep_scan_requests_.find(request.request_token()) ==
+      deep_scan_requests_.end()) {
+    for (auto* webui_listener : webui_instances_)
+      webui_listener->NotifyDeepScanRequestJsListener(request);
+  }
 
-  return pg_pings_.size() - 1;
+  deep_scan_requests_[request.request_token()] = request;
 }
 
 void WebUIInfoSingleton::AddToDeepScanResponses(
@@ -277,9 +278,8 @@ void WebUIInfoSingleton::AddToDeepScanResponses(
 }
 
 void WebUIInfoSingleton::ClearDeepScans() {
-  std::vector<DeepScanningClientRequest>().swap(deep_scan_requests_);
-  std::map<std::string, std::pair<std::string, DeepScanningClientResponse>>()
-      .swap(deep_scan_responses_);
+  DeepScanningRequestMap().swap(deep_scan_requests_);
+  StatusAndDeepScanningResponseMap().swap(deep_scan_responses_);
 }
 #endif
 void WebUIInfoSingleton::RegisterWebUIInstance(SafeBrowsingUIHandler* webui) {
@@ -1823,16 +1823,13 @@ void SafeBrowsingUIHandler::GetLogMessages(const base::ListValue* args) {
 
 #if BUILDFLAG(FULL_SAFE_BROWSING)
 void SafeBrowsingUIHandler::GetDeepScanRequests(const base::ListValue* args) {
-  const std::vector<DeepScanningClientRequest>& requests =
-      WebUIInfoSingleton::GetInstance()->deep_scan_requests();
-
   base::ListValue pings_sent;
-  for (size_t request_index = 0; request_index < requests.size();
-       request_index++) {
+  for (const auto& token_and_request :
+       WebUIInfoSingleton::GetInstance()->deep_scan_requests()) {
     base::ListValue ping_entry;
-    ping_entry.Append(base::Value(int(request_index)));
+    ping_entry.Append(base::Value(token_and_request.first));
     ping_entry.Append(
-        base::Value(SerializeDeepScanningRequest(requests[request_index])));
+        base::Value(SerializeDeepScanningRequest(token_and_request.second)));
     pings_sent.Append(std::move(ping_entry));
   }
 
@@ -1843,9 +1840,8 @@ void SafeBrowsingUIHandler::GetDeepScanRequests(const base::ListValue* args) {
 }
 
 void SafeBrowsingUIHandler::GetDeepScanResponses(const base::ListValue* args) {
-  const std::map<std::string,
-                 std::pair<std::string, DeepScanningClientResponse>>
-      responses = WebUIInfoSingleton::GetInstance()->deep_scan_responses();
+  const WebUIInfoSingleton::StatusAndDeepScanningResponseMap& responses =
+      WebUIInfoSingleton::GetInstance()->deep_scan_responses();
 
   base::ListValue responses_sent;
   for (const auto& token_and_status_and_response : responses) {

components/safe_browsing/content/web_ui/safe_browsing_ui.h

@@ -220,6 +220,13 @@ class SafeBrowsingUI : public content::WebUIController {
 
 class WebUIInfoSingleton {
  public:
+#if BUILDFLAG(FULL_SAFE_BROWSING)
+  using DeepScanningRequestMap =
+      std::map<std::string, DeepScanningClientRequest>;
+  using StatusAndDeepScanningResponseMap =
+      std::map<std::string, std::pair<std::string, DeepScanningClientResponse>>;
+#endif
+
   static WebUIInfoSingleton* GetInstance();
 
   // Returns true when there is a listening chrome://safe-browsing tab.
@@ -306,9 +313,10 @@ class WebUIInfoSingleton {
 
 #if BUILDFLAG(FULL_SAFE_BROWSING)
   // Add the new request to |deep_scan_requests_| and send it to all the open
-  // chrome://safe-browsing tabs. Returns a token that can be used in
-  // |AddToDeepScanResponses| to correlate a ping and response.
-  int AddToDeepScanRequests(const DeepScanningClientRequest& request);
+  // chrome://safe-browsing tabs. Uses |request.request_token()| as an
+  // identifier that can be used in |AddToDeepScanResponses| to correlate a ping
+  // and response.
+  void AddToDeepScanRequests(const DeepScanningClientRequest& request);
 
   // Add the new response to |deep_scan_responses_| and send it to all the open
   // chrome://safe-browsing tabs.
@@ -389,17 +397,16 @@ class WebUIInfoSingleton {
   }
 
 #if BUILDFLAG(FULL_SAFE_BROWSING)
-  // Get the list of deep scanning requests since the oldest currently open
-  // chrome://safe-browsing tab was opened.
-  const std::vector<DeepScanningClientRequest>& deep_scan_requests() const {
+  // Get the collection of deep scanning requests since the oldest currently
+  // open chrome://safe-browsing tab was opened. Returns a map from a unique
+  // token to the request proto.
+  const DeepScanningRequestMap& deep_scan_requests() const {
     return deep_scan_requests_;
   }
 
   // Get the list of deep scan responses since the oldest currently open
   // chrome://safe-browsing tab was opened.
-  const std::map<std::string,
-                 std::pair<std::string, DeepScanningClientResponse>>&
-  deep_scan_responses() const {
+  const StatusAndDeepScanningResponseMap& deep_scan_responses() const {
     return deep_scan_responses_;
   }
 #endif
@@ -498,14 +505,14 @@ class WebUIInfoSingleton {
   std::vector<base::Value> reporting_events_;
 
 #if BUILDFLAG(FULL_SAFE_BROWSING)
-  // List of deep scan requests sent since the oldest currently open
-  // chrome://safe-browsing tab was opened.
-  std::vector<DeepScanningClientRequest> deep_scan_requests_;
+  // Map of deep scan requests sent since the oldest currently open
+  // chrome://safe-browsing tab was opened. Maps from the unique token per
+  // request to the request proto.
+  DeepScanningRequestMap deep_scan_requests_;
 
   // List of deep scan responses received since the oldest currently open
   // chrome://safe-browsing tab was opened.
-  std::map<std::string, std::pair<std::string, DeepScanningClientResponse>>
-      deep_scan_responses_;
+  StatusAndDeepScanningResponseMap deep_scan_responses_;
 #endif
 
   // The current referrer chain provider, if any. Can be nullptr.