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

chrome/browser/renderer_host/download_throttling_resource_handler.cc

 // Copyright (c) 2011 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.
 
 #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"
 
 DownloadThrottlingResourceHandler::DownloadThrottlingResourceHandler(
     ResourceDispatcherHost* host,
     net::URLRequest* request,
     const GURL& url,
     int render_process_host_id,
     int render_view_id,
@@ skipping 145 matching lines @@
 
 void DownloadThrottlingResourceHandler::CancelDownload() {
   if (!request_closed_)
     host_->CancelRequest(render_process_host_id_, request_id_, false);
   Release();  // Release the additional reference from constructor.
 }
 
 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,
                                     DownloadSaveInfo());
     if (response_.get())
       download_handler_->OnResponseStarted(request_id_, response_.get());
 
     if (tmp_buffer_length_)
       CopyTmpBufferToDownloadHandler();
 
     // And let the request continue.
     host_->PauseRequest(render_process_host_id_, request_id_, false);
   }
   Release();  // Release the addtional reference from constructor.
 }
 
 void DownloadThrottlingResourceHandler::CopyTmpBufferToDownloadHandler() {
   // Copy over the tmp buffer.
   net::IOBuffer* buffer;
   int buf_size;
   if (download_handler_->OnWillRead(request_id_, &buffer, &buf_size,
                                     tmp_buffer_length_)) {
     CHECK(buf_size >= tmp_buffer_length_);
     memcpy(buffer->data(), tmp_buffer_->data(), tmp_buffer_length_);
     download_handler_->OnReadCompleted(request_id_, &tmp_buffer_length_);
   }
   tmp_buffer_length_ = 0;
   tmp_buffer_ = NULL;
 }