Do not CHECK on the result of Sql::Statement::Run.

content/browser/media/webrtc_identity_store_backend.cc

Index: content/browser/media/webrtc_identity_store_backend.cc
diff --git a/content/browser/media/webrtc_identity_store_backend.cc b/content/browser/media/webrtc_identity_store_backend.cc
index d599dcda742d08d3ffb80598acabdf432bb5a3d0..2968c41492b9741d3475bf74e6bbfc4154b3ab36 100644
--- a/content/browser/media/webrtc_identity_store_backend.cc
+++ b/content/browser/media/webrtc_identity_store_backend.cc
@@ -6,6 +6,7 @@
 
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/strings/string_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/net_errors.h"
 #include "sql/error_delegate_util.h"
@@ -31,6 +32,7 @@ static bool InitDB(sql::Connection* db) {
         db->DoesColumnExist(kWebRTCIdentityStoreDBName, "private_key") &&
         db->DoesColumnExist(kWebRTCIdentityStoreDBName, "creation_time"))
       return true;
+
     if (!db->Execute("DROP TABLE webrtc_identity_store"))
       return false;
   }
@@ -343,7 +345,7 @@ void WebRTCIdentityStoreBackend::OnLoaded(scoped_ptr<IdentityMap> out_map) {
   if (state_ != LOADING)
     return;
 
-  DVLOG(2) << "WebRTC identity store has loaded.";
+  DVLOG(3) << "WebRTC identity store has loaded.";
 
   state_ = LOADED;
   identities_.swap(*out_map);
@@ -370,7 +372,7 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Load(IdentityMap* out_map) {
   // from it.
   const base::FilePath dir = path_.DirName();
   if (!base::PathExists(dir) && !base::CreateDirectory(dir)) {
-    DLOG(ERROR) << "Unable to open DB file path.";
+    DVLOG(2) << "Unable to open DB file path.";
     return;
   }
 
@@ -379,13 +381,13 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Load(IdentityMap* out_map) {
   db_->set_error_callback(base::Bind(&SqlLiteStorage::OnDatabaseError, this));
 
   if (!db_->Open(path_)) {
-    DLOG(ERROR) << "Unable to open DB.";
+    DVLOG(2) << "Unable to open DB.";
     db_.reset();
     return;
   }
 
   if (!InitDB(db_.get())) {
-    DLOG(ERROR) << "Unable to init DB.";
+    DVLOG(2) << "Unable to init DB.";
     db_.reset();
     return;
   }
@@ -470,21 +472,26 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::DeleteBetween(
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin()) {
-    DLOG(ERROR) << "Failed to begin the transaction.";
+    DVLOG(2) << "Failed to begin the transaction.";
     return;
   }
 
-  CHECK(del_stmt.Run());
-  transaction.Commit();
+  if (!del_stmt.Run()) {
+    DVLOG(2) << "Failed to run the delete statement.";
+    return;
+  }
+
+  if (!transaction.Commit())
+    DVLOG(2) << "Failed to commit the transaction.";
 }
 
 void WebRTCIdentityStoreBackend::SqlLiteStorage::OnDatabaseError(
     int error,
     sql::Statement* stmt) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-  if (!sql::IsErrorCatastrophic(error))
-    return;
+
   db_->RazeAndClose();
+  // It's not safe to reset |db_| here.
 }
 
 void WebRTCIdentityStoreBackend::SqlLiteStorage::BatchOperation(
@@ -542,7 +549,7 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Commit() {
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin()) {
-    DLOG(ERROR) << "Failed to begin the transaction.";
+    DVLOG(2) << "Failed to begin the transaction.";
     return;
   }
 
@@ -561,14 +568,20 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Commit() {
         const std::string& private_key = po->identity.private_key;
         add_stmt.BindBlob(4, private_key.data(), private_key.size());
         add_stmt.BindInt64(5, po->identity.creation_time);
-        CHECK(add_stmt.Run());
+        if (!add_stmt.Run()) {
+          DVLOG(2) << "Failed to add the identity to DB.";
+          return;

[Ami GONE FROM CHROMIUM, 2014/05/27 19:25:06]

In case of error here or at l.583 or 595 below, the rest of pending_operations_
will not be tried, and p_o_ will not be cleared (it may contain a mix of ptrs
pointing to live & freed memory, too).  Is that ok?
(should the first two return's be continue's?  Should p_o_.clear() be called no
matter what?)

[jiayl, 2014/05/27 20:57:25]

Fixed now. Always clear p_o_.

+        }
         break;
       }
       case DELETE_IDENTITY:
         del_stmt.Reset(true);
         del_stmt.BindString(0, po->origin.spec());
         del_stmt.BindString(1, po->identity_name);
-        CHECK(del_stmt.Run());
+        if (!del_stmt.Run()) {
+          DVLOG(2) << "Failed to delete the identity from DB.";
+          return;
+        }
         break;
 
       default:
@@ -576,7 +589,12 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Commit() {
         break;
     }
   }
-  transaction.Commit();
+
+  if (!transaction.Commit()) {
+    DVLOG(2) << "Failed to commit the transaction.";
+    return;
+  }
+
   pending_operations_.clear();
 }