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

chrome/browser/history/history_backend.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/history/history_backend.h"
 
 #include <list>
 #include <map>
 #include <set>
 #include <vector>
@@ skipping 1058 matching lines @@
     db_->GetMostRecentKeywordSearchTerms(keyword_id, prefix, max_count,
                                          &(request->value));
   }
   request->ForwardResult(
       GetMostRecentKeywordSearchTermsRequest::TupleType(request->handle(),
                                                         &request->value));
 }
 
 // Downloads -------------------------------------------------------------------
 
+void HistoryBackend::GetNextDownloadId(
+    scoped_refptr<DownloadNextIdRequest> request) {
+  if (request->canceled()) return;
+  if (db_.get()) {
+    request->value = db_->next_download_id();
+  }
+  request->ForwardResult(DownloadNextIdRequest::TupleType(request->value));

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

This seems like it'll leave an uninitialized value in the case where db_.get()
is false.  Shouldn't we put some kind of value in?  (You should probably try and
figure out in which cases that can happen so we know what the right way to
handle it is.)

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

Done.

For this cl, while DownloadManager says next_id_ += next_id; setting it to 0
turns DownloadManager::OnHistoryGetNextId() into a noop.
I think that setting it to 0 will be ok even for future CLs because, if there's
no database, then there won't be any download items loaded from the database,
and new download items won't be written to the database, because there's no
database. That logic works as long as the db_ isn't initialized after
GetNextDownloadId, which we can ensure in ProfileImpl.

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

Yes--no collisions to avoid.  There may be circumstances in which db_ is null
when there is a database; there are comments sprinkled through the code about
"the database being offline".  But I believe in those circumstances we don't
talk to the database (can't initialize from it, can't save to it) so we're ok.  

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

Done.

+}
+
 // Get all the download entries from the database.
 void HistoryBackend::QueryDownloads(
     scoped_refptr<DownloadQueryRequest> request) {
   if (request->canceled())
     return;
   if (db_.get())
     db_->QueryDownloads(&request->value);
   request->ForwardResult(DownloadQueryRequest::TupleType(&request->value));
 }
 
@@ skipping 1140 matching lines @@
   return true;
 }
 
 BookmarkService* HistoryBackend::GetBookmarkService() {
   if (bookmark_service_)
     bookmark_service_->BlockTillLoaded();
   return bookmark_service_;
 }
 
 }  // namespace history