Make sure we end the update process if we get an errorfrom the servers. Previously, if we got a 400 (or other)error response, we'd leave the database and transactionopen.BUG=5060 (http://crbug.com/5060)

Review URL: http://codereview.chromium.org/12918

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

3 files changed, 18 insertions, 8 deletions

diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 0f43a03..cd2c1be 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -218,6 +218,7 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
                       << "failed parse.";
         must_back_off = true;
         chunk_request_urls_.clear();
+        sb_service_->UpdateFinished(false);
       }
 
       if (request_type_ == CHUNK_REQUEST && parsed_ok) {
@@ -234,6 +235,7 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
       must_back_off = true;
       if (request_type_ == CHUNK_REQUEST)
         chunk_request_urls_.clear();
+      sb_service_->UpdateFinished(false);
       SB_DLOG(INFO) << "SafeBrowsing request for: " << source->url()
                     << ", failed with error: " << response_code;
     }
@@ -267,7 +269,6 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url,
                               &next_update_sec, &re_key,
                               &reset, chunk_deletes, &chunk_urls)) {
         delete chunk_deletes;
-        sb_service_->UpdateFinished(false);
         return false;
       }
 
@@ -433,7 +434,7 @@ int SafeBrowsingProtocolManager::GetNextBackOffTime(int* error_count,
 //              isn't that much overhead. Measure!
 void SafeBrowsingProtocolManager::IssueUpdateRequest() {
   request_type_ = UPDATE_REQUEST;
-  sb_service_->GetAllChunks();
+  sb_service_->UpdateStarted();
 }
 
 void SafeBrowsingProtocolManager::IssueChunkRequest() {
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 3a6fff3..d60f409 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -32,7 +32,8 @@ SafeBrowsingService::SafeBrowsingService()
       protocol_manager_(NULL),
       enabled_(false),
       resetting_(false),
-      database_loaded_(false) {
+      database_loaded_(false),
+      update_in_progress_(false) {
   new_safe_browsing_ = !CommandLine().HasSwitch(switches::kUseOldSafeBrowsing);
 }
 
@@ -469,9 +470,11 @@ void SafeBrowsingService::HandleOneCheck(
   delete check;
 }
 
-void SafeBrowsingService::GetAllChunks() {
+void SafeBrowsingService::UpdateStarted() {
   DCHECK(MessageLoop::current() == io_loop_);
   DCHECK(enabled_);
+  DCHECK(!update_in_progress_);
+  update_in_progress_ = true;
   db_thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
       this, &SafeBrowsingService::GetAllChunksFromDatabase));
 }
@@ -479,8 +482,11 @@ void SafeBrowsingService::GetAllChunks() {
 void SafeBrowsingService::UpdateFinished(bool update_succeeded) {
   DCHECK(MessageLoop::current() == io_loop_);
   DCHECK(enabled_);
-  db_thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &SafeBrowsingService::DatabaseUpdateFinished, update_succeeded));
+  if (update_in_progress_) {
+    update_in_progress_ = false;
+    db_thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
+        this, &SafeBrowsingService::DatabaseUpdateFinished, update_succeeded));
+  }
 }
 
 void SafeBrowsingService::DatabaseUpdateFinished(bool update_succeeded) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 761acb2..5b02aeb4 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -134,9 +134,9 @@ class SafeBrowsingService
       bool can_cache);
   void HandleChunk(const std::string& list, std::deque<SBChunk>* chunks);
   void HandleChunkDelete(std::vector<SBChunkDelete>* chunk_deletes);
-  void GetAllChunks();
 
-  // Called when a complete update cycle has finished.
+  // Update management.
+  void UpdateStarted();
   void UpdateFinished(bool update_succeeded);
 
   // The blocking page on the UI thread has completed.
@@ -290,6 +290,9 @@ class SafeBrowsingService
   // Indicates if the database has finished initialization.
   bool database_loaded_;
 
+  // Indicates if we're currently in an update cycle.
+  bool update_in_progress_;
+
   // Clients that we've queued up for checking later once the database is ready.
   typedef struct {
     Client* client;