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..d4c518a8cf6f6c5d75146fc3c7444a2fd762a48b 100644
--- a/content/browser/media/webrtc_identity_store_backend.cc
+++ b/content/browser/media/webrtc_identity_store_backend.cc
@@ -343,7 +343,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 +370,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 +379,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,12 +470,17 @@ 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(
@@ -484,7 +489,11 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::OnDatabaseError(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   if (!sql::IsErrorCatastrophic(error))

[Ami GONE FROM CHROMIUM, 2014/05/23 23:05:37]

CL description implies that all the things that previously were CHECK failures
would now be handled by OnDatabaseError but this has the potential to falsify
that.  Do you have good reason to believe this is safe?

Should you track an "end_the_world_" bool that any detected failure should cause
to make this function treat as catastrophic?

[jiayl, 2014/05/23 23:25:40]

Changed to always RazeAndClose for "DELETE" failures. Transient "INSERT"
failures are safe to ignore.

     return;
+
+  DVLOG(2) << "Database error " << error << " for statement "
+           << stmt->GetSQLStatement();
   db_->RazeAndClose();
+  db_.reset();
 }
 
 void WebRTCIdentityStoreBackend::SqlLiteStorage::BatchOperation(
@@ -542,7 +551,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 +570,16 @@ 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.";

[Ami GONE FROM CHROMIUM, 2014/05/23 23:05:37]

return?

[jiayl, 2014/05/23 23:25:40]

Done.

         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.";

[Ami GONE FROM CHROMIUM, 2014/05/23 23:05:37]

ditto

[jiayl, 2014/05/23 23:25:40]

Done.

         break;
 
       default:
@@ -576,7 +587,10 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Commit() {
         break;
     }
   }
-  transaction.Commit();
+
+  if (!transaction.Commit())
+    DVLOG(2) << "Failed to commit the transaction.";
+
   pending_operations_.clear();
 }