Call WebWorkerClient on the main thread. This makes it consistent with the rest of the WebKit API, which is single threaded. Also a bunch of small fixes to make layout tests pass: the dll was being unloaded while its functions were still queued to be dispatched, and a string allocated in the dll was being GC'd in test shell.

BUG=11011
Review URL: http://codereview.chromium.org/102005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@15087 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 35 insertions, 85 deletions

diff --git a/webkit/tools/test_shell/test_worker/test_webworker.cc b/webkit/tools/test_shell/test_worker/test_webworker.cc
index 6b073b5..29733b4 100644
--- a/webkit/tools/test_shell/test_worker/test_webworker.cc
+++ b/webkit/tools/test_shell/test_worker/test_webworker.cc
@@ -19,21 +19,6 @@ using WebKit::WebURL;
 using WebKit::WebWorker;
 using WebKit::WebWorkerClient;
 
-// We use this class to pass a WebString between threads.  This works by
-// copying to a string16 which is itself safe to pass between threads.
-// TODO(jam): Remove this once all worker callbacks happen on the main thread.
-class ThreadSafeWebString {
- public:
-  explicit ThreadSafeWebString(const WebString& str)
-      : str_(str) {
-  }
-  operator WebString() const {
-    return str_;
-  }
- private:
-  string16 str_;
-};
-
 TestWebWorker::TestWebWorker(WebWorkerClient* client,
                              TestWebWorkerHelper* webworker_helper)
     : webworkerclient_delegate_(client),
@@ -81,32 +66,22 @@ void TestWebWorker::workerObjectDestroyed() {
 }
 
 void TestWebWorker::postMessageToWorkerObject(const WebString& message) {
-  if (webworker_helper_->IsMainThread()) {
-    if (webworkerclient_delegate_)
-      webworkerclient_delegate_->postMessageToWorkerObject(message);
-  } else {
-    webworker_helper_->DispatchToMainThread(
-        InvokeMainThreadMethod, NewRunnableMethod(
-            this, &TestWebWorker::postMessageToWorkerObject,
-            ThreadSafeWebString(message)));
-  }
+  if (!webworkerclient_delegate_)
+    return;
+  // The string was created in the dll's memory space as a result of a postTask.
+  // If we pass it to test shell's memory space, it'll cause problems when GC
+  // occurs.  So duplicate it from the test shell's memory space first.
+  webworkerclient_delegate_->postMessageToWorkerObject(
+      webworker_helper_->DuplicateString(message));
 }
 
 void TestWebWorker::postExceptionToWorkerObject(const WebString& error_message,
                                                 int line_number,
                                                 const WebString& source_url) {
-  if (webworker_helper_->IsMainThread()) {
-    if (webworkerclient_delegate_)
-      webworkerclient_delegate_->postExceptionToWorkerObject(error_message,
-                                                             line_number,
-                                                             source_url);
-  } else {
-    webworker_helper_->DispatchToMainThread(
-        InvokeMainThreadMethod, NewRunnableMethod(
-            this, &TestWebWorker::postExceptionToWorkerObject,
-            ThreadSafeWebString(error_message), line_number,
-            ThreadSafeWebString(source_url)));
-  }
+  if (webworkerclient_delegate_)
+    webworkerclient_delegate_->postExceptionToWorkerObject(error_message,
+                                                           line_number,
+                                                           source_url);
 }
 
 void TestWebWorker::postConsoleMessageToWorkerObject(
@@ -116,63 +91,28 @@ void TestWebWorker::postConsoleMessageToWorkerObject(
     const WebString& message,
     int line_number,
     const WebString& source_url) {
-  if (webworker_helper_->IsMainThread()) {
-    if (webworkerclient_delegate_)
-      webworkerclient_delegate_->postConsoleMessageToWorkerObject(
-          destination_id, source_id, message_level, message, line_number,
-          source_url);
-  } else {
-    webworker_helper_->DispatchToMainThread(
-        InvokeMainThreadMethod, NewRunnableMethod(
-            this, &TestWebWorker::postConsoleMessageToWorkerObject,
-            destination_id, source_id, message_level,
-            ThreadSafeWebString(message), line_number,
-            ThreadSafeWebString(source_url)));
-  }
+  if (webworkerclient_delegate_)
+    webworkerclient_delegate_->postConsoleMessageToWorkerObject(
+        destination_id, source_id, message_level, message, line_number,
+        source_url);
 }
 
 void TestWebWorker::confirmMessageFromWorkerObject(bool has_pending_activity) {
-  if (webworker_helper_->IsMainThread()) {
-    if (webworkerclient_delegate_)
-      webworkerclient_delegate_->confirmMessageFromWorkerObject(
-          has_pending_activity);
-  } else {
-    webworker_helper_->DispatchToMainThread(
-        InvokeMainThreadMethod, NewRunnableMethod(
-            this, &TestWebWorker::confirmMessageFromWorkerObject,
-            has_pending_activity));
-  }
+  if (webworkerclient_delegate_)
+    webworkerclient_delegate_->confirmMessageFromWorkerObject(
+        has_pending_activity);
 }
 
 void TestWebWorker::reportPendingActivity(bool has_pending_activity) {
-  if (webworker_helper_->IsMainThread()) {
-    if (webworkerclient_delegate_)
-      webworkerclient_delegate_->reportPendingActivity(has_pending_activity);
-  } else {
-    webworker_helper_->DispatchToMainThread(
-        InvokeMainThreadMethod, NewRunnableMethod(
-            this, &TestWebWorker::reportPendingActivity,
-            has_pending_activity));
-  }
+  if (webworkerclient_delegate_)
+    webworkerclient_delegate_->reportPendingActivity(has_pending_activity);
 }
 
 void TestWebWorker::workerContextDestroyed() {
-  if (webworker_helper_->IsMainThread()) {
-    if (webworkerclient_delegate_)
-      webworkerclient_delegate_->workerContextDestroyed();
-    Release();    // Releases the reference held for worker context object.
-  } else {
-    webworker_impl_ = NULL;
-    webworker_helper_->DispatchToMainThread(
-        InvokeMainThreadMethod, NewRunnableMethod(
-            this, &TestWebWorker::workerContextDestroyed));
-  }
-}
-
-void TestWebWorker::InvokeMainThreadMethod(void* param) {
-  Task* task = static_cast<Task*>(param);
-  task->Run();
-  delete task;
+  webworker_impl_ = NULL;
+  if (webworkerclient_delegate_)
+    webworkerclient_delegate_->workerContextDestroyed();
+  Release();    // Releases the reference held for worker context object.
 }
 
 #endif
diff --git a/webkit/tools/test_shell/test_worker/test_worker_main.cc b/webkit/tools/test_shell/test_worker/test_worker_main.cc
index 6695a11..9273d35 100644
--- a/webkit/tools/test_shell/test_worker/test_worker_main.cc
+++ b/webkit/tools/test_shell/test_worker/test_worker_main.cc
@@ -29,6 +29,9 @@ static base::AtExitManager global_at_exit_manager;
 // Stub WebKit Client.
 class WorkerWebKitClientImpl : public webkit_glue::WebKitClientImpl {
  public:
+  explicit WorkerWebKitClientImpl(TestWebWorkerHelper* helper)
+      : helper_(helper) { }
+
   // WebKitClient methods:
   virtual WebKit::WebClipboard* clipboard() {
     NOTREACHED();
@@ -76,6 +79,13 @@ class WorkerWebKitClientImpl : public webkit_glue::WebKitClientImpl {
     NOTREACHED();
     return WebKit::WebString();
   }
+
+  virtual void callOnMainThread(void (*func)()) {
+    helper_->DispatchToMainThread(func);
+  }
+
+ private:
+  TestWebWorkerHelper* helper_;
 };
 
 // WebKit client used in DLL.
@@ -89,7 +99,7 @@ extern "C" {
 WebWorker* API_CALL CreateWebWorker(WebWorkerClient* webworker_client,
                                     TestWebWorkerHelper* webworker_helper) {
   if (!WebKit::webKitClient()) {
-    webkit_client.reset(new WorkerWebKitClientImpl());
+    webkit_client.reset(new WorkerWebKitClientImpl(webworker_helper));
     WebKit::initialize(webkit_client.get());
   }