Tell IDB frontend about data loss

content/browser/indexed_db/indexed_db_database.cc

Index: content/browser/indexed_db/indexed_db_database.cc
diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc
index 5d0b26534003eb93a48652d936da9a507b66bae5..b4a14fd37f34d5386a62704004aa49b9d3621c60 100644
--- a/content/browser/indexed_db/indexed_db_database.cc
+++ b/content/browser/indexed_db/indexed_db_database.cc
@@ -63,12 +63,14 @@ class IndexedDBDatabase::VersionChangeOperation
       int64 transaction_id,
       int64 version,
       scoped_refptr<IndexedDBCallbacksWrapper> callbacks,
-      scoped_refptr<IndexedDBDatabaseCallbacksWrapper> database_callbacks)
+      scoped_refptr<IndexedDBDatabaseCallbacksWrapper> database_callbacks,
+      bool data_loss)

[jsbell, 2013/06/14 20:42:58]

Pluming this through VersionChangeOperation seems odd at first glance, although
it makes sense if you follow the flow of the flag through the system.

Some alternatives that come to mind.
* Have it be a state on database, i.e. "when opened, there was some data loss".
I guess it would need to be reset after being plucked out so only the first
connection would see it.
* Have it be a state on the backing store itself, i.e. "when opened, there was
some data loss". This would let us signal each subsequent database open that
there was data loss, but then every database would get it until all handles were
closed, unless we added even more state tracking. :(

I don't think either of those are better than what's here, though.

[dgrogan, 2013/06/14 22:06:43]

I considered keeping some state like you describe but also didn't like any of
the options. Shrug.

       : database_(database),
         transaction_id_(transaction_id),
         version_(version),
         callbacks_(callbacks),
-        database_callbacks_(database_callbacks) {}
+        database_callbacks_(database_callbacks),
+        data_loss_(data_loss) {}
   virtual void Perform(IndexedDBTransaction* transaction) OVERRIDE;
 
  private:
@@ -77,6 +79,7 @@ class IndexedDBDatabase::VersionChangeOperation
   int64 version_;
   scoped_refptr<IndexedDBCallbacksWrapper> callbacks_;
   scoped_refptr<IndexedDBDatabaseCallbacksWrapper> database_callbacks_;
+  bool data_loss_;
 };
 
 class CreateObjectStoreAbortOperation : public IndexedDBTransaction::Operation {
@@ -1394,7 +1397,8 @@ void IndexedDBDatabase::VersionChangeOperation::Perform(
   DCHECK(!database_->pending_second_half_open_);
   database_->pending_second_half_open_.reset(new PendingOpenCall(
       callbacks_, database_callbacks_, transaction_id_, version_));
-  callbacks_->OnUpgradeNeeded(old_version, database_, database_->metadata());
+  callbacks_->OnUpgradeNeeded(
+      old_version, database_, database_->metadata(), data_loss_);
 }
 
 void IndexedDBDatabase::TransactionStarted(IndexedDBTransaction* transaction) {
@@ -1467,7 +1471,8 @@ void IndexedDBDatabase::ProcessPendingCalls() {
     RunVersionChangeTransactionFinal(pending_call->Callbacks(),
                                      pending_call->DatabaseCallbacks(),
                                      pending_call->TransactionId(),
-                                     pending_call->Version());
+                                     pending_call->Version(),
+                                     false);

[jsbell, 2013/06/14 20:42:58]

Can you give the argument a default value instead of passing explicit false in
these cases?

[dgrogan, 2013/06/14 22:06:43]

Done.

     DCHECK_EQ(static_cast<size_t>(1), ConnectionCount());
     // Fall through would be a no-op, since transaction must complete
     // asynchronously.
@@ -1501,7 +1506,8 @@ void IndexedDBDatabase::ProcessPendingCalls() {
       OpenConnection(pending_open_call->Callbacks(),
                      pending_open_call->DatabaseCallbacks(),
                      pending_open_call->TransactionId(),
-                     pending_open_call->Version());
+                     pending_open_call->Version(),
+                     false);

[jsbell, 2013/06/14 20:42:58]

Ditto.

[dgrogan, 2013/06/14 22:06:43]

Done.

     }
   }
 }
@@ -1535,12 +1541,17 @@ void IndexedDBDatabase::OpenConnection(
     scoped_refptr<IndexedDBCallbacksWrapper> callbacks,
     scoped_refptr<IndexedDBDatabaseCallbacksWrapper> database_callbacks,
     int64 transaction_id,
-    int64 version) {
+    int64 version,
+    bool data_loss) {
   DCHECK(backing_store_.get());
 
   // TODO(jsbell): Should have a priority queue so that higher version
   // requests are processed first. http://crbug.com/225850
   if (IsOpenConnectionBlocked()) {
+    // 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(!data_loss);
     pending_open_calls_.push_back(new PendingOpenCall(
         callbacks, database_callbacks, transaction_id, version));
     return;
@@ -1597,7 +1608,7 @@ void IndexedDBDatabase::OpenConnection(
   if (version > metadata_.int_version) {
     database_callbacks_set_.insert(database_callbacks);
     RunVersionChangeTransaction(
-        callbacks, database_callbacks, transaction_id, version);
+        callbacks, database_callbacks, transaction_id, version, data_loss);
     return;
   }
   if (version < metadata_.int_version) {
@@ -1617,11 +1628,13 @@ void IndexedDBDatabase::RunVersionChangeTransaction(
     scoped_refptr<IndexedDBCallbacksWrapper> callbacks,
     scoped_refptr<IndexedDBDatabaseCallbacksWrapper> database_callbacks,
     int64 transaction_id,
-    int64 requested_version) {
+    int64 requested_version,
+    bool data_loss) {
 
   DCHECK(callbacks.get());
   DCHECK(database_callbacks_set_.has(database_callbacks));
   if (ConnectionCount() > 1) {
+    DCHECK(!data_loss);
     // Front end ensures the event is not fired at connections that have
     // close_pending set.
     for (DatabaseCallbacksSet::const_iterator it =
@@ -1641,15 +1654,19 @@ void IndexedDBDatabase::RunVersionChangeTransaction(
         callbacks, database_callbacks, transaction_id, requested_version));
     return;
   }
-  RunVersionChangeTransactionFinal(
-      callbacks, database_callbacks, transaction_id, requested_version);
+  RunVersionChangeTransactionFinal(callbacks,
+                                   database_callbacks,
+                                   transaction_id,
+                                   requested_version,
+                                   data_loss);
 }
 
 void IndexedDBDatabase::RunVersionChangeTransactionFinal(
     scoped_refptr<IndexedDBCallbacksWrapper> callbacks,
     scoped_refptr<IndexedDBDatabaseCallbacksWrapper> database_callbacks,
     int64 transaction_id,
-    int64 requested_version) {
+    int64 requested_version,
+    bool data_loss) {
 
   std::vector<int64> object_store_ids;
   CreateTransaction(transaction_id,
@@ -1664,7 +1681,8 @@ void IndexedDBDatabase::RunVersionChangeTransactionFinal(
                                  transaction_id,
                                  requested_version,
                                  callbacks,
-                                 database_callbacks),
+                                 database_callbacks,
+                                 data_loss),
       new VersionChangeAbortOperation(
           this, metadata_.version, metadata_.int_version));