IndexedDB: Simplify data loss code/message routing

Rather than passing a code/message pair through the entire open call
sequence, let the data loss status be assigned to the callbacks object
which is already passed along.

R=dgrogan
BUG=329931

Review URL: https://codereview.chromium.org/119343004

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

content/browser/indexed_db/indexed_db_callbacks.cc

@@ -32,7 +32,8 @@ IndexedDBCallbacks::IndexedDBCallbacks(IndexedDBDispatcherHost* dispatcher_host,
       ipc_cursor_id_(kNoCursor),
       host_transaction_id_(kNoTransaction),
       ipc_database_id_(kNoDatabase),
-      ipc_database_callbacks_id_(kNoDatabaseCallbacks) {}
+      ipc_database_callbacks_id_(kNoDatabaseCallbacks),
+      data_loss_(blink::WebIDBDataLossNone) {}
 
 IndexedDBCallbacks::IndexedDBCallbacks(IndexedDBDispatcherHost* dispatcher_host,
                                        int32 ipc_thread_id,
@@ -44,7 +45,8 @@ IndexedDBCallbacks::IndexedDBCallbacks(IndexedDBDispatcherHost* dispatcher_host,
       ipc_cursor_id_(ipc_cursor_id),
       host_transaction_id_(kNoTransaction),
       ipc_database_id_(kNoDatabase),
-      ipc_database_callbacks_id_(kNoDatabaseCallbacks) {}
+      ipc_database_callbacks_id_(kNoDatabaseCallbacks),
+      data_loss_(blink::WebIDBDataLossNone) {}
 
 IndexedDBCallbacks::IndexedDBCallbacks(IndexedDBDispatcherHost* dispatcher_host,
                                        int32 ipc_thread_id,
@@ -59,7 +61,8 @@ IndexedDBCallbacks::IndexedDBCallbacks(IndexedDBDispatcherHost* dispatcher_host,
       host_transaction_id_(host_transaction_id),
       origin_url_(origin_url),
       ipc_database_id_(kNoDatabase),
-      ipc_database_callbacks_id_(ipc_database_callbacks_id) {}
+      ipc_database_callbacks_id_(ipc_database_callbacks_id),
+      data_loss_(blink::WebIDBDataLossNone) {}
 
 IndexedDBCallbacks::~IndexedDBCallbacks() {}
 
@@ -78,6 +81,7 @@ void IndexedDBCallbacks::OnSuccess(const std::vector<base::string16>& value) {
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   std::vector<base::string16> list;
   for (unsigned i = 0; i < value.size(); ++i)
@@ -101,12 +105,17 @@ void IndexedDBCallbacks::OnBlocked(int64 existing_version) {
       ipc_thread_id_, ipc_callbacks_id_, existing_version));
 }
 
+void IndexedDBCallbacks::OnDataLoss(blink::WebIDBDataLoss data_loss,
+                                    std::string data_loss_message) {
+  DCHECK_NE(blink::WebIDBDataLossNone, data_loss);
+  data_loss_ = data_loss;
+  data_loss_message_ = data_loss_message;
+}
+
 void IndexedDBCallbacks::OnUpgradeNeeded(
     int64 old_version,
     scoped_ptr<IndexedDBConnection> connection,
-    const IndexedDBDatabaseMetadata& metadata,
-    blink::WebIDBDataLoss data_loss,
-    std::string data_loss_message) {
+    const IndexedDBDatabaseMetadata& metadata) {
   DCHECK(dispatcher_host_.get());
 
   DCHECK_EQ(kNoCursor, ipc_cursor_id_);
@@ -127,8 +136,8 @@ void IndexedDBCallbacks::OnUpgradeNeeded(
   params.ipc_database_callbacks_id = ipc_database_callbacks_id_;
   params.old_version = old_version;
   params.idb_metadata = IndexedDBDispatcherHost::ConvertMetadata(metadata);
-  params.data_loss = data_loss;
-  params.data_loss_message = data_loss_message;
+  params.data_loss = data_loss_;
+  params.data_loss_message = data_loss_message_;
   dispatcher_host_->Send(new IndexedDBMsg_CallbacksUpgradeNeeded(params));
 }
 
@@ -169,6 +178,7 @@ void IndexedDBCallbacks::OnSuccess(scoped_refptr<IndexedDBCursor> cursor,
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   int32 ipc_object_id = dispatcher_host_->Add(cursor.get());
   IndexedDBMsg_CallbacksSuccessIDBCursor_Params params;
@@ -195,6 +205,7 @@ void IndexedDBCallbacks::OnSuccess(const IndexedDBKey& key,
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   IndexedDBCursor* idb_cursor =
       dispatcher_host_->GetCursorFromId(ipc_cursor_id_);
@@ -230,6 +241,7 @@ void IndexedDBCallbacks::OnSuccessWithPrefetch(
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   std::vector<IndexedDBKey> msgKeys;
   std::vector<IndexedDBKey> msgPrimaryKeys;
@@ -260,6 +272,7 @@ void IndexedDBCallbacks::OnSuccess(std::string* value,
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   std::string value_copy;
   if (value && !value->empty())
@@ -282,6 +295,7 @@ void IndexedDBCallbacks::OnSuccess(std::string* value) {
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   std::string value_copy;
   if (value && !value->empty())
@@ -302,6 +316,7 @@ void IndexedDBCallbacks::OnSuccess(const IndexedDBKey& value) {
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   dispatcher_host_->Send(new IndexedDBMsg_CallbacksSuccessIndexedDBKey(
       ipc_thread_id_, ipc_callbacks_id_, value));
@@ -315,6 +330,7 @@ void IndexedDBCallbacks::OnSuccess(int64 value) {
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   dispatcher_host_->Send(new IndexedDBMsg_CallbacksSuccessInteger(
       ipc_thread_id_, ipc_callbacks_id_, value));
@@ -328,6 +344,7 @@ void IndexedDBCallbacks::OnSuccess() {
   DCHECK_EQ(kNoTransaction, host_transaction_id_);
   DCHECK_EQ(kNoDatabase, ipc_database_id_);
   DCHECK_EQ(kNoDatabaseCallbacks, ipc_database_callbacks_id_);
+  DCHECK_EQ(blink::WebIDBDataLossNone, data_loss_);
 
   dispatcher_host_->Send(new IndexedDBMsg_CallbacksSuccessUndefined(
       ipc_thread_id_, ipc_callbacks_id_));

content/browser/indexed_db/indexed_db_callbacks.h

@@ -57,12 +57,12 @@ class CONTENT_EXPORT IndexedDBCallbacks
   virtual void OnBlocked(int64 existing_version);
 
   // IndexedDBFactory::Open
+  virtual void OnDataLoss(blink::WebIDBDataLoss data_loss,
+                          std::string data_loss_message);
   virtual void OnUpgradeNeeded(
       int64 old_version,
       scoped_ptr<IndexedDBConnection> connection,
-      const content::IndexedDBDatabaseMetadata& metadata,
-      blink::WebIDBDataLoss data_loss,
-      std::string data_loss_message);
+      const content::IndexedDBDatabaseMetadata& metadata);
   virtual void OnSuccess(scoped_ptr<IndexedDBConnection> connection,
                          const content::IndexedDBDatabaseMetadata& metadata);
 
@@ -101,6 +101,8 @@ class CONTENT_EXPORT IndexedDBCallbacks
   // IndexedDBCursor::Continue / Advance (when complete)
   virtual void OnSuccess();
 
+  blink::WebIDBDataLoss data_loss() const { return data_loss_; }
+
  protected:
   virtual ~IndexedDBCallbacks();
 
@@ -120,6 +122,10 @@ class CONTENT_EXPORT IndexedDBCallbacks
   GURL origin_url_;
   int32 ipc_database_id_;
   int32 ipc_database_callbacks_id_;
+
+  // Stored in OnDataLoss, merged with OnUpgradeNeeded response.
+  blink::WebIDBDataLoss data_loss_;
+  std::string data_loss_message_;
 };
 
 }  // namespace content

content/browser/indexed_db/indexed_db_database.cc

@@ -1230,8 +1230,6 @@ void IndexedDBDatabase::VersionChangeOperation(
     int64 version,
     scoped_refptr<IndexedDBCallbacks> callbacks,
     scoped_ptr<IndexedDBConnection> connection,
-    blink::WebIDBDataLoss data_loss,
-    std::string data_loss_message,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::VersionChangeOperation");
   int64 old_version = metadata_.int_version;
@@ -1253,8 +1251,7 @@ void IndexedDBDatabase::VersionChangeOperation(
   DCHECK(!pending_second_half_open_);
   pending_second_half_open_.reset(
       new PendingSuccessCall(callbacks, connection.get(), version));
-  callbacks->OnUpgradeNeeded(
-      old_version, connection.Pass(), metadata(), data_loss, data_loss_message);
+  callbacks->OnUpgradeNeeded(old_version, connection.Pass(), metadata());
 }
 
 void IndexedDBDatabase::TransactionFinished(IndexedDBTransaction* transaction,
@@ -1401,19 +1398,6 @@ void IndexedDBDatabase::OpenConnection(
     scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
     int64 transaction_id,
     int64 version) {
-  const blink::WebIDBDataLoss kDataLoss =
-      blink::WebIDBDataLossNone;
-  OpenConnection(
-      callbacks, database_callbacks, transaction_id, version, kDataLoss, "");
-}
-
-void IndexedDBDatabase::OpenConnection(
-    scoped_refptr<IndexedDBCallbacks> callbacks,
-    scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
-    int64 transaction_id,
-    int64 version,
-    blink::WebIDBDataLoss data_loss,
-    std::string data_loss_message) {
   DCHECK(backing_store_);
 
   // TODO(jsbell): Should have a priority queue so that higher version
@@ -1422,7 +1406,7 @@ void IndexedDBDatabase::OpenConnection(
     // The backing store only detects data loss when it is first opened. The
     // presence of existing connections means we didn't even check for data loss
     // so there'd better not be any.
-    DCHECK_NE(blink::WebIDBDataLossTotal, data_loss);
+    DCHECK_NE(blink::WebIDBDataLossTotal, callbacks->data_loss());
     pending_open_calls_.push_back(new PendingOpenCall(
         callbacks, database_callbacks, transaction_id, version));
     return;
@@ -1482,12 +1466,8 @@ void IndexedDBDatabase::OpenConnection(
 
   if (version > metadata_.int_version) {
     connections_.insert(connection.get());
-    RunVersionChangeTransaction(callbacks,
-                                connection.Pass(),
-                                transaction_id,
-                                version,
-                                data_loss,
-                                data_loss_message);
+    RunVersionChangeTransaction(
+        callbacks, connection.Pass(), transaction_id, version);
     return;
   }
   if (version < metadata_.int_version) {
@@ -1507,14 +1487,12 @@ void IndexedDBDatabase::RunVersionChangeTransaction(
     scoped_refptr<IndexedDBCallbacks> callbacks,
     scoped_ptr<IndexedDBConnection> connection,
     int64 transaction_id,
-    int64 requested_version,
-    blink::WebIDBDataLoss data_loss,
-    std::string data_loss_message) {
+    int64 requested_version) {
 
   DCHECK(callbacks);
   DCHECK(connections_.count(connection.get()));
   if (ConnectionCount() > 1) {
-    DCHECK_NE(blink::WebIDBDataLossTotal, data_loss);
+    DCHECK_NE(blink::WebIDBDataLossTotal, callbacks->data_loss());
     // Front end ensures the event is not fired at connections that have
     // close_pending set.
     for (ConnectionSet::const_iterator it = connections_.begin();
@@ -1535,12 +1513,8 @@ void IndexedDBDatabase::RunVersionChangeTransaction(
         callbacks, connection.Pass(), transaction_id, requested_version));
     return;
   }
-  RunVersionChangeTransactionFinal(callbacks,
-                                   connection.Pass(),
-                                   transaction_id,
-                                   requested_version,
-                                   data_loss,
-                                   data_loss_message);
+  RunVersionChangeTransactionFinal(
+      callbacks, connection.Pass(), transaction_id, requested_version);
 }
 
 void IndexedDBDatabase::RunVersionChangeTransactionFinal(
@@ -1548,23 +1522,6 @@ void IndexedDBDatabase::RunVersionChangeTransactionFinal(
     scoped_ptr<IndexedDBConnection> connection,
     int64 transaction_id,
     int64 requested_version) {
-  const blink::WebIDBDataLoss kDataLoss =
-      blink::WebIDBDataLossNone;
-  RunVersionChangeTransactionFinal(callbacks,
-                                   connection.Pass(),
-                                   transaction_id,
-                                   requested_version,
-                                   kDataLoss,
-                                   "");
-}
-
-void IndexedDBDatabase::RunVersionChangeTransactionFinal(
-    scoped_refptr<IndexedDBCallbacks> callbacks,
-    scoped_ptr<IndexedDBConnection> connection,
-    int64 transaction_id,
-    int64 requested_version,
-    blink::WebIDBDataLoss data_loss,
-    std::string data_loss_message) {
 
   std::vector<int64> object_store_ids;
   CreateTransaction(transaction_id,
@@ -1577,9 +1534,7 @@ void IndexedDBDatabase::RunVersionChangeTransactionFinal(
                                 this,
                                 requested_version,
                                 callbacks,
-                                base::Passed(&connection),
-                                data_loss,
-                                data_loss_message),
+                                base::Passed(&connection)),
                      base::Bind(&IndexedDBDatabase::VersionChangeAbortOperation,
                                 this,
                                 metadata_.version,

content/browser/indexed_db/indexed_db_database.h

@@ -76,13 +76,6 @@ class CONTENT_EXPORT IndexedDBDatabase
       scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
       int64 transaction_id,
       int64 version);
-  void OpenConnection(
-      scoped_refptr<IndexedDBCallbacks> callbacks,
-      scoped_refptr<IndexedDBDatabaseCallbacks> database_callbacks,
-      int64 transaction_id,
-      int64 version,
-      blink::WebIDBDataLoss data_loss,
-      std::string data_loss_message);
   void DeleteDatabase(scoped_refptr<IndexedDBCallbacks> callbacks);
   const IndexedDBDatabaseMetadata& metadata() const { return metadata_; }
 
@@ -193,8 +186,6 @@ class CONTENT_EXPORT IndexedDBDatabase
   void VersionChangeOperation(int64 version,
                               scoped_refptr<IndexedDBCallbacks> callbacks,
                               scoped_ptr<IndexedDBConnection> connection,
-                              blink::WebIDBDataLoss data_loss,
-                              std::string data_loss_message,
                               IndexedDBTransaction* transaction);
   void VersionChangeAbortOperation(const base::string16& previous_version,
                                    int64 previous_int_version,
@@ -252,21 +243,12 @@ class CONTENT_EXPORT IndexedDBDatabase
   void RunVersionChangeTransaction(scoped_refptr<IndexedDBCallbacks> callbacks,
                                    scoped_ptr<IndexedDBConnection> connection,
                                    int64 transaction_id,
-                                   int64 requested_version,
-                                   blink::WebIDBDataLoss data_loss,
-                                   std::string data_loss_message);
+                                   int64 requested_version);
   void RunVersionChangeTransactionFinal(
       scoped_refptr<IndexedDBCallbacks> callbacks,
       scoped_ptr<IndexedDBConnection> connection,
       int64 transaction_id,
       int64 requested_version);
-  void RunVersionChangeTransactionFinal(
-      scoped_refptr<IndexedDBCallbacks> callbacks,
-      scoped_ptr<IndexedDBConnection> connection,
-      int64 transaction_id,
-      int64 requested_version,
-      blink::WebIDBDataLoss data_loss,
-      std::string data_loss_message);
   void ProcessPendingCalls();
 
   bool IsDeleteDatabaseBlocked() const;

content/browser/indexed_db/indexed_db_factory.cc

@@ -316,12 +316,11 @@ void IndexedDBFactory::Open(
     database = it->second;
   }
 
-  database->OpenConnection(callbacks,
-                           database_callbacks,
-                           transaction_id,
-                           version,
-                           data_loss,
-                           data_loss_message);
+  if (data_loss != blink::WebIDBDataLossNone)
+    callbacks->OnDataLoss(data_loss, data_loss_message);
+
+  database->OpenConnection(
+      callbacks, database_callbacks, transaction_id, version);
 
   if (!was_open && database->ConnectionCount() > 0)
     database_map_[unique_identifier] = database;

content/browser/indexed_db/indexed_db_factory_unittest.cc

@@ -400,9 +400,7 @@ class UpgradeNeededCallbacks : public MockIndexedDBCallbacks {
   virtual void OnUpgradeNeeded(
       int64 old_version,
       scoped_ptr<IndexedDBConnection> connection,
-      const content::IndexedDBDatabaseMetadata& metadata,
-      blink::WebIDBDataLoss data_loss,
-      std::string data_loss_message) OVERRIDE {
+      const content::IndexedDBDatabaseMetadata& metadata) OVERRIDE {
     connection_ = connection.Pass();
   }