Use ResourceRequestInfo::GetWebContents in DownloadRequestLimiter

chrome/browser/download/download_request_limiter.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_LIMITER_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_LIMITER_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 164 matching lines @@
 
   static void SetContentSettingsForTesting(HostContentSettingsMap* settings);
 
   DownloadRequestLimiter();
 
   // Returns the download status for a page. This does not change the state in
   // anyway.
   DownloadStatus GetDownloadStatus(content::WebContents* tab);
 
   // Check if download can proceed and notifies the callback on UI thread.
-  void CanDownload(int render_process_host_id,
-                   int render_view_id,
-                   const GURL& url,
-                   const std::string& request_method,
-                   const Callback& callback);
+  void CanDownload(
+      const base::Callback<content::WebContents*(void)>& web_contents_getter,

[asanka, 2015/11/24 16:13:19]

Instead of a bunch of naked base::Callback<>s, could you define a type somewhere
and use it here? That way you can describe what the callback does and its
guarantees. A naked callback doesn't imply anything about thread restrictions,
lifetime guarantees, validity etc.. (E.g. a callback that's bound to a method on
an object referenced via a weak pointer will become null when the weak reference
is invalidated. It's not sufficient that the callback be bound. For a callback
to be valid, the weak reference to the object against which the method call is
indirected must also be valid).

Everywhere you use the callback, you are assume that it's non-null with no API
contract anywhere that it is.

[clamy, 2015/11/25 14:02:02]

I went back to having a typedef in ResourceRequestInfo, with additional
precision about the callback.

+      const GURL& url,
+      const std::string& request_method,
+      const Callback& callback);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(DownloadTest, DownloadResourceThrottleCancels);
   friend class base::RefCountedThreadSafe<DownloadRequestLimiter>;
   friend class DownloadRequestLimiterTest;
   friend class TabDownloadState;
 
   ~DownloadRequestLimiter();
 
   // Gets the download state for the specified controller. If the
   // TabDownloadState does not exist and |create| is true, one is created.
   // See TabDownloadState's constructor description for details on the two
   // controllers.
   //
   // The returned TabDownloadState is owned by the DownloadRequestLimiter and
   // deleted when no longer needed (the Remove method is invoked).
   TabDownloadState* GetDownloadState(
       content::WebContents* web_contents,
       content::WebContents* originating_web_contents,
       bool create);
 
   // Does the work of updating the download status on the UI thread and
   // potentially prompting the user.
   void CanDownloadImpl(content::WebContents* originating_contents,
                        const std::string& request_method,
                        const Callback& callback);
 
   // Invoked when decision to download has been made.
-  void OnCanDownloadDecided(int render_process_host_id,
-                            int render_view_id,
-                            const std::string& request_method,
-                            const Callback& orig_callback,
-                            bool allow);
+  void OnCanDownloadDecided(
+      const base::Callback<content::WebContents*(void)>& web_contents_getter,
+      const std::string& request_method,
+      const Callback& orig_callback,
+      bool allow);
 
   // Removes the specified TabDownloadState from the internal map and deletes
   // it. This has the effect of resetting the status for the tab to
   // ALLOW_ONE_DOWNLOAD.
   void Remove(TabDownloadState* state, content::WebContents* contents);
 
   static HostContentSettingsMap* content_settings_;
   static HostContentSettingsMap* GetContentSettings(
       content::WebContents* contents);
 
   // Maps from tab to download state. The download state for a tab only exists
   // if the state is other than ALLOW_ONE_DOWNLOAD. Similarly once the state
   // transitions from anything but ALLOW_ONE_DOWNLOAD back to ALLOW_ONE_DOWNLOAD
   // the TabDownloadState is removed and deleted (by way of Remove).
   typedef std::map<content::WebContents*, TabDownloadState*> StateMap;
   StateMap state_map_;
 
   // Weak ptr factory used when |CanDownload| asks the delegate asynchronously
   // about the download.
   base::WeakPtrFactory<DownloadRequestLimiter> factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadRequestLimiter);
 };
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_REQUEST_LIMITER_H_