Fix race condition in OnReceivedThreatDOMDetails.

If the window is closed after GetThreatDOMDetails finishes, but before
OnReceivedThreatDOMDetails is run, |sender| is invalid. Instead, we
observe FrameDeleted and track which frames are closed. Then we know
to skip this RenderFrameHost when GetThreatDOMDetails runs.

Bug: 817724, 860445
Change-Id: I2c8ed6c6a160264d91ca9b5a4760e53a7606f487
Reviewed-on: https://chromium-review.googlesource.com/1146997Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577206}

components/safe_browsing/browser/threat_details.cc

@@ -579,6 +579,7 @@ void ThreatDetails::RequestThreatDOMDetails(content::RenderFrameHost* frame) {
   frame->GetRemoteInterfaces()->GetInterface(&threat_reporter);
   safe_browsing::mojom::ThreatReporter* raw_threat_report =
       threat_reporter.get();
+  pending_render_frame_hosts_.insert(frame);
   raw_threat_report->GetThreatDOMDetails(
       base::BindOnce(&ThreatDetails::OnReceivedThreatDOMDetails, this,
                      base::Passed(&threat_reporter), frame));
@@ -589,6 +590,11 @@ void ThreatDetails::OnReceivedThreatDOMDetails(
     mojom::ThreatReporterPtr threat_reporter,
     content::RenderFrameHost* sender,
     std::vector<mojom::ThreatDOMDetailsNodePtr> params) {
+  // If the RenderFrameHost was closed between sending the IPC and this callback
+  // running, |sender| will be invalid.
+  if (pending_render_frame_hosts_.erase(sender) == 0)
+    return;
+
   // Lookup the FrameTreeNode ID of any child frames in the list of DOM nodes.
   const int sender_process_id = sender->GetProcess()->GetID();
   const int sender_frame_tree_node_id = sender->GetFrameTreeNodeId();
@@ -806,4 +812,9 @@ void ThreatDetails::AllDone() {
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(done_callback_, base::Unretained(web_contents())));
 }
+
+void ThreatDetails::FrameDeleted(RenderFrameHost* render_frame_host) {
+  pending_render_frame_hosts_.erase(render_frame_host);
+}
+
 }  // namespace safe_browsing

components/safe_browsing/browser/threat_details.h

@@ -101,6 +101,9 @@ class ThreatDetails : public base::RefCounted<ThreatDetails>,
 
   void OnRedirectionCollectionReady();
 
+  // WebContentsObserver implementation:
+  void FrameDeleted(content::RenderFrameHost* render_frame_host) override;
+
  protected:
   friend class ThreatDetailsFactoryImpl;
   friend class TestThreatDetailsFactory;
@@ -260,6 +263,10 @@ class ThreatDetails : public base::RefCounted<ThreatDetails>,
   // Whether the |done_callback_| has been invoked.
   bool is_all_done_;
 
+  // The set of RenderFrameHosts that have pending requests and haven't been
+  // deleted.
+  std::set<content::RenderFrameHost*> pending_render_frame_hosts_;
+
   FRIEND_TEST_ALL_PREFIXES(ThreatDetailsTest, HistoryServiceUrls);
   FRIEND_TEST_ALL_PREFIXES(ThreatDetailsTest, HttpsResourceSanitization);
   FRIEND_TEST_ALL_PREFIXES(ThreatDetailsTest, HTTPCacheNoEntries);