test_shell_webblobregistry_impl.cc posts CString to other threads without

webkit/tools/test_shell/test_shell_webblobregistry_impl.cc

Index: webkit/tools/test_shell/test_shell_webblobregistry_impl.cc
diff --git a/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc b/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc
index 9c451cc3ff767b083a88b7d7e212461bf2e0b51b..e1e2ae711367f316f0e19b38bbdf4d63de1caca1 100644
--- a/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc
+++ b/webkit/tools/test_shell/test_shell_webblobregistry_impl.cc
@@ -5,8 +5,10 @@
 #include "webkit/tools/test_shell/test_shell_webblobregistry_impl.h"
 
 #include "base/message_loop.h"
+#include "base/task.h"
 #include "googleurl/src/gurl.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBlobData.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebCString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebURL.h"
 #include "webkit/blob/blob_data.h"
@@ -21,6 +23,15 @@ namespace {
 MessageLoop* g_io_thread;
 webkit_blob::BlobStorageController* g_blob_storage_controller;
 
+// Copy WebURL object to safely pass it to a different thread.

[jianli, 2011/04/19 23:58:00]

Maybe better to add more comment like the following:
// Note: NewRunnableMethod does copy WebURL object, but not its underlying
WebCString object since it is ref-counted. To pass safely to another thread, we
need to make a copy of WebCString data.

[Dmitry Lomov, 2011/04/20 00:27:01]

Done.

+static WebURL GetWebURLCopy(const WebURL& source) {

[jianli, 2011/04/19 23:58:00]

I think you can remove "static".

[Dmitry Lomov, 2011/04/20 00:27:01]

Done.

+  // Creates a copy of WebCString with refcount 1.

[jianli, 2011/04/19 23:58:00]

I think this comment can be removed with the additional comment above.

[Dmitry Lomov, 2011/04/20 00:27:01]

Done.

+  const WebKit::WebCString spec(source.spec());
+  const url_parse::Parsed& parsed(source.parsed());
+  const bool is_valid = source.isValid();
+  return WebURL(spec, parsed, is_valid);
+}
+
 }  // namespace
 
 /* static */
@@ -42,34 +53,46 @@ TestShellWebBlobRegistryImpl::TestShellWebBlobRegistryImpl() {
 void TestShellWebBlobRegistryImpl::registerBlobURL(
     const WebURL& url, WebBlobData& data) {
   DCHECK(g_io_thread);
-  // Note: BlobData is not refcounted thread safe.
-  scoped_refptr<webkit_blob::BlobData> blob_data(
+  CancelableTask* task;
+  {
+    scoped_refptr<webkit_blob::BlobData> blob_data(
       new webkit_blob::BlobData(data));
-  g_io_thread->PostTask(
-      FROM_HERE,
+    WebURL url_copy = ::GetWebURLCopy(url);
+    task =
       NewRunnableMethod(
-          this, &TestShellWebBlobRegistryImpl::DoRegisterBlobUrl, url,
-          blob_data));
+          this, &TestShellWebBlobRegistryImpl::DoRegisterBlobUrl, url_copy,
+          blob_data);
+  }
+  g_io_thread->PostTask(FROM_HERE, task);
 }
 
 void TestShellWebBlobRegistryImpl::registerBlobURL(
     const WebURL& url, const WebURL& src_url) {
   DCHECK(g_io_thread);
-  g_io_thread->PostTask(
-      FROM_HERE,
+  CancelableTask* task;
+  {
+    WebURL url_copy = ::GetWebURLCopy(url);
+    WebURL src_url_copy = ::GetWebURLCopy(src_url);
+    task =
       NewRunnableMethod(this,
                         &TestShellWebBlobRegistryImpl::DoRegisterBlobUrlFrom,
-                        url,
-                        src_url));
+                        url_copy,

[jianli, 2011/04/19 23:58:00]

You can put GetWebURLCopy(url) here directly. Doing so will also remove the need
of {}.

Also no need to add "::".

[Dmitry Lomov, 2011/04/20 00:27:01]

Done.

+                        src_url_copy);
+  }
+  g_io_thread->PostTask(FROM_HERE, task);
 }
 
 void TestShellWebBlobRegistryImpl::unregisterBlobURL(const WebURL& url) {
   DCHECK(g_io_thread);
-  g_io_thread->PostTask(
-      FROM_HERE,
+  CancelableTask* task;
+  {
+    WebURL url_copy = ::GetWebURLCopy(url);
+    task =
       NewRunnableMethod(this,
                         &TestShellWebBlobRegistryImpl::DoUnregisterBlobUrl,
-                        url));
+                        url_copy);
+  }
+  g_io_thread->PostTask(FROM_HERE, task);
 }
 
 void TestShellWebBlobRegistryImpl::DoRegisterBlobUrl(