IndexedDB: IO error when checking schema should destroy LevelDB directory
authordgrogan@chromium.org <dgrogan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Feb 2013 18:11:23 +0000 (18:11 +0000)
committerdgrogan@chromium.org <dgrogan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Feb 2013 18:11:23 +0000 (18:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110675

Reviewed by Adam Barth.

Source/WebCore:

Also some refactoring to remove IDBBackingStore's dependence on
static LevelDBDatabase methods. This facilitated the unit test.

New unit test - IDBIOErrorTest.CleanUpTest

* Modules/indexeddb/IDBBackingStore.cpp:
(DefaultLevelDBFactory):
Wraps the previous behavior.
(WebCore::IDBBackingStore::open):
The default parameter provides the previous behavior, which is what
non-tests want.
* Modules/indexeddb/IDBBackingStore.h:
* platform/leveldb/LevelDBDatabase.h:

Source/WebKit/chromium:

* WebKit.gyp:
This was cargo-culted. The component build wouldn't run otherwise.

* WebKit.gypi:
* tests/IDBCleanupOnIOErrorTest.cpp: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@144323 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/IDBBackingStore.h
Source/WebCore/platform/leveldb/LevelDBDatabase.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/WebKit.gyp
Source/WebKit/chromium/WebKit.gypi
Source/WebKit/chromium/tests/IDBCleanupOnIOErrorTest.cpp [new file with mode: 0644]

index 304dad0..1501a8e 100644 (file)
@@ -1,3 +1,24 @@
+2013-02-28  David Grogan  <dgrogan@chromium.org>
+
+        IndexedDB: IO error when checking schema should destroy LevelDB directory
+        https://bugs.webkit.org/show_bug.cgi?id=110675
+
+        Reviewed by Adam Barth.
+
+        Also some refactoring to remove IDBBackingStore's dependence on
+        static LevelDBDatabase methods. This facilitated the unit test.
+
+        New unit test - IDBIOErrorTest.CleanUpTest
+
+        * Modules/indexeddb/IDBBackingStore.cpp:
+        (DefaultLevelDBFactory):
+        Wraps the previous behavior.
+        (WebCore::IDBBackingStore::open):
+        The default parameter provides the previous behavior, which is what
+        non-tests want.
+        * Modules/indexeddb/IDBBackingStore.h:
+        * platform/leveldb/LevelDBDatabase.h:
+
 2013-02-28  Kentaro Hara  <haraken@chromium.org>
 
         Unreviewed, rolling out r144157.
index 370cc15..22641b4 100644 (file)
@@ -324,6 +324,18 @@ WARN_UNUSED_RETURN static bool getMaxObjectStoreId(DBOrTransaction* db, const Ve
     return true;
 }
 
+class DefaultLevelDBFactory : public LevelDBFactory {
+public:
+    virtual PassOwnPtr<LevelDBDatabase> openLevelDB(const String& fileName, const LevelDBComparator* comparator)
+    {
+        return LevelDBDatabase::open(fileName, comparator);
+    }
+    virtual bool destroyLevelDB(const String& fileName)
+    {
+        return LevelDBDatabase::destroy(fileName);
+    }
+};
+
 IDBBackingStore::IDBBackingStore(const String& identifier, IDBFactoryBackendImpl* factory, PassOwnPtr<LevelDBDatabase> db)
     : m_identifier(identifier)
     , m_factory(factory)
@@ -361,6 +373,12 @@ enum IDBLevelDBBackingStoreOpenResult {
 
 PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier, IDBFactoryBackendImpl* factory)
 {
+    DefaultLevelDBFactory levelDBFactory;
+    return IDBBackingStore::open(securityOrigin, pathBaseArg, fileIdentifier, factory, &levelDBFactory);
+}
+
+PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier, IDBFactoryBackendImpl* factory, LevelDBFactory* levelDBFactory)
+{
     IDB_TRACE("IDBBackingStore::open");
     String pathBase = pathBaseArg;
 
@@ -379,15 +397,15 @@ PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin
 
         String path = pathByAppendingComponent(pathBase, securityOrigin->databaseIdentifier() + ".indexeddb.leveldb");
 
-        db = LevelDBDatabase::open(path, comparator.get());
+        db = levelDBFactory->openLevelDB(path, comparator.get());
         if (db) {
             bool known = false;
             bool ok = isSchemaKnown(db.get(), known);
             if (!ok) {
+                LOG_ERROR("IndexedDB had IO error checking schema, treating it as failure to open");
                 HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedIOErrCheckingSchema, IDBLevelDBBackingStoreOpenMax);
-                return PassRefPtr<IDBBackingStore>();
-            }
-            if (!known) {
+                db.clear();
+            } else if (!known) {
                 LOG_ERROR("IndexedDB backing store had unknown schema, treating it as failure to open");
                 HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedUnknownSchema, IDBLevelDBBackingStoreOpenMax);
                 db.clear();
@@ -398,7 +416,7 @@ PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin
             HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenSuccess, IDBLevelDBBackingStoreOpenMax);
         else {
             LOG_ERROR("IndexedDB backing store open failed, attempting cleanup");
-            bool success = LevelDBDatabase::destroy(path);
+            bool success = levelDBFactory->destroyLevelDB(path);
             if (!success) {
                 LOG_ERROR("IndexedDB backing store cleanup failed");
                 HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupDestroyFailed, IDBLevelDBBackingStoreOpenMax);
@@ -406,7 +424,7 @@ PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin
             }
 
             LOG_ERROR("IndexedDB backing store cleanup succeeded, reopening");
-            db = LevelDBDatabase::open(path, comparator.get());
+            db = levelDBFactory->openLevelDB(path, comparator.get());
             if (!db) {
                 LOG_ERROR("IndexedDB backing store reopen after recovery failed");
                 HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupReopenFailed, IDBLevelDBBackingStoreOpenMax);
index 8b49bc8..923d7dc 100644 (file)
@@ -45,12 +45,19 @@ class IDBKeyRange;
 class SecurityOrigin;
 class SharedBuffer;
 
+class LevelDBFactory {
+public:
+    virtual PassOwnPtr<LevelDBDatabase> openLevelDB(const String& fileName, const LevelDBComparator*) = 0;
+    virtual bool destroyLevelDB(const String& fileName) = 0;
+};
+
 class IDBBackingStore : public RefCounted<IDBBackingStore> {
 public:
     class Transaction;
 
     virtual ~IDBBackingStore();
     static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier, IDBFactoryBackendImpl*);
+    static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier, IDBFactoryBackendImpl*, LevelDBFactory*);
 
     virtual Vector<String> getDatabaseNames();
     virtual bool getIDBDatabaseMetaData(const String& name, IDBDatabaseMetadata*, bool& success) WARN_UNUSED_RETURN;
index f9f4d4e..11283cb 100644 (file)
@@ -65,18 +65,20 @@ public:
     static PassOwnPtr<LevelDBDatabase> open(const String& fileName, const LevelDBComparator*);
     static PassOwnPtr<LevelDBDatabase> openInMemory(const LevelDBComparator*);
     static bool destroy(const String& fileName);
-    ~LevelDBDatabase();
+    virtual ~LevelDBDatabase();
 
     bool put(const LevelDBSlice& key, const Vector<char>& value);
     bool remove(const LevelDBSlice& key);
-    bool safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found, const LevelDBSnapshot* = 0);
+    virtual bool safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found, const LevelDBSnapshot* = 0);
     bool write(LevelDBWriteBatch&);
     PassOwnPtr<LevelDBIterator> createIterator(const LevelDBSnapshot* = 0);
     const LevelDBComparator* comparator() const;
 
+protected:
+    LevelDBDatabase();
+
 private:
     friend class LevelDBSnapshot;
-    LevelDBDatabase();
 
     OwnPtr<leveldb::Env> m_env;
     OwnPtr<leveldb::Comparator> m_comparatorAdapter;
index 5874dec..f94d32e 100644 (file)
@@ -1,3 +1,16 @@
+2013-02-28  David Grogan  <dgrogan@chromium.org>
+
+        IndexedDB: IO error when checking schema should destroy LevelDB directory
+        https://bugs.webkit.org/show_bug.cgi?id=110675
+
+        Reviewed by Adam Barth.
+
+        * WebKit.gyp:
+        This was cargo-culted. The component build wouldn't run otherwise.
+
+        * WebKit.gypi:
+        * tests/IDBCleanupOnIOErrorTest.cpp: Added.
+
 2013-02-28  Stephen Chenney  <schenney@chromium.org>
 
         RenderTableCellDeathTest unit test fails on mac
index 82259c2..9d9b606 100644 (file)
                                 # We should not include files depending on webkit_support.
                                 # These tests depend on webkit_support and
                                 # functions defined only in !WEBKIT_IMPLEMENTATION.
+                                'tests/IDBCleanupOnIOErrorTest.cpp',
                                 'tests/LevelDBTest.cpp',
                             ],
                             'conditions': [
index 3b0a7a5..3ae3afe 100644 (file)
@@ -76,6 +76,7 @@
             'tests/GraphicsLayerChromiumTest.cpp',
             'tests/IDBAbortOnCorruptTest.cpp',
             'tests/IDBBindingUtilitiesTest.cpp',
+            'tests/IDBCleanupOnIOErrorTest.cpp',
             'tests/IDBDatabaseBackendTest.cpp',
             'tests/IDBFakeBackingStore.h',
             'tests/IDBKeyPathTest.cpp',
diff --git a/Source/WebKit/chromium/tests/IDBCleanupOnIOErrorTest.cpp b/Source/WebKit/chromium/tests/IDBCleanupOnIOErrorTest.cpp
new file mode 100644 (file)
index 0000000..90dccd6
--- /dev/null
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 2013 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "IDBBackingStore.h"
+#include "LevelDBDatabase.h"
+#include "SecurityOrigin.h"
+#include <gtest/gtest.h>
+#include <webkit/support/webkit_support.h>
+
+#if ENABLE(INDEXED_DATABASE)
+
+using namespace WebCore;
+
+namespace {
+
+class BustedLevelDBDatabase : public LevelDBDatabase {
+public:
+    static PassOwnPtr<LevelDBDatabase> open(const String& fileName, const LevelDBComparator*)
+    {
+        return adoptPtr(new BustedLevelDBDatabase);
+    }
+    virtual bool safeGet(const LevelDBSlice& key, Vector<char>& value, bool& found, const LevelDBSnapshot* = 0)
+    {
+        // false means IO error.
+        return false;
+    }
+};
+
+class MockLevelDBFactory : public LevelDBFactory {
+public:
+    MockLevelDBFactory() : m_destroyCalled(false) { }
+    virtual PassOwnPtr<LevelDBDatabase> openLevelDB(const String& fileName, const LevelDBComparator* comparator)
+    {
+        return BustedLevelDBDatabase::open(fileName, comparator);
+    }
+    virtual bool destroyLevelDB(const String& fileName)
+    {
+        EXPECT_FALSE(m_destroyCalled);
+        m_destroyCalled = true;
+        return false;
+    }
+    virtual ~MockLevelDBFactory()
+    {
+        EXPECT_TRUE(m_destroyCalled);
+    }
+private:
+    bool m_destroyCalled;
+};
+
+TEST(IDBIOErrorTest, CleanUpTest)
+{
+    RefPtr<SecurityOrigin> origin = SecurityOrigin::create("http", "localhost", 81);
+    OwnPtr<webkit_support::ScopedTempDirectory> tempDirectory = adoptPtr(webkit_support::CreateScopedTempDirectory());
+    tempDirectory->CreateUniqueTempDir();
+    const String path = String::fromUTF8(tempDirectory->path().c_str());
+    String dummyFileIdentifier;
+    IDBFactoryBackendImpl* dummyIDBFactory = 0;
+    MockLevelDBFactory mockLevelDBFactory;
+    RefPtr<IDBBackingStore> backingStore = IDBBackingStore::open(origin.get(), path, dummyFileIdentifier, dummyIDBFactory, &mockLevelDBFactory);
+}
+
+} // namespace
+
+#endif // ENABLE(INDEXED_DATABASE)