summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--content/child/web_database_observer_impl.cc22
-rw-r--r--content/child/web_database_observer_impl.h7
-rw-r--r--content/common/database_connections_unittest.cc36
-rw-r--r--content/renderer/render_thread_impl.cc11
-rw-r--r--storage/common/database/database_connections.cc48
-rw-r--r--storage/common/database/database_connections.h14
-rw-r--r--third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp76
7 files changed, 88 insertions, 126 deletions
diff --git a/content/child/web_database_observer_impl.cc b/content/child/web_database_observer_impl.cc
index 68534a2..8209f88 100644
--- a/content/child/web_database_observer_impl.cc
+++ b/content/child/web_database_observer_impl.cc
@@ -4,8 +4,11 @@
#include "content/child/web_database_observer_impl.h"
+#include "base/bind.h"
#include "base/metrics/histogram.h"
+#include "base/single_thread_task_runner.h"
#include "base/strings/string16.h"
+#include "base/thread_task_runner_handle.h"
#include "content/common/database_messages.h"
#include "third_party/WebKit/public/platform/WebCString.h"
#include "third_party/WebKit/public/platform/WebString.h"
@@ -54,8 +57,10 @@ int DetermineHistogramResult(int websql_error, int sqlite_error) {
WebDatabaseObserverImpl::WebDatabaseObserverImpl(IPC::SyncMessageFilter* sender)
: sender_(sender),
- open_connections_(new storage::DatabaseConnectionsWrapper) {
+ open_connections_(new storage::DatabaseConnectionsWrapper),
+ main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
DCHECK(sender);
+ DCHECK(main_thread_task_runner_);
}
WebDatabaseObserverImpl::~WebDatabaseObserverImpl() {
@@ -83,8 +88,13 @@ void WebDatabaseObserverImpl::databaseModified(
void WebDatabaseObserverImpl::databaseClosed(
const WebString& origin_identifier,
const WebString& database_name) {
- sender_->Send(new DatabaseHostMsg_Closed(
- origin_identifier.utf8(), database_name));
+ DCHECK(!main_thread_task_runner_->RunsTasksOnCurrentThread());
+ main_thread_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(
+ base::IgnoreResult(&IPC::SyncMessageFilter::Send),
+ sender_,
+ new DatabaseHostMsg_Closed(origin_identifier.utf8(), database_name)));
open_connections_->RemoveOpenConnection(origin_identifier.utf8(),
database_name);
}
@@ -156,8 +166,10 @@ void WebDatabaseObserverImpl::reportVacuumDatabaseResult(
HandleSqliteError(origin_identifier, database_name, sqlite_error);
}
-void WebDatabaseObserverImpl::WaitForAllDatabasesToClose() {
- open_connections_->WaitForAllDatabasesToClose();
+bool WebDatabaseObserverImpl::WaitForAllDatabasesToClose(
+ base::TimeDelta timeout) {
+ DCHECK(main_thread_task_runner_->RunsTasksOnCurrentThread());
+ return open_connections_->WaitForAllDatabasesToClose(timeout);
}
void WebDatabaseObserverImpl::HandleSqliteError(
diff --git a/content/child/web_database_observer_impl.h b/content/child/web_database_observer_impl.h
index 0a05773..bfda6c4 100644
--- a/content/child/web_database_observer_impl.h
+++ b/content/child/web_database_observer_impl.h
@@ -10,6 +10,10 @@
#include "storage/common/database/database_connections.h"
#include "third_party/WebKit/public/platform/WebDatabaseObserver.h"
+namespace base {
+class SingleThreadTaskRunner;
+}
+
namespace content {
class WebDatabaseObserverImpl : public blink::WebDatabaseObserver {
@@ -55,7 +59,7 @@ class WebDatabaseObserverImpl : public blink::WebDatabaseObserver {
const blink::WebString& database_name,
int sqlite_error) override;
- void WaitForAllDatabasesToClose();
+ bool WaitForAllDatabasesToClose(base::TimeDelta timeout);
private:
void HandleSqliteError(const blink::WebString& origin_identifier,
@@ -64,6 +68,7 @@ class WebDatabaseObserverImpl : public blink::WebDatabaseObserver {
scoped_refptr<IPC::SyncMessageFilter> sender_;
scoped_refptr<storage::DatabaseConnectionsWrapper> open_connections_;
+ scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
DISALLOW_COPY_AND_ASSIGN(WebDatabaseObserverImpl);
};
diff --git a/content/common/database_connections_unittest.cc b/content/common/database_connections_unittest.cc
index 8992f0d..a329553d 100644
--- a/content/common/database_connections_unittest.cc
+++ b/content/common/database_connections_unittest.cc
@@ -26,16 +26,6 @@ void RemoveConnectionTask(
obj->RemoveOpenConnection(origin_id, database_name);
}
-void ScheduleRemoveConnectionTask(
- base::Thread* thread, const std::string& origin_id,
- const base::string16& database_name,
- scoped_refptr<DatabaseConnectionsWrapper> obj,
- bool* did_task_execute) {
- thread->task_runner()->PostTask(
- FROM_HERE, base::Bind(&RemoveConnectionTask, origin_id, database_name,
- obj, did_task_execute));
-}
-
} // anonymous namespace
TEST(DatabaseConnectionsTest, DatabaseConnectionsTest) {
@@ -115,29 +105,21 @@ TEST(DatabaseConnectionsTest, DatabaseConnectionsWrapperTest) {
EXPECT_TRUE(obj->HasOpenConnections());
obj->RemoveOpenConnection(kOriginId, kName);
EXPECT_FALSE(obj->HasOpenConnections());
- obj->WaitForAllDatabasesToClose(); // should return immediately
-
- // Test WaitForAllDatabasesToClose with the last connection
- // being removed on the current thread.
- obj->AddOpenConnection(kOriginId, kName);
- bool did_task_execute = false;
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(&RemoveConnectionTask, kOriginId, kName, obj,
- &did_task_execute));
- obj->WaitForAllDatabasesToClose(); // should return after the task executes
- EXPECT_TRUE(did_task_execute);
- EXPECT_FALSE(obj->HasOpenConnections());
+ EXPECT_TRUE(obj->WaitForAllDatabasesToClose(base::TimeDelta()));
// Test WaitForAllDatabasesToClose with the last connection
// being removed on another thread.
obj->AddOpenConnection(kOriginId, kName);
+ EXPECT_FALSE(obj->WaitForAllDatabasesToClose(base::TimeDelta()));
base::Thread thread("WrapperTestThread");
thread.Start();
- did_task_execute = false;
- base::ThreadTaskRunnerHandle::Get()->PostTask(
- FROM_HERE, base::Bind(&ScheduleRemoveConnectionTask, &thread, kOriginId,
- kName, obj, &did_task_execute));
- obj->WaitForAllDatabasesToClose(); // should return after the task executes
+ bool did_task_execute = false;
+ thread.task_runner()->PostTask(
+ FROM_HERE, base::Bind(&RemoveConnectionTask, kOriginId, kName, obj,
+ &did_task_execute));
+ // Use a long timeout value to avoid timeouts on test bots.
+ EXPECT_TRUE(obj->WaitForAllDatabasesToClose(
+ base::TimeDelta::FromSeconds(15)));
EXPECT_TRUE(did_task_execute);
EXPECT_FALSE(obj->HasOpenConnections());
}
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index b6d9dd6..6293989 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -819,13 +819,10 @@ void RenderThreadImpl::Shutdown() {
// Wait for all databases to be closed.
if (blink_platform_impl_) {
- // WaitForAllDatabasesToClose might run a nested message loop. To avoid
- // processing timer events while we're already in the process of shutting
- // down blink, put a ScopePageLoadDeferrer on the stack.
- WebView::willEnterModalLoop();
- blink_platform_impl_->web_database_observer_impl()
- ->WaitForAllDatabasesToClose();
- WebView::didExitModalLoop();
+ // Crash the process if they fail to close after a generous amount of time.
+ bool all_closed = blink_platform_impl_->web_database_observer_impl()
+ ->WaitForAllDatabasesToClose(base::TimeDelta::FromSeconds(60));
+ CHECK(all_closed);
}
// Shutdown in reverse of the initialization order.
diff --git a/storage/common/database/database_connections.cc b/storage/common/database/database_connections.cc
index afa9ee3..35bc139 100644
--- a/storage/common/database/database_connections.cc
+++ b/storage/common/database/database_connections.cc
@@ -8,6 +8,7 @@
#include "base/bind.h"
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
+#include "base/synchronization/waitable_event.h"
#include "base/thread_task_runner_handle.h"
namespace storage {
@@ -121,27 +122,13 @@ bool DatabaseConnections::RemoveConnectionsHelper(
return true;
}
-DatabaseConnectionsWrapper::DatabaseConnectionsWrapper()
- : waiting_for_dbs_to_close_(false),
- main_thread_(base::ThreadTaskRunnerHandle::Get()) {
+DatabaseConnectionsWrapper::DatabaseConnectionsWrapper() {
}
DatabaseConnectionsWrapper::~DatabaseConnectionsWrapper() {
}
-void DatabaseConnectionsWrapper::WaitForAllDatabasesToClose() {
- // We assume that new databases won't be open while we're waiting.
- DCHECK(main_thread_->BelongsToCurrentThread());
- if (HasOpenConnections()) {
- base::AutoReset<bool> auto_reset(&waiting_for_dbs_to_close_, true);
- base::MessageLoop::ScopedNestableTaskAllower allow(
- base::MessageLoop::current());
- base::MessageLoop::current()->Run();
- }
-}
-
bool DatabaseConnectionsWrapper::HasOpenConnections() {
- DCHECK(main_thread_->BelongsToCurrentThread());
base::AutoLock auto_lock(open_connections_lock_);
return !open_connections_.IsEmpty();
}
@@ -149,7 +136,6 @@ bool DatabaseConnectionsWrapper::HasOpenConnections() {
void DatabaseConnectionsWrapper::AddOpenConnection(
const std::string& origin_identifier,
const base::string16& database_name) {
- // We add to the collection immediately on any thread.
base::AutoLock auto_lock(open_connections_lock_);
open_connections_.AddConnection(origin_identifier, database_name);
}
@@ -157,19 +143,27 @@ void DatabaseConnectionsWrapper::AddOpenConnection(
void DatabaseConnectionsWrapper::RemoveOpenConnection(
const std::string& origin_identifier,
const base::string16& database_name) {
- // But only remove from the collection on the main thread
- // so we can handle the waiting_for_dbs_to_close_ case.
- if (!main_thread_->BelongsToCurrentThread()) {
- main_thread_->PostTask(
- FROM_HERE,
- base::Bind(&DatabaseConnectionsWrapper::RemoveOpenConnection, this,
- origin_identifier, database_name));
- return;
- }
base::AutoLock auto_lock(open_connections_lock_);
open_connections_.RemoveConnection(origin_identifier, database_name);
- if (waiting_for_dbs_to_close_ && open_connections_.IsEmpty())
- base::MessageLoop::current()->QuitWhenIdle();
+ if (waiting_to_close_event_ && open_connections_.IsEmpty())
+ waiting_to_close_event_->Signal();
+}
+
+bool DatabaseConnectionsWrapper::WaitForAllDatabasesToClose(
+ base::TimeDelta timeout) {
+ base::WaitableEvent waitable_event(true, false);
+ {
+ base::AutoLock auto_lock(open_connections_lock_);
+ if (open_connections_.IsEmpty())
+ return true;
+ waiting_to_close_event_ = &waitable_event;
+ }
+ waitable_event.TimedWait(timeout);
+ {
+ base::AutoLock auto_lock(open_connections_lock_);
+ waiting_to_close_event_ = nullptr;
+ return open_connections_.IsEmpty();
+ }
}
} // namespace storage
diff --git a/storage/common/database/database_connections.h b/storage/common/database/database_connections.h
index 24bcbd0..ec36b0f 100644
--- a/storage/common/database/database_connections.h
+++ b/storage/common/database/database_connections.h
@@ -12,10 +12,12 @@
#include "base/memory/ref_counted.h"
#include "base/strings/string16.h"
#include "base/synchronization/lock.h"
+#include "base/time/time.h"
#include "storage/common/storage_common_export.h"
namespace base {
class SingleThreadTaskRunner;
+class WaitableEvent;
}
namespace storage {
@@ -74,24 +76,22 @@ class STORAGE_COMMON_EXPORT DatabaseConnectionsWrapper
public:
DatabaseConnectionsWrapper();
- // The Wait and Has methods should only be called on the
- // main thread (the thread on which the wrapper is constructed).
- void WaitForAllDatabasesToClose();
bool HasOpenConnections();
-
- // Add and Remove may be called on any thread.
void AddOpenConnection(const std::string& origin_identifier,
const base::string16& database_name);
void RemoveOpenConnection(const std::string& origin_identifier,
const base::string16& database_name);
+
+ // Returns true if all databases are closed.
+ bool WaitForAllDatabasesToClose(base::TimeDelta timeout);
+
private:
~DatabaseConnectionsWrapper();
friend class base::RefCountedThreadSafe<DatabaseConnectionsWrapper>;
- bool waiting_for_dbs_to_close_;
base::Lock open_connections_lock_;
DatabaseConnections open_connections_;
- scoped_refptr<base::SingleThreadTaskRunner> main_thread_;
+ base::WaitableEvent* waiting_to_close_event_ = nullptr;
};
} // namespace storage
diff --git a/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp b/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
index 8bb0199..e6833f6 100644
--- a/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
+++ b/third_party/WebKit/Source/modules/webdatabase/DatabaseTracker.cpp
@@ -106,60 +106,36 @@ void DatabaseTracker::addOpenDatabase(Database* database)
databaseSet->add(database);
}
-class NotifyDatabaseObserverOnCloseTask final : public ExecutionContextTask {
-public:
- static PassOwnPtr<NotifyDatabaseObserverOnCloseTask> create(Database* database)
- {
- return adoptPtr(new NotifyDatabaseObserverOnCloseTask(database));
- }
-
- void performTask(ExecutionContext*) override
- {
- databaseClosed(m_database.get());
- }
-
-private:
- explicit NotifyDatabaseObserverOnCloseTask(Database* database)
- : m_database(database)
- {
- }
-
- CrossThreadPersistent<Database> m_database;
-};
-
void DatabaseTracker::removeOpenDatabase(Database* database)
{
- String originIdentifier = createDatabaseIdentifierFromSecurityOrigin(database->securityOrigin());
- MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
- ASSERT(m_openDatabaseMap);
- DatabaseNameMap* nameMap = m_openDatabaseMap->get(originIdentifier);
- if (!nameMap)
- return;
+ {
+ MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
+ String originIdentifier = createDatabaseIdentifierFromSecurityOrigin(database->securityOrigin());
+ ASSERT(m_openDatabaseMap);
+ DatabaseNameMap* nameMap = m_openDatabaseMap->get(originIdentifier);
+ if (!nameMap)
+ return;
- String name(database->stringIdentifier());
- DatabaseSet* databaseSet = nameMap->get(name);
- if (!databaseSet)
- return;
+ String name(database->stringIdentifier());
+ DatabaseSet* databaseSet = nameMap->get(name);
+ if (!databaseSet)
+ return;
- DatabaseSet::iterator found = databaseSet->find(database);
- if (found == databaseSet->end())
- return;
+ DatabaseSet::iterator found = databaseSet->find(database);
+ if (found == databaseSet->end())
+ return;
- databaseSet->remove(found);
- if (databaseSet->isEmpty()) {
- nameMap->remove(name);
- delete databaseSet;
- if (nameMap->isEmpty()) {
- m_openDatabaseMap->remove(originIdentifier);
- delete nameMap;
+ databaseSet->remove(found);
+ if (databaseSet->isEmpty()) {
+ nameMap->remove(name);
+ delete databaseSet;
+ if (nameMap->isEmpty()) {
+ m_openDatabaseMap->remove(originIdentifier);
+ delete nameMap;
+ }
}
}
-
- ExecutionContext* executionContext = database->databaseContext()->executionContext();
- if (!executionContext->isContextThread())
- executionContext->postTask(BLINK_FROM_HERE, NotifyDatabaseObserverOnCloseTask::create(database));
- else
- databaseClosed(database);
+ databaseClosed(database);
}
void DatabaseTracker::prepareToOpenDatabase(Database* database)
@@ -176,11 +152,7 @@ void DatabaseTracker::prepareToOpenDatabase(Database* database)
void DatabaseTracker::failedToOpenDatabase(Database* database)
{
- ExecutionContext* executionContext = database->databaseContext()->executionContext();
- if (!executionContext->isContextThread())
- executionContext->postTask(BLINK_FROM_HERE, NotifyDatabaseObserverOnCloseTask::create(database));
- else
- databaseClosed(database);
+ databaseClosed(database);
}
unsigned long long DatabaseTracker::getMaxSizeForDatabase(const Database* database)