Skip to content

Commit

Permalink
IndexedDB: Notify Quota about write errors.
Browse files Browse the repository at this point in the history 
Adds calls to QuotaManagerProxy::NotifyWriteFailed() where write
errors might occur in IndexedDB code. With this information, the quota
system can reasonably guess if the disk storing the user's profile is
out of space, and decide if any actions should be taken from there.

Bug: 997258
Change-Id: Ifd7ea5c129a8467e47676a5d9ab157ef2015daea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2029308
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741674}
  • Loading branch information
Jarryd authored and Commit Bot committed Feb 15, 2020
1 parent 57ae4c5 commit 1752f0c
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 15 deletions.
10 changes: 10 additions & 0 deletions content/browser/indexed_db/indexed_db_factory_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,10 @@ IndexedDBFactoryImpl::GetOrOpenOriginFactory(
origin);

if (disk_full) {
context_->IOTaskRunner()->PostTask(
FROM_HERE,
base::BindOnce(&storage::QuotaManagerProxy::NotifyWriteFailed,
context_->quota_manager_proxy(), origin));
return {IndexedDBOriginStateHandle(), s,
IndexedDBDatabaseError(
blink::mojom::IDBException::kQuotaError,
Expand Down Expand Up @@ -985,6 +989,12 @@ void IndexedDBFactoryImpl::OnDatabaseError(const url::Origin& origin,
base::ASCIIToUTF16(status.ToString()));
HandleBackingStoreCorruption(origin, error);
} else {
if (status.IsIOError()) {
context_->IOTaskRunner()->PostTask(
FROM_HERE,
base::BindOnce(&storage::QuotaManagerProxy::NotifyWriteFailed,
context_->quota_manager_proxy(), origin));
}
HandleBackingStoreFailure(origin);
}
}
Expand Down
62 changes: 47 additions & 15 deletions content/browser/indexed_db/indexed_db_factory_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include <stdint.h>
#include <memory>
#include <utility>

#include "base/auto_reset.h"
Expand All @@ -11,6 +12,7 @@
#include "base/files/scoped_temp_dir.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/scoped_refptr.h"
#include "base/run_loop.h"
#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
Expand All @@ -23,6 +25,7 @@
#include "components/services/storage/indexed_db/leveldb/fake_leveldb_factory.h"
#include "components/services/storage/indexed_db/transactional_leveldb/transactional_leveldb_database.h"
#include "components/services/storage/public/mojom/indexed_db_control.mojom-test-utils.h"
#include "content/browser/indexed_db/indexed_db_backing_store.h"
#include "content/browser/indexed_db/indexed_db_class_factory.h"
#include "content/browser/indexed_db/indexed_db_connection.h"
#include "content/browser/indexed_db/indexed_db_context_impl.h"
Expand All @@ -39,6 +42,7 @@
#include "content/public/test/test_utils.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "storage/browser/test/mock_quota_manager_proxy.h"
#include "storage/browser/test/mock_special_storage_policy.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/indexeddb/web_idb_types.h"
#include "third_party/blink/public/mojom/indexeddb/indexeddb.mojom.h"
Expand All @@ -53,11 +57,6 @@ namespace content {

namespace {

base::FilePath CreateAndReturnTempDir(base::ScopedTempDir* temp_dir) {
CHECK(temp_dir->CreateUniqueTempDir());
return temp_dir->GetPath();
}

void CreateAndBindTransactionPlaceholder(
base::WeakPtr<IndexedDBTransaction> transaction) {}

Expand All @@ -66,17 +65,22 @@ void CreateAndBindTransactionPlaceholder(
class IndexedDBFactoryTest : public testing::Test {
public:
IndexedDBFactoryTest()
: task_environment_(std::make_unique<BrowserTaskEnvironment>()),
quota_manager_proxy_(
base::MakeRefCounted<storage::MockQuotaManagerProxy>(nullptr,
nullptr)) {}
: task_environment_(std::make_unique<BrowserTaskEnvironment>()) {}

explicit IndexedDBFactoryTest(
std::unique_ptr<BrowserTaskEnvironment> task_environment)
: task_environment_(std::move(task_environment)),
quota_manager_proxy_(
base::MakeRefCounted<storage::MockQuotaManagerProxy>(nullptr,
nullptr)) {}
: task_environment_(std::move(task_environment)) {}

void SetUp() override {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
quota_policy_ = base::MakeRefCounted<storage::MockSpecialStoragePolicy>();
quota_manager_ = base::MakeRefCounted<storage::MockQuotaManager>(
false /*is_incognito*/, temp_dir_.GetPath(),
base::ThreadTaskRunnerHandle::Get().get(), quota_policy_.get());

quota_manager_proxy_ = base::MakeRefCounted<storage::MockQuotaManagerProxy>(
quota_manager_.get(), base::ThreadTaskRunnerHandle::Get().get());
}

void TearDown() override {
quota_manager_proxy_->SimulateQuotaManagerDestroyed();
Expand Down Expand Up @@ -118,11 +122,12 @@ class IndexedDBFactoryTest : public testing::Test {
if (temp_dir_.IsValid())
ASSERT_TRUE(temp_dir_.Delete());
IndexedDBClassFactory::Get()->SetLevelDBFactoryForTesting(nullptr);
quota_manager_.reset();
}

void SetupContext() {
context_ = base::MakeRefCounted<IndexedDBContextImpl>(
CreateAndReturnTempDir(&temp_dir_),
temp_dir_.GetPath(),
/*special_storage_policy=*/nullptr, quota_manager_proxy_.get(),
base::DefaultClock::GetInstance(),
/*blob_storage_context=*/mojo::NullRemote(),
Expand All @@ -144,7 +149,7 @@ class IndexedDBFactoryTest : public testing::Test {

void SetupContextWithFactories(LevelDBFactory* factory, base::Clock* clock) {
context_ = base::MakeRefCounted<IndexedDBContextImpl>(
CreateAndReturnTempDir(&temp_dir_),
temp_dir_.GetPath(),
/*special_storage_policy=*/nullptr, quota_manager_proxy_.get(), clock,
/*blob_storage_context=*/mojo::NullRemote(),
/*native_file_system_context=*/mojo::NullRemote(),
Expand Down Expand Up @@ -221,10 +226,14 @@ class IndexedDBFactoryTest : public testing::Test {
return handle.origin_state();
}

storage::MockQuotaManager* quota_manager() { return quota_manager_.get(); }

private:
std::unique_ptr<BrowserTaskEnvironment> task_environment_;

base::ScopedTempDir temp_dir_;
scoped_refptr<storage::MockSpecialStoragePolicy> quota_policy_;
scoped_refptr<storage::MockQuotaManager> quota_manager_;
scoped_refptr<storage::MockQuotaManagerProxy> quota_manager_proxy_;
scoped_refptr<IndexedDBContextImpl> context_;

Expand Down Expand Up @@ -741,6 +750,29 @@ TEST_F(IndexedDBFactoryTest, QuotaErrorOnDiskFull) {
std::move(create_transaction_callback));
factory()->Open(name, std::move(connection), origin, context()->data_path());
EXPECT_TRUE(callbacks->error_called());
base::RunLoop().RunUntilIdle();

ASSERT_EQ(1U, quota_manager()->write_error_tracker().size());
EXPECT_EQ(origin, quota_manager()->write_error_tracker().begin()->first);
EXPECT_EQ(1, quota_manager()->write_error_tracker().begin()->second);
}

TEST_F(IndexedDBFactoryTest, NotifyQuotaOnDatabaseError) {
SetupContext();
const Origin origin = Origin::Create(GURL("www.example.com"));
factory()->OnDatabaseError(origin,
leveldb::Status::Corruption("Corrupted stuff."),
"Corrupted stuff.");
base::RunLoop().RunUntilIdle();
// Quota should not be notified unless the status is IOError.
ASSERT_EQ(0U, quota_manager()->write_error_tracker().size());

factory()->OnDatabaseError(origin, leveldb::Status::IOError("Disk is full."),
"Disk is full.");
base::RunLoop().RunUntilIdle();
ASSERT_EQ(1U, quota_manager()->write_error_tracker().size());
EXPECT_EQ(origin, quota_manager()->write_error_tracker().begin()->first);
EXPECT_EQ(1, quota_manager()->write_error_tracker().begin()->second);
}

class ErrorCallbacks : public MockIndexedDBCallbacks {
Expand Down

0 comments on commit 1752f0c

Please sign in to comment.