sql::MetaTable.next_download_id, DownloadManager::GetNextId()

chrome/browser/renderer_host/download_throttling_resource_handler.cc

Index: chrome/browser/renderer_host/download_throttling_resource_handler.cc
diff --git a/chrome/browser/renderer_host/download_throttling_resource_handler.cc b/chrome/browser/renderer_host/download_throttling_resource_handler.cc
index 34d90aee6376ad7696c04b3086fde073d21a3117..25e83140fdfe5161d429478a322e911481ba26f2 100644
--- a/chrome/browser/renderer_host/download_throttling_resource_handler.cc
+++ b/chrome/browser/renderer_host/download_throttling_resource_handler.cc
@@ -5,9 +5,12 @@
 #include "chrome/browser/renderer_host/download_throttling_resource_handler.h"
 
 #include "base/logging.h"
+#include "chrome/browser/download/download_id.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/renderer_host/download_resource_handler.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
+#include "content/browser/renderer_host/resource_dispatcher_host_request_info.h"
+#include "content/browser/resource_context.h"
 #include "content/common/resource_response.h"
 #include "net/base/io_buffer.h"
 #include "net/base/mime_sniffer.h"
@@ -173,12 +176,23 @@ void DownloadThrottlingResourceHandler::CancelDownload() {
 void DownloadThrottlingResourceHandler::ContinueDownload() {
   DCHECK(!download_handler_.get());
   if (!request_closed_) {
+    ResourceDispatcherHostRequestInfo* info =
+        ResourceDispatcherHost::InfoForRequest(request_);
+    DownloadId dl_id;
+    void* next_download_id_thunk = info->context()->GetUserData(
+        reinterpret_cast<void*>(BASE_HASH_NAMESPACE::hash<std::string>()(
+            "next_download_id_thunk")));
+    if (next_download_id_thunk != NULL) {

[Randy Smith (Not in Mondays), 2011/07/28 21:03:16]

Under what circumstances can next_download_id_thunk be null?

If next_download_id_thunk's null, we'll DCHECK in the download resource handler
constructor, and I think DCHECKs are more useful the closer they are to the
original source of the error, so I'd argue for one here as well.

[benjhayden, 2011/08/03 17:44:46]

Whenever a ProfileIOData hasn't set it. I'm new here.
Done.

[Randy Smith (Not in Mondays), 2011/08/03 21:10:29]

:-}.  

I'm afraid you still get the unenviable task of nailing down the answer to the
question, since if there are cases in which that can happen in normal operation,
this code'll produce a crash (debug mode) or random values and possibly behavior
(debug mode).  Sorry :-{.

[benjhayden, 2011/08/04 17:15:00]

Apologies for the smartass. Haven't been sleeping well.
I'll start studying ResourceContext today.

[benjhayden, 2011/08/04 18:49:44]

According to codesearch, ProfileIOData is the only thing that instantiates the
only subclass of ResourceContext.
So, there can't be a ResourceContext that wasn't initialized by ProfileIOData,
which always sets the thunk.
I can change the if to a check if you want.

[Randy Smith (Not in Mondays), 2011/08/04 19:52:27]

No worries; sorry if I came across as harsh.

[Randy Smith (Not in Mondays), 2011/08/04 19:52:27]

Yes, please.  The problem is that the if implies that we might not go through
that loop, and in that case we have an uninitialized download id, which is bad. 
If we believe we're never going to have an uninitialized download id, let's have
the code reflect that, which means a check.

[benjhayden, 2011/08/04 21:05:13]

Done.

[benjhayden, 2011/08/04 21:05:13]

Not at all.

+      dl_id = reinterpret_cast<DownloadManager::GetNextIdThunkType*>(
+          next_download_id_thunk)->Run();
+    }
     download_handler_ =
         new DownloadResourceHandler(host_,
                                     render_process_host_id_,
                                     render_view_id_,
                                     request_id_,
                                     url_,
+                                    dl_id,
                                     host_->download_file_manager(),
                                     request_,
                                     false,