Un-revert Handling LevelDB errors encountered after open.

The previous IndexedDB implementation only checked for (and reported)
corrupted LevelDB's during open. If they are determined to be corrupted
during use then the database was simply closed. The page would be unaware of
this, and would likely reopen the database at a later time (which would work)
only to fail again during a read/write operation. There was no way for the
page to get out of that corrupted state - the user would have to delete all
origin data.

This change will record the fact that the backing store was corrupted, and
during open this would be read and, if present, result in the same dataLoss
state being reported to the page in the upgradeneeded event.

Also split IndexedDBBackingStore's REPORT_ERROR macro in two:
REPORT_ERROR_UNTESTED (which calls NOTREACHED) and REPORT_ERROR. REPORT_ERROR
is used for errors which are encountered during tests. This was done so that
runtime errors could still be caught during a debug build run.

The original change (r260147) had a buffer overrun in a unit test fixed
in this CL.

BUG=322707
R=isherman@chromium.org, jochen@chromium.org, jsbell@chromium.org

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

Patch from Christopher Mumford <cmumford@chromium.org>.

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

content/browser/indexed_db/indexed_db_backing_store.h

@@ -78,6 +78,8 @@ class CONTENT_EXPORT IndexedDBBackingStore
       const GURL& origin_url,
       LevelDBFactory* factory);
 
+  // Compact is public for testing.
+  virtual void Compact();
   virtual std::vector<base::string16> GetDatabaseNames();
   virtual leveldb::Status GetIDBDatabaseMetaData(
       const base::string16& name,
@@ -94,6 +96,12 @@ class CONTENT_EXPORT IndexedDBBackingStore
       int64 int_version);
   virtual leveldb::Status DeleteDatabase(const base::string16& name);
 
+  // Assumes caller has already closed the backing store.
+  static leveldb::Status DestroyBackingStore(const base::FilePath& path_base,
+                                             const GURL& origin_url);
+  static bool RecordCorruptionInfo(const base::FilePath& path_base,
+                                   const GURL& origin_url,
+                                   const std::string& message);
   leveldb::Status GetObjectStores(
       int64 database_id,
       IndexedDBDatabaseMetadata::ObjectStoreMap* map) WARN_UNUSED_RESULT;
@@ -325,6 +333,9 @@ class CONTENT_EXPORT IndexedDBBackingStore
       const GURL& origin_url,
       scoped_ptr<LevelDBDatabase> db,
       scoped_ptr<LevelDBComparator> comparator);
+  static bool ReadCorruptionInfo(const base::FilePath& path_base,
+                                 const GURL& origin_url,
+                                 std::string& message);
 
   leveldb::Status FindKeyInIndex(
       IndexedDBBackingStore::Transaction* transaction,

content/browser/indexed_db/indexed_db_browsertest.cc

@@ -5,9 +5,11 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
+#include "base/platform_file.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/thread_test_helper.h"
 #include "content/browser/browser_main_loop.h"
@@ -24,6 +26,10 @@
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/shell/browser/shell.h"
+#include "net/base/net_errors.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "webkit/browser/database/database_util.h"
 #include "webkit/browser/quota/quota_manager.h"
 
@@ -356,6 +362,140 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CanDeleteWhenOverQuotaTest) {
   SimpleTest(GetTestUrl("indexeddb", "delete_over_quota.html"));
 }
 
+namespace {
+
+static void CompactIndexedDBBackingStore(
+    scoped_refptr<IndexedDBContextImpl> context,
+    const GURL& origin_url) {
+  IndexedDBFactory* factory = context->GetIDBFactory();
+
+  std::pair<IndexedDBFactory::OriginDBMapIterator,
+            IndexedDBFactory::OriginDBMapIterator> range =
+      factory->GetOpenDatabasesForOrigin(origin_url);
+
+  if (range.first == range.second)  // If no open db's for this origin
+    return;
+
+  // Compact the first db's backing store since all the db's are in the same
+  // backing store.
+  IndexedDBDatabase* db = range.first->second;
+  IndexedDBBackingStore* backing_store = db->backing_store();
+  backing_store->Compact();
+}
+
+static void CorruptIndexedDBDatabase(
+    IndexedDBContextImpl* context,
+    const GURL& origin_url,
+    base::WaitableEvent* signal_when_finished) {
+
+  CompactIndexedDBBackingStore(context, origin_url);
+
+  int numFiles = 0;
+  int numErrors = 0;
+  base::FilePath idb_data_path = context->GetFilePath(origin_url);
+  const bool recursive = false;
+  base::FileEnumerator enumerator(
+      idb_data_path, recursive, base::FileEnumerator::FILES);
+  for (base::FilePath idb_file = enumerator.Next(); !idb_file.empty();
+       idb_file = enumerator.Next()) {
+    int64 size(0);
+    GetFileSize(idb_file, &size);
+
+    if (idb_file.Extension() == FILE_PATH_LITERAL(".ldb")) {
+      numFiles++;
+      base::PlatformFile f = base::CreatePlatformFile(
+          idb_file,
+          base::PLATFORM_FILE_WRITE | base::PLATFORM_FILE_OPEN_TRUNCATED,
+          NULL,
+          NULL);
+      if (f) {
+        // Was opened truncated, expand back to the original
+        // file size and fill with zeros (corrupting the file).
+        base::TruncatePlatformFile(f, size);
+        if (!base::ClosePlatformFile(f))
+          numErrors++;
+      } else {
+        numErrors++;
+      }
+    }
+  }
+
+  VLOG(0) << "There were " << numFiles << " in " << idb_data_path.value()
+          << " with " << numErrors << " errors";
+  signal_when_finished->Signal();
+}
+
+const std::string s_corrupt_db_test_prefix = "/corrupt/test/";
+
+static scoped_ptr<net::test_server::HttpResponse> CorruptDBRequestHandler(
+    IndexedDBContextImpl* context,
+    const GURL& origin_url,
+    const std::string& path,
+    const net::test_server::HttpRequest& request) {
+
+  std::string request_path;
+  if (path.find(s_corrupt_db_test_prefix) != std::string::npos)
+    request_path = request.relative_url.substr(s_corrupt_db_test_prefix.size());
+  else
+    return scoped_ptr<net::test_server::HttpResponse>();
+
+  // Remove the query string if present.
+  std::string request_query;
+  size_t query_pos = request_path.find('?');
+  if (query_pos != std::string::npos) {
+    request_query = request_path.substr(query_pos + 1);
+    request_path = request_path.substr(0, query_pos);
+  }
+
+  if (request_path == "corruptdb" && !request_query.empty()) {
+    VLOG(0) << "Requested to corrupt IndexedDB: " << request_query;
+    base::WaitableEvent signal_when_finished(false, false);
+    context->TaskRunner()->PostTask(FROM_HERE,
+                                    base::Bind(&CorruptIndexedDBDatabase,
+                                               base::ConstRef(context),
+                                               origin_url,
+                                               &signal_when_finished));
+    signal_when_finished.Wait();
+
+    scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+        new net::test_server::BasicHttpResponse);
+    http_response->set_code(net::HTTP_OK);
+    return http_response.PassAs<net::test_server::HttpResponse>();
+  }
+
+  // A request for a test resource
+  base::FilePath resourcePath =
+      content::GetTestFilePath("indexeddb", request_path.c_str());
+  scoped_ptr<net::test_server::BasicHttpResponse> http_response(
+      new net::test_server::BasicHttpResponse);
+  http_response->set_code(net::HTTP_OK);
+  std::string file_contents;
+  if (!base::ReadFileToString(resourcePath, &file_contents))
+    return scoped_ptr<net::test_server::HttpResponse>();
+  http_response->set_content(file_contents);
+  return http_response.PassAs<net::test_server::HttpResponse>();
+}
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CorruptedOpenDatabase) {
+  ASSERT_TRUE(embedded_test_server()->Started() ||
+              embedded_test_server()->InitializeAndWaitUntilReady());
+  const GURL& origin_url = embedded_test_server()->base_url();
+  embedded_test_server()->RegisterRequestHandler(
+      base::Bind(&CorruptDBRequestHandler,
+                 base::ConstRef(GetContext()),
+                 origin_url,
+                 s_corrupt_db_test_prefix));
+
+  std::string test_file =
+      s_corrupt_db_test_prefix + "corrupted_open_db_detection.html";
+  SimpleTest(embedded_test_server()->GetURL(test_file));
+
+  test_file = s_corrupt_db_test_prefix + "corrupted_open_db_recovery.html";
+  SimpleTest(embedded_test_server()->GetURL(test_file));
+}
+
 IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, DeleteCompactsBackingStore) {
   const GURL test_url = GetTestUrl("indexeddb", "delete_compact.html");
   SimpleTest(GURL(test_url.spec() + "#fill"));
@@ -449,6 +589,7 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, ForceCloseEventTest) {
 
   base::string16 expected_title16(ASCIIToUTF16("connection closed"));
   TitleWatcher title_watcher(shell()->web_contents(), expected_title16);
+  title_watcher.AlsoWaitForTitle(ASCIIToUTF16("connection closed with error"));
   EXPECT_EQ(expected_title16, title_watcher.WaitAndGetTitle());
 }
 

content/browser/indexed_db/indexed_db_database.cc

@@ -14,6 +14,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.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_cursor.h"
 #include "content/browser/indexed_db/indexed_db_factory.h"
 #include "content/browser/indexed_db/indexed_db_index_writer.h"
@@ -294,17 +295,22 @@ void IndexedDBDatabase::CreateObjectStoreOperation(
     const IndexedDBObjectStoreMetadata& object_store_metadata,
     IndexedDBTransaction* transaction) {
   IDB_TRACE("IndexedDBDatabase::CreateObjectStoreOperation");
-  if (!backing_store_->CreateObjectStore(
-          transaction->BackingStoreTransaction(),
-          transaction->database()->id(),
-          object_store_metadata.id,
-          object_store_metadata.name,
-          object_store_metadata.key_path,
-          object_store_metadata.auto_increment).ok()) {
-    transaction->Abort(IndexedDBDatabaseError(
+  leveldb::Status s =
+      backing_store_->CreateObjectStore(transaction->BackingStoreTransaction(),
+                                        transaction->database()->id(),
+                                        object_store_metadata.id,
+                                        object_store_metadata.name,
+                                        object_store_metadata.key_path,
+                                        object_store_metadata.auto_increment);
+  if (!s.ok()) {
+    IndexedDBDatabaseError error(
         blink::WebIDBDatabaseExceptionUnknownError,
         ASCIIToUTF16("Internal error creating object store '") +
-            object_store_metadata.name + ASCIIToUTF16("'.")));
+            object_store_metadata.name + ASCIIToUTF16("'."));
+    transaction->Abort(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
     return;
   }
 }
@@ -436,8 +442,12 @@ void IndexedDBDatabase::DeleteIndexOperation(
     base::string16 error_string =
         ASCIIToUTF16("Internal error deleting index '") +
         index_metadata.name + ASCIIToUTF16("'.");
-    transaction->Abort(IndexedDBDatabaseError(
-        blink::WebIDBDatabaseExceptionUnknownError, error_string));
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 error_string);
+    transaction->Abort(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
   }
 }
 
@@ -569,9 +579,13 @@ void IndexedDBDatabase::GetOperation(
                                   *key,
                                   &value);
     if (!s.ok()) {
-      callbacks->OnError(
-          IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                                 "Internal error in GetRecord."));
+      IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                   "Internal error in GetRecord.");
+      callbacks->OnError(error);
+
+      if (s.IsCorruption())
+        factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                               error);
       return;
     }
 
@@ -599,9 +613,12 @@ void IndexedDBDatabase::GetOperation(
       *key,
       &primary_key);
   if (!s.ok()) {
-    callbacks->OnError(
-        IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                               "Internal error in GetPrimaryKeyViaIndex."));
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 "Internal error in GetPrimaryKeyViaIndex.");
+    callbacks->OnError(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
     return;
   }
   if (!primary_key) {
@@ -622,9 +639,12 @@ void IndexedDBDatabase::GetOperation(
                                 *primary_key,
                                 &value);
   if (!s.ok()) {
-    callbacks->OnError(
-        IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                               "Internal error in GetRecord."));
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 "Internal error in GetRecord.");
+    callbacks->OnError(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
     return;
   }
 
@@ -761,9 +781,12 @@ void IndexedDBDatabase::PutOperation(scoped_ptr<PutOperationParams> params,
         &record_identifier,
         &found);
     if (!s.ok()) {
-      params->callbacks->OnError(
-          IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                                 "Internal error checking key existence."));
+      IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                   "Internal error checking key existence.");
+      params->callbacks->OnError(error);
+      if (s.IsCorruption())
+        factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                               error);
       return;
     }
     if (found) {
@@ -809,9 +832,13 @@ void IndexedDBDatabase::PutOperation(scoped_ptr<PutOperationParams> params,
                                 params->value,
                                 &record_identifier);
   if (!s.ok()) {
-    params->callbacks->OnError(IndexedDBDatabaseError(
+    IndexedDBDatabaseError error(
         blink::WebIDBDatabaseExceptionUnknownError,
-        "Internal error: backing store error performing put/add."));
+        "Internal error: backing store error performing put/add.");
+    params->callbacks->OnError(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
     return;
   }
 
@@ -834,9 +861,12 @@ void IndexedDBDatabase::PutOperation(scoped_ptr<PutOperationParams> params,
                                            *key,
                                            !key_was_generated);
     if (!s.ok()) {
-      params->callbacks->OnError(
-          IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                                 "Internal error updating key generator."));
+      IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                   "Internal error updating key generator.");
+      params->callbacks->OnError(error);
+      if (s.IsCorruption())
+        factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                               error);
       return;
     }
   }
@@ -866,9 +896,12 @@ void IndexedDBDatabase::SetIndexKeys(int64 transaction_id,
       &record_identifier,
       &found);
   if (!s.ok()) {
-    transaction->Abort(
-        IndexedDBDatabaseError(blink::WebIDBDatabaseExceptionUnknownError,
-                               "Internal error setting index keys."));
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 "Internal error setting index keys.");
+    transaction->Abort(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
     return;
   }
   if (!found) {
@@ -1206,8 +1239,12 @@ void IndexedDBDatabase::DeleteObjectStoreOperation(
     base::string16 error_string =
         ASCIIToUTF16("Internal error deleting object store '") +
         object_store_metadata.name + ASCIIToUTF16("'.");
-    transaction->Abort(IndexedDBDatabaseError(
-        blink::WebIDBDatabaseExceptionUnknownError, error_string));
+    IndexedDBDatabaseError error(blink::WebIDBDatabaseExceptionUnknownError,
+                                 error_string);
+    transaction->Abort(error);
+    if (s.IsCorruption())
+      factory_->HandleBackingStoreCorruption(backing_store_->origin_url(),
+                                             error);
   }
 }