Handling LevelDB errors encountered after open.

content/browser/indexed_db/indexed_db_browsertest.cc

Index: content/browser/indexed_db/indexed_db_browsertest.cc
diff --git a/content/browser/indexed_db/indexed_db_browsertest.cc b/content/browser/indexed_db/indexed_db_browsertest.cc
index 5a2389333516c899ec373d118fcb7d21d26a0aa6..ae601298a82760264d5e1e57e7af66c80600672b 100644
--- a/content/browser/indexed_db/indexed_db_browsertest.cc
+++ b/content/browser/indexed_db/indexed_db_browsertest.cc
@@ -5,6 +5,7 @@
 #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"
@@ -24,6 +25,10 @@
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test.h"
 #include "content/test/content_browser_test_utils.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 +361,132 @@ IN_PROC_BROWSER_TEST_F(IndexedDBBrowserTest, CanDeleteWhenOverQuotaTest) {
   SimpleTest(GetTestUrl("indexeddb", "delete_over_quota.html"));
 }
 
+namespace {
+
+static void FlushIndexedDBDatabase(scoped_refptr<IndexedDBContextImpl> context,

[jsbell, 2014/03/26 18:16:58]

'Database' or 'BackingStore' ?

[cmumford, 2014/03/26 21:40:36]

Renamed to 'BackingStore'

+                                   const GURL& origin_url) {
+  IndexedDBFactory* factory = context->GetIDBFactory();
+
+  std::pair<IndexedDBFactory::OriginDBMapIterator,
+            IndexedDBFactory::OriginDBMapIterator> range =
+      factory->GetOpenDatabasesForOrigin(origin_url);
+
+  while (range.first != range.second) {
+    IndexedDBDatabase* db = range.first->second;
+    IndexedDBBackingStore* backing_store = db->backing_store();
+    backing_store->Flush();

[jsbell, 2014/03/26 18:16:58]

All databases in an origin share a backing store, so this is redundant.
Intentional? Paranoid? (add comment to explain?)

[cmumford, 2014/03/26 21:40:36]

Done.

+    ++range.first;
+  }
+}
+
+static void CorruptIndexedDBDatabase(

[jsbell, 2014/03/26 18:16:58]

Add comment about why it's async?

[cmumford, 2014/03/26 21:40:36]

Actually it's effectively sync, but I didn't see a better alternative. Is there
one?

[jsbell, 2014/03/26 22:49:00]

Oops, yeah, I shouldn't have said "async". It wasn't obvious (until I dug
further) that it was intended to be run "sync" but on another thread. On more
reflection... yeah, no need for a comment.

+    IndexedDBContextImpl* context,
+    const GURL& origin_url,
+    base::WaitableEvent* signal_when_finished) {
+
+  FlushIndexedDBDatabase(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);

[jsbell, 2014/03/26 18:16:58]

size_t? 

[cmumford, 2014/03/26 21:40:36]

Not according to base/file_util.h

[jsbell, 2014/03/26 22:49:00]

Sorry. I swear I saw it as size_t. :(

+    GetFileSize(idb_file, &size);
+
+    if (idb_file.Extension() == ".ldb") {
+      numFiles++;
+      FILE* f = fopen(idb_file.value().c_str(), "w");

[jsbell, 2014/03/26 18:16:58]

Why FILE/fopen/fwrite instead of base::PlatformFile abstractions?

[cmumford, 2014/03/26 21:40:36]

I think I was unaware way back when I wrote this. Fixed.

+      if (f) {
+        char zero(0);
+        if (size != (int64)fwrite(&zero, sizeof(zero), size, f))
+          numErrors++;
+        fclose(f);
+      } 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(

[jsbell, 2014/03/26 18:16:58]

Nit: Capitalize DB

[cmumford, 2014/03/26 21:40:36]

Done.

+    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.

[jsbell, 2014/03/26 18:16:58]

Can you use a GURL to make this parsing easier?

[cmumford, 2014/03/26 21:40:36]

No, this is only the resource/query part of the URL - already processed.

+  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 currupt IndexedDB: " << request_query;

[jsbell, 2014/03/26 18:16:58]

Typo: corrupt

[cmumford, 2014/03/26 21:40:36]

Done.

+    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>();
+  } else {

[jsbell, 2014/03/26 18:16:58]

No need for else block since if() early-exits.

[cmumford, 2014/03/26 21:40:36]

Done.

+    // A request for a test resource

[jsbell, 2014/03/26 18:16:58]

Do we need to handle this? Can't the registered handler's prefix be something
only the test's XHR hits?

[cmumford, 2014/03/26 21:40:36]

From what I can tell this BasicHttpResponse object is used both to serve up the
tests themselves (i.e. corrupted_open_db_detection.html) and handle the XHR
call. I bet there is a cleaner more general way to do this, but the solution
isn't obvious.

I'd hoped to clean this up a bit, but was pressed for time. My goal is to
improve this a bit and then to refactor our other PRE_, PRE_PRE_ tests to use
XHR's.

[jsbell, 2014/03/26 22:49:00]

Again, sorry - I thought the RegisterRequestHandler call itself took a prefix
path and was just registering to handle on a subset of requests.

+    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 +580,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());
 }