Handle the possibility of failure when starting a

SafeBrowsing update transaction.

If we fail to start the transaction, we report the error
to the protocol manager which aborts the update process.
Review URL: http://codereview.chromium.org/9778

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5207 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 10 insertions, 26 deletions

diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 1e760c6..bc37759 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -272,7 +272,6 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url,
       }
 
       last_update_ = Time::Now();
-      sb_service_->UpdateStarted();
 
       if (update_state_ == FIRST_REQUEST)
         update_state_ = SECOND_REQUEST;
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.h b/chrome/browser/safe_browsing/safe_browsing_database.h
index 9e07297..e504054 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database.h
@@ -82,7 +82,7 @@ class SafeBrowsingDatabase {
   // Called when the user's machine has resumed from a lower power state.
   virtual void HandleResume() = 0;
 
-  virtual void UpdateStarted() {}
+  virtual bool UpdateStarted() { return true; }
   virtual void UpdateFinished(bool update_succeeded) {}
 
   virtual std::wstring filename() const { return filename_; }
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
index e20fa87..d47b94f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.cc
@@ -381,15 +381,15 @@ void SafeBrowsingDatabaseBloom::InsertChunks(const std::string& list_name,
     chunk_inserted_callback_->Run();
 }
 
-void SafeBrowsingDatabaseBloom::UpdateStarted() {
+bool SafeBrowsingDatabaseBloom::UpdateStarted() {
   DCHECK(insert_transaction_.get() == NULL);
   insert_transaction_.reset(new SQLTransaction(db_));
   if (insert_transaction_->Begin() != SQLITE_OK) {
-    // TODO(paulg): We should abort the update, and possibly enter back off mode
-    // if we have a problem with the database.
     DCHECK(false) << "Safe browsing database couldn't start transaction";
     insert_transaction_.reset();
+    return false;
   }
+  return true;
 }
 
 void SafeBrowsingDatabaseBloom::UpdateFinished(bool update_succeeded) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_database_bloom.h b/chrome/browser/safe_browsing/safe_browsing_database_bloom.h
index 9cebde6..7ecb2ab 100644
--- a/chrome/browser/safe_browsing/safe_browsing_database_bloom.h
+++ b/chrome/browser/safe_browsing/safe_browsing_database_bloom.h
@@ -72,7 +72,8 @@ class SafeBrowsingDatabaseBloom : public SafeBrowsingDatabase {
   // Called when the user's machine has resumed from a lower power state.
   virtual void HandleResume();
 
-  virtual void UpdateStarted();
+  // Returns true if we have successfully started the update transaction.
+  virtual bool UpdateStarted();
   virtual void UpdateFinished(bool update_succeeded);
 
   virtual bool NeedToCheckUrl(const GURL& url);
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 5a2f257..792a4d8 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -465,13 +465,6 @@ void SafeBrowsingService::GetAllChunks() {
       this, &SafeBrowsingService::GetAllChunksFromDatabase));
 }
 
-void SafeBrowsingService::UpdateStarted() {
-  DCHECK(MessageLoop::current() == io_loop_);
-  DCHECK(enabled_);
-  db_thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &SafeBrowsingService::DatabaseUpdateStarted));
-}
-
 void SafeBrowsingService::UpdateFinished(bool update_succeeded) {
   DCHECK(MessageLoop::current() == io_loop_);
   DCHECK(enabled_);
@@ -479,12 +472,6 @@ void SafeBrowsingService::UpdateFinished(bool update_succeeded) {
       this, &SafeBrowsingService::DatabaseUpdateFinished, update_succeeded));
 }
 
-void SafeBrowsingService::DatabaseUpdateStarted() {
-  DCHECK(MessageLoop::current() == db_thread_->message_loop());
-  if (GetDatabase())
-    GetDatabase()->UpdateStarted();
-}
-
 void SafeBrowsingService::DatabaseUpdateFinished(bool update_succeeded) {
   DCHECK(MessageLoop::current() == db_thread_->message_loop());
   if (GetDatabase())
@@ -606,12 +593,11 @@ void SafeBrowsingService::DeleteChunks(
 // Database worker function.
 void SafeBrowsingService::GetAllChunksFromDatabase() {
   DCHECK(MessageLoop::current() == db_thread_->message_loop());
-  bool database_error = false;
+  bool database_error = true;
   std::vector<SBListChunkRanges> lists;
-  if (GetDatabase()) {
+  if (GetDatabase() && GetDatabase()->UpdateStarted()) {
     GetDatabase()->GetListsInfo(&lists);
-  } else {
-    database_error = true;
+    database_error = false;
   }
 
   io_loop_->PostTask(FROM_HERE, NewRunnableMethod(
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index d85dd89..9c07636 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -136,8 +136,7 @@ class SafeBrowsingService
   void HandleChunkDelete(std::vector<SBChunkDelete>* chunk_deletes);
   void GetAllChunks();
 
-  // Methods called to indicate the beginning and end of a complete update.
-  void UpdateStarted();
+  // Called when a complete update cycle has finished.
   void UpdateFinished(bool update_succeeded);
 
   // The blocking page on the UI thread has completed.
@@ -209,7 +208,6 @@ class SafeBrowsingService
 
   void NotifyClientBlockingComplete(Client* client, bool proceed);
 
-  void DatabaseUpdateStarted();
   void DatabaseUpdateFinished(bool update_succeeded);
 
   void Start();