[IndexedDB] Abort transaction on unknown blob

content/browser/indexed_db/database_impl.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/indexed_db/database_impl.h"
 
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/indexed_db/indexed_db_connection.h"
 #include "content/browser/indexed_db/indexed_db_context_impl.h"
 #include "content/browser/indexed_db/indexed_db_dispatcher_host.h"
 #include "content/browser/indexed_db/indexed_db_transaction.h"
 #include "content/browser/indexed_db/indexed_db_value.h"
 #include "storage/browser/blob/blob_storage_context.h"
 #include "storage/browser/quota/quota_manager_proxy.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBDatabaseException.h"
 
 using std::swap;
 
 namespace content {
+class IndexedDBDatabaseError;
 
 namespace {
-const char kInvalidBlobUuid[] = "Blob UUID is invalid";
+const char kInvalidBlobUuid[] = "Blob does not exist";
 const char kInvalidBlobFilePath[] = "Blob file path is invalid";
 }  // namespace
 
 class DatabaseImpl::IDBThreadHelper {
  public:
   IDBThreadHelper(std::unique_ptr<IndexedDBConnection> connection,
                   const url::Origin& origin,
                   scoped_refptr<IndexedDBDispatcherHost> dispatcher_host);
   ~IDBThreadHelper();
 
@@ skipping 75 matching lines @@
                    bool unique,
                    bool multi_entry);
   void DeleteIndex(int64_t transaction_id,
                    int64_t object_store_id,
                    int64_t index_id);
   void RenameIndex(int64_t transaction_id,
                    int64_t object_store_id,
                    int64_t index_id,
                    const base::string16& new_name);
   void Abort(int64_t transaction_id);
+  void AbortWithError(int64_t transaction_id,
+                      const IndexedDBDatabaseError& error);
   void Commit(int64_t transaction_id);
   void OnGotUsageAndQuotaForCommit(int64_t transaction_id,
                                    storage::QuotaStatusCode status,
                                    int64_t usage,
                                    int64_t quota);
   void AckReceivedBlobs(const std::vector<std::string>& uuids);
 
  private:
   scoped_refptr<IndexedDBDispatcherHost> dispatcher_host_;
   std::unique_ptr<IndexedDBConnection> connection_;
@@ skipping 120 matching lines @@
     int64_t transaction_id,
     int64_t object_store_id,
     ::indexed_db::mojom::ValuePtr value,
     const IndexedDBKey& key,
     blink::WebIDBPutMode mode,
     const std::vector<IndexedDBIndexKeys>& index_keys,
     ::indexed_db::mojom::CallbacksAssociatedPtrInfo callbacks_info) {
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
+  scoped_refptr<IndexedDBCallbacks> callbacks(new IndexedDBCallbacks(
+      dispatcher_host_.get(), origin_, std::move(callbacks_info)));
+
   std::vector<std::unique_ptr<storage::BlobDataHandle>> handles(
       value->blob_or_file_info.size());
   std::vector<IndexedDBBlobInfo> blob_info(value->blob_or_file_info.size());
   for (size_t i = 0; i < value->blob_or_file_info.size(); ++i) {
     ::indexed_db::mojom::BlobInfoPtr& info = value->blob_or_file_info[i];
 
     std::unique_ptr<storage::BlobDataHandle> handle =
         dispatcher_host_->blob_storage_context()->GetBlobDataFromUUID(
             info->uuid);
+
+    // Due to known issue crbug.com/351753, blobs can die while being passed to
+    // a different process. So this case must be handled gracefully.
+    UMA_HISTOGRAM_BOOLEAN("Storage.IndexedDB.PutValidBlob", !!handle);

[Mark P, 2017/03/28 18:17:07]

nit: !!handle looks ugly to me.  Can you rewrite it in something more easily
interpretable?

[dmurph, 2017/03/29 18:21:34]

Done.

     if (!handle) {
-      mojo::ReportBadMessage(kInvalidBlobUuid);
+      IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,

[cmumford, 2017/03/28 18:49:31]

We do want to go back to using mojo::ReportBadMessage once crbug.com/351753 is
fixed right? If so can you add a TODO above, or a comment to that bug so that we
don't forget?

[dmurph, 2017/03/29 18:21:34]

Done.

+                                   kInvalidBlobUuid);
+      idb_runner_->PostTask(FROM_HERE, base::Bind(&IndexedDBCallbacks::OnError,
+                                                  callbacks, error));
+      idb_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&IDBThreadHelper::AbortWithError,
+                     base::Unretained(helper_), transaction_id, error));
       return;
     }
+    UMA_HISTOGRAM_COUNTS("Storage.IndexedDB.PutBlobSizeKB",

[Mark P, 2017/03/28 18:17:07]

Why not UMA_HISTOGRAM_MEMORY_KB?

[dmurph, 2017/03/29 18:21:34]

sgtm

+                         handle->size() / 1024ull);
+
     handles[i] = std::move(handle);
 
     if (info->file) {
       if (!info->file->path.empty() &&
           !policy->CanReadFile(dispatcher_host_->ipc_process_id(),
                                info->file->path)) {
         mojo::ReportBadMessage(kInvalidBlobFilePath);
         return;
       }
       blob_info[i] = IndexedDBBlobInfo(info->uuid, info->file->path,
                                        info->file->name, info->mime_type);
       if (info->size != -1) {
         blob_info[i].set_last_modified(info->file->last_modified);
         blob_info[i].set_size(info->size);
       }
     } else {
       blob_info[i] = IndexedDBBlobInfo(info->uuid, info->mime_type, info->size);
     }
   }
 
-  scoped_refptr<IndexedDBCallbacks> callbacks(new IndexedDBCallbacks(
-      dispatcher_host_.get(), origin_, std::move(callbacks_info)));
-
   idb_runner_->PostTask(
       FROM_HERE,
       base::Bind(&IDBThreadHelper::Put, base::Unretained(helper_),
                  transaction_id, object_store_id, base::Passed(&value),
                  base::Passed(&handles), base::Passed(&blob_info), key, mode,
                  index_keys, base::Passed(&callbacks)));
 }
 
 void DatabaseImpl::SetIndexKeys(
     int64_t transaction_id,
@@ skipping 478 matching lines @@
     return;
 
   IndexedDBTransaction* transaction =
       connection_->GetTransaction(transaction_id);
   if (!transaction)
     return;
 
   connection_->AbortTransaction(transaction);
 }
 
+void DatabaseImpl::IDBThreadHelper::AbortWithError(
+    int64_t transaction_id,
+    const IndexedDBDatabaseError& error) {
+  if (!connection_->IsConnected())
+    return;
+
+  IndexedDBTransaction* transaction =
+      connection_->GetTransaction(transaction_id);
+  if (!transaction)
+    return;
+
+  connection_->AbortTransaction(transaction, error);
+}
+
 void DatabaseImpl::IDBThreadHelper::Commit(int64_t transaction_id) {
   if (!connection_->IsConnected())
     return;
 
   IndexedDBTransaction* transaction =
       connection_->GetTransaction(transaction_id);
   if (!transaction)
     return;
 
   // Always allow empty or delete-only transactions.
@@ skipping 27 matching lines @@
       usage + transaction->size() <= quota) {
     connection_->database()->Commit(transaction);
   } else {
     connection_->AbortTransaction(
         transaction,
         IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionQuotaError));
   }
 }
 
 }  // namespace content