Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(396)

Unified Diff: third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp

Issue 1733353004: Introduce WorkerBackingThread (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
diff --git a/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp b/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
index 2154ae7f54d597edf8f2a6120b3a773ccd3dca00..7e997a9f7493e7e54c25a146281641ccbbc5407d 100644
--- a/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
+++ b/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
@@ -8,6 +8,7 @@
#include "bindings/core/v8/V8GCController.h"
#include "core/inspector/ConsoleMessage.h"
#include "core/testing/DummyPageHolder.h"
+#include "core/workers/WorkerBackingThread.h"
#include "core/workers/WorkerLoaderProxy.h"
#include "core/workers/WorkerObjectProxy.h"
#include "core/workers/WorkerThreadStartupData.h"
@@ -23,45 +24,6 @@
namespace blink {
namespace {
-class TestCompositorWorkerThread : public CompositorWorkerThread {
-public:
- TestCompositorWorkerThread(WorkerLoaderProxyProvider* loaderProxyProvider, WorkerObjectProxy& objectProxy, double timeOrigin, WaitableEvent* startEvent)
- : CompositorWorkerThread(WorkerLoaderProxy::create(loaderProxyProvider), objectProxy, timeOrigin)
- , m_startEvent(startEvent)
- {
- }
-
- ~TestCompositorWorkerThread() override {}
-
- void setCallbackAfterV8Termination(PassOwnPtr<Function<void()>> callback)
- {
- m_v8TerminationCallback = callback;
- }
-
-private:
- // WorkerThread:
- void didStartWorkerThread() override
- {
- m_startEvent->signal();
- }
- void terminateV8Execution() override
- {
- CompositorWorkerThread::terminateV8Execution();
- if (m_v8TerminationCallback)
- (*m_v8TerminationCallback)();
- }
-
- void willDestroyIsolate() override
- {
- v8::Isolate::GetCurrent()->RequestGarbageCollectionForTesting(v8::Isolate::kFullGarbageCollection);
- Heap::collectAllGarbage();
- CompositorWorkerThread::willDestroyIsolate();
- }
-
- WaitableEvent* m_startEvent;
- OwnPtr<Function<void()>> m_v8TerminationCallback;
-};
-
// A null WorkerObjectProxy, supplied when creating CompositorWorkerThreads.
class TestCompositorWorkerObjectProxy : public WorkerObjectProxy {
public:
@@ -119,6 +81,7 @@ class CompositorWorkerThreadTest : public ::testing::Test {
public:
void SetUp() override
{
+ CompositorWorkerThread::resetSharedBackingThreadForTest();
m_page = DummyPageHolder::create();
m_objectProxy = TestCompositorWorkerObjectProxy::create(&m_page->document());
m_securityOrigin = SecurityOrigin::create(KURL(ParsedURLString, "http://fake.url/"));
@@ -126,15 +89,15 @@ public:
void TearDown() override
{
- ASSERT(!hasThread());
- ASSERT(!hasIsolate());
m_page.clear();
+ CompositorWorkerThread::resetSharedBackingThreadForTest();
}
- PassRefPtr<TestCompositorWorkerThread> createCompositorWorker(WaitableEvent* startEvent)
+ PassRefPtr<CompositorWorkerThread> createCompositorWorker()
{
- TestCompositorWorkerThread* workerThread = new TestCompositorWorkerThread(nullptr, *m_objectProxy, 0, startEvent);
+ RefPtr<CompositorWorkerThread> workerThread = CompositorWorkerThread::create(nullptr, *m_objectProxy, 0);
OwnPtrWillBeRawPtr<WorkerClients> clients = nullptr;
+ workerThread->workerBackingThread().setInitializationEventForTest();
workerThread->start(WorkerThreadStartupData::create(
KURL(ParsedURLString, "http://fake.url/"),
"fake user agent",
@@ -145,12 +108,8 @@ public:
m_securityOrigin.get(),
clients.release(),
V8CacheOptionsDefault));
- return adoptRef(workerThread);
- }
- void createWorkerAdapter(RefPtr<CompositorWorkerThread>* workerThread, WaitableEvent* creationEvent)
- {
- *workerThread = createCompositorWorker(creationEvent);
+ return workerThread;
}
// Attempts to run some simple script for |worker|.
@@ -162,20 +121,19 @@ public:
waitEvent->wait();
}
- void waitForWaitableEventAfterIteratingCurrentLoop(WaitableEvent* waitEvent)
- {
- testing::runPendingTasks();
- waitEvent->wait();
- }
-
- bool hasThread() const
+ bool hasCurrentIsolateInWorkerThread(WorkerBackingThread* thread)
{
- return CompositorWorkerThread::hasThreadForTest();
+ WaitableEvent event;
+ bool hasIsolate = false;
+ thread->backingThread().platformThread().taskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&hasCurrentIsolate, AllowCrossThreadAccess(&event), AllowCrossThreadAccess(&hasIsolate)));
+ event.wait();
+ return hasIsolate;
}
- bool hasIsolate() const
+ void waitForWaitableEventAfterIteratingCurrentLoop(WaitableEvent* waitEvent)
{
- return CompositorWorkerThread::hasIsolateForTest();
+ testing::runPendingTasks();
+ waitEvent->wait();
}
private:
@@ -187,6 +145,12 @@ private:
waitEvent->signal();
}
+ static void hasCurrentIsolate(WaitableEvent* event, bool* hasIsolate)
+ {
+ *hasIsolate = !!v8::Isolate::GetCurrent();
+ event->signal();
+ }
+
OwnPtr<DummyPageHolder> m_page;
RefPtr<SecurityOrigin> m_securityOrigin;
OwnPtr<WorkerObjectProxy> m_objectProxy;
@@ -195,9 +159,8 @@ private:
TEST_F(CompositorWorkerThreadTest, Basic)
{
- OwnPtr<WaitableEvent> creationEvent = adoptPtr(new WaitableEvent());
- RefPtr<CompositorWorkerThread> compositorWorker = createCompositorWorker(creationEvent.get());
- waitForWaitableEventAfterIteratingCurrentLoop(creationEvent.get());
+ RefPtr<CompositorWorkerThread> compositorWorker = createCompositorWorker();
+ waitForWaitableEventAfterIteratingCurrentLoop(compositorWorker->workerBackingThread().initializationEventForTest());
kinuko 2016/02/29 09:32:54 Would it be possible to just replace this with pos
yhirano 2016/02/29 23:47:32 Done.
checkWorkerCanExecuteScript(compositorWorker.get());
compositorWorker->terminateAndWait();
}
@@ -206,25 +169,22 @@ TEST_F(CompositorWorkerThreadTest, Basic)
TEST_F(CompositorWorkerThreadTest, CreateSecondAndTerminateFirst)
{
// Create the first worker and wait until it is initialized.
- OwnPtr<WaitableEvent> firstCreationEvent = adoptPtr(new WaitableEvent());
- RefPtr<CompositorWorkerThread> firstWorker = createCompositorWorker(firstCreationEvent.get());
- WebThreadSupportingGC* firstThread = CompositorWorkerThread::sharedBackingThread();
+ RefPtr<CompositorWorkerThread> firstWorker = createCompositorWorker();
+ WebThreadSupportingGC* firstThread = &firstWorker->backingThread();
ASSERT(firstThread);
- waitForWaitableEventAfterIteratingCurrentLoop(firstCreationEvent.get());
+ waitForWaitableEventAfterIteratingCurrentLoop(firstWorker->workerBackingThread().initializationEventForTest());
v8::Isolate* firstIsolate = firstWorker->isolate();
ASSERT(firstIsolate);
// Create the second worker and immediately destroy the first worker.
- OwnPtr<WaitableEvent> secondCreationEvent = adoptPtr(new WaitableEvent());
- RefPtr<CompositorWorkerThread> secondWorker = createCompositorWorker(secondCreationEvent.get());
+ RefPtr<CompositorWorkerThread> secondWorker = createCompositorWorker();
firstWorker->terminateAndWait();
// Wait until the second worker is initialized. Verify that the second worker is using the same
// thread and Isolate as the first worker.
- WebThreadSupportingGC* secondThread = CompositorWorkerThread::sharedBackingThread();
+ WebThreadSupportingGC* secondThread = &secondWorker->backingThread();
ASSERT(secondThread);
- waitForWaitableEventAfterIteratingCurrentLoop(secondCreationEvent.get());
- EXPECT_EQ(firstThread, secondThread);
+ ASSERT_EQ(firstThread, secondThread);
v8::Isolate* secondIsolate = secondWorker->isolate();
ASSERT(secondIsolate);
@@ -246,25 +206,32 @@ static void checkCurrentIsolate(v8::Isolate* isolate, WaitableEvent* event)
TEST_F(CompositorWorkerThreadTest, TerminateFirstAndCreateSecond)
{
// Create the first worker, wait until it is initialized, and terminate it.
- OwnPtr<WaitableEvent> creationEvent = adoptPtr(new WaitableEvent());
- RefPtr<CompositorWorkerThread> compositorWorker = createCompositorWorker(creationEvent.get());
- WebThreadSupportingGC* firstThread = CompositorWorkerThread::sharedBackingThread();
- waitForWaitableEventAfterIteratingCurrentLoop(creationEvent.get());
- ASSERT(compositorWorker->isolate());
+ RefPtr<CompositorWorkerThread> compositorWorker = createCompositorWorker();
+ RefPtr<WorkerBackingThread> workerBackingThread = &compositorWorker->workerBackingThread();
+ WebThreadSupportingGC* firstThread = &compositorWorker->backingThread();
+ waitForWaitableEventAfterIteratingCurrentLoop(compositorWorker->workerBackingThread().initializationEventForTest());
+ v8::Isolate* firstIsolate = compositorWorker->isolate();
+ ASSERT(firstIsolate);
+
+ EXPECT_TRUE(hasCurrentIsolateInWorkerThread(workerBackingThread.get()));
compositorWorker->terminateAndWait();
- // Create the second worker. Verify that the second worker lives in a different WebThread since the first
- // thread will have been destroyed after destroying the first worker.
- creationEvent = adoptPtr(new WaitableEvent());
- compositorWorker = createCompositorWorker(creationEvent.get());
- WebThreadSupportingGC* secondThread = CompositorWorkerThread::sharedBackingThread();
- EXPECT_NE(firstThread, secondThread);
- waitForWaitableEventAfterIteratingCurrentLoop(creationEvent.get());
+ // The backing thread is still alive but its isolate is detached.
+ // TODO(yhirano): Enable this check if / when returning null from
+ // v8::Isolate::GetCurrent() is allowed.
+ // EXPECT_FALSE(hasCurrentIsolateInWorkerThread(workerBackingThread.get()));
+
+ // Create the second worker. The backing thread is same.
+ compositorWorker = createCompositorWorker();
+ WebThreadSupportingGC* secondThread = &compositorWorker->backingThread();
+ EXPECT_EQ(firstThread, secondThread);
+ waitForWaitableEventAfterIteratingCurrentLoop(compositorWorker->workerBackingThread().initializationEventForTest());
// Jump over to the worker's thread to verify that the Isolate is set up correctly and execute script.
OwnPtr<WaitableEvent> checkEvent = adoptPtr(new WaitableEvent());
secondThread->platformThread().taskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&checkCurrentIsolate, AllowCrossThreadAccess(compositorWorker->isolate()), AllowCrossThreadAccess(checkEvent.get())));
waitForWaitableEventAfterIteratingCurrentLoop(checkEvent.get());
+ EXPECT_TRUE(hasCurrentIsolateInWorkerThread(workerBackingThread.get()));
checkWorkerCanExecuteScript(compositorWorker.get());
compositorWorker->terminateAndWait();
@@ -273,21 +240,22 @@ TEST_F(CompositorWorkerThreadTest, TerminateFirstAndCreateSecond)
// Tests that v8::Isolate and WebThread are correctly set-up if a worker is created while another is terminating.
TEST_F(CompositorWorkerThreadTest, CreatingSecondDuringTerminationOfFirst)
{
- OwnPtr<WaitableEvent> firstCreationEvent = adoptPtr(new WaitableEvent());
- RefPtr<TestCompositorWorkerThread> firstWorker = createCompositorWorker(firstCreationEvent.get());
- waitForWaitableEventAfterIteratingCurrentLoop(firstCreationEvent.get());
+ RefPtr<CompositorWorkerThread> firstWorker = createCompositorWorker();
+ waitForWaitableEventAfterIteratingCurrentLoop(firstWorker->workerBackingThread().initializationEventForTest());
v8::Isolate* firstIsolate = firstWorker->isolate();
ASSERT(firstIsolate);
- // Request termination of the first worker, and set-up to make sure the second worker is created right as
- // the first worker terminates its isolate.
- OwnPtr<WaitableEvent> secondCreationEvent = adoptPtr(new WaitableEvent());
- RefPtr<CompositorWorkerThread> secondWorker;
- firstWorker->setCallbackAfterV8Termination(bind(&CompositorWorkerThreadTest::createWorkerAdapter, this, &secondWorker, secondCreationEvent.get()));
- firstWorker->terminateAndWait();
+ // Request termination of the first worker and create the second worker
+ // as soon as possible.
+ EXPECT_EQ(1u, firstWorker->workerBackingThread().workerScriptCount());
+ firstWorker->terminate();
+ // We don't wait for its termination.
+ // Note: We rely on the assumption that the termination steps don't run
+ // on the worker thread so quickly. This could be a source of flakiness.
+
+ RefPtr<CompositorWorkerThread> secondWorker = createCompositorWorker();
ASSERT(secondWorker);
- waitForWaitableEventAfterIteratingCurrentLoop(secondCreationEvent.get());
v8::Isolate* secondIsolate = secondWorker->isolate();
ASSERT(secondIsolate);
EXPECT_EQ(firstIsolate, secondIsolate);

Powered by Google App Engine
This is Rietveld 408576698