[Sync] Add type conversion constructor for WeakHandle

This allows the use of e.g.:

WeakHandle<T> weak_handle = MakeWeakHandle(WeakPtr<U>());

as long as U is convertible to T, instead of:

WeakHandle<T> weak_handle = WeakHandle<T>(WeakPtr<U>());

Fix all callers.

BUG=
TEST=


Review URL: http://codereview.chromium.org/8568041

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

9 files changed, 90 insertions, 12 deletions

diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc
index 7f133ed..f4fe74f 100644
--- a/chrome/browser/sync/internal_api/sync_manager.cc
+++ b/chrome/browser/sync/internal_api/sync_manager.cc
@@ -826,7 +826,7 @@ bool SyncManager::SyncInternal::Init(
   // a freed pointer. This is because UI thread is not shut down.
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                     OnInitializationComplete(
-                        WeakHandle<JsBackend>(weak_ptr_factory_.GetWeakPtr()),
+                        MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()),
                         signed_in));
 
   if (!signed_in && !setup_for_test_mode_)
diff --git a/chrome/browser/sync/js/js_test_util.cc b/chrome/browser/sync/js/js_test_util.cc
index 94a02f5..542f53f 100644
--- a/chrome/browser/sync/js/js_test_util.cc
+++ b/chrome/browser/sync/js/js_test_util.cc
@@ -110,7 +110,7 @@ MockJsBackend::MockJsBackend() {}
 MockJsBackend::~MockJsBackend() {}
 
 WeakHandle<JsBackend> MockJsBackend::AsWeakHandle() {
-  return WeakHandle<JsBackend>(AsWeakPtr());
+  return MakeWeakHandle(AsWeakPtr());
 }
 
 MockJsController::MockJsController() {}
@@ -120,7 +120,7 @@ MockJsController::~MockJsController() {}
 MockJsEventHandler::MockJsEventHandler() {}
 
 WeakHandle<JsEventHandler> MockJsEventHandler::AsWeakHandle() {
-  return WeakHandle<JsEventHandler>(AsWeakPtr());
+  return MakeWeakHandle(AsWeakPtr());
 }
 
 MockJsEventHandler::~MockJsEventHandler() {}
@@ -130,7 +130,7 @@ MockJsReplyHandler::MockJsReplyHandler() {}
 MockJsReplyHandler::~MockJsReplyHandler() {}
 
 WeakHandle<JsReplyHandler> MockJsReplyHandler::AsWeakHandle() {
-  return WeakHandle<JsReplyHandler>(AsWeakPtr());
+  return MakeWeakHandle(AsWeakPtr());
 }
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/notifier/chrome_invalidation_client_unittest.cc b/chrome/browser/sync/notifier/chrome_invalidation_client_unittest.cc
index b581d6a..09a473e 100644
--- a/chrome/browser/sync/notifier/chrome_invalidation_client_unittest.cc
+++ b/chrome/browser/sync/notifier/chrome_invalidation_client_unittest.cc
@@ -75,7 +75,7 @@ class ChromeInvalidationClientTest : public testing::Test {
   virtual void SetUp() {
     client_.Start(kClientId, kClientInfo, kState,
                   InvalidationVersionMap(),
-                  browser_sync::WeakHandle<InvalidationVersionTracker>(
+                  browser_sync::MakeWeakHandle(
                       mock_invalidation_version_tracker_.AsWeakPtr()),
                   &mock_listener_, &mock_state_writer_,
                   fake_base_task_.AsWeakPtr());
diff --git a/chrome/browser/sync/notifier/non_blocking_invalidation_notifier_unittest.cc b/chrome/browser/sync/notifier/non_blocking_invalidation_notifier_unittest.cc
index 1d934ca..2dc621a 100644
--- a/chrome/browser/sync/notifier/non_blocking_invalidation_notifier_unittest.cc
+++ b/chrome/browser/sync/notifier/non_blocking_invalidation_notifier_unittest.cc
@@ -43,7 +43,7 @@ class NonBlockingInvalidationNotifierTest : public testing::Test {
         new NonBlockingInvalidationNotifier(
             notifier_options,
             InvalidationVersionMap(),
-            browser_sync::WeakHandle<InvalidationVersionTracker>(
+            browser_sync::MakeWeakHandle(
                 base::WeakPtr<sync_notifier::InvalidationVersionTracker>()),
             "fake_client_info"));
     invalidation_notifier_->AddObserver(&mock_observer_);
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 84af84f..3e9b99e 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -311,7 +311,7 @@ void ProfileSyncService::InitializeBackend(bool delete_stale_data) {
 
   backend_->Initialize(
       this,
-      WeakHandle<JsEventHandler>(sync_js_controller_.AsWeakPtr()),
+      MakeWeakHandle(sync_js_controller_.AsWeakPtr()),
       sync_service_url_,
       initial_types,
       credentials,
diff --git a/chrome/browser/sync/sync_js_controller.cc b/chrome/browser/sync/sync_js_controller.cc
index 3f45b4e1..e904c25 100644
--- a/chrome/browser/sync/sync_js_controller.cc
+++ b/chrome/browser/sync/sync_js_controller.cc
@@ -74,8 +74,7 @@ void SyncJsController::UpdateBackendEventHandler() {
     // handlers.
     WeakHandle<JsEventHandler> backend_event_handler =
         (js_event_handlers_.size() > 0) ?
-        WeakHandle<JsEventHandler>(AsWeakPtr()) :
-        WeakHandle<JsEventHandler>();
+        MakeWeakHandle(AsWeakPtr()) : WeakHandle<SyncJsController>();
     js_backend_.Call(FROM_HERE, &JsBackend::SetJsEventHandler,
                      backend_event_handler);
   }
diff --git a/chrome/browser/sync/util/weak_handle.h b/chrome/browser/sync/util/weak_handle.h
index 2ce3d80..3c92697 100644
--- a/chrome/browser/sync/util/weak_handle.h
+++ b/chrome/browser/sync/util/weak_handle.h
@@ -54,6 +54,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
@@ -280,6 +281,17 @@ class WeakHandle {
   explicit WeakHandle(const base::WeakPtr<T>& ptr)
       : core_(new internal::WeakHandleCore<T>(ptr)) {}
 
+  // Allow conversion from WeakHandle<U> to WeakHandle<T> if U is
+  // convertible to T, but we *must* be on |other|'s owner thread.
+  // Note that this doesn't override the regular copy constructor, so
+  // that one can be called on any thread.
+  template <typename U>
+  WeakHandle(const browser_sync::WeakHandle<U>& other)  // NOLINT
+      : core_(
+          other.IsInitialized() ?
+          new internal::WeakHandleCore<T>(other.Get()) :
+          NULL) {}
+
   // Returns true iff this WeakHandle is initialized.  Note that being
   // initialized isn't a guarantee that the underlying object is still
   // alive.
@@ -348,6 +360,11 @@ class WeakHandle {
   }
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(WeakHandleTest,
+                           TypeConversionConstructor);
+  FRIEND_TEST_ALL_PREFIXES(WeakHandleTest,
+                           TypeConversionConstructorAssignment);
+
   scoped_refptr<internal::WeakHandleCore<T> > core_;
 };
 
diff --git a/chrome/browser/sync/util/weak_handle_unittest.cc b/chrome/browser/sync/util/weak_handle_unittest.cc
index e23b71e..4a56f9e 100644
--- a/chrome/browser/sync/util/weak_handle_unittest.cc
+++ b/chrome/browser/sync/util/weak_handle_unittest.cc
@@ -14,7 +14,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace browser_sync {
-namespace {
 
 using ::testing::_;
 using ::testing::SaveArg;
@@ -44,6 +43,8 @@ class Base {
   base::WeakPtrFactory<Base> weak_ptr_factory_;
 };
 
+class Derived : public Base, public base::SupportsWeakPtr<Derived> {};
+
 class WeakHandleTest : public ::testing::Test {
  protected:
   virtual void TearDown() {
@@ -260,5 +261,66 @@ TEST_F(WeakHandleTest, InitializedAcrossCopyAssign) {
   PumpLoop();
 }
 
-}  // namespace
+TEST_F(WeakHandleTest, TypeConversionConstructor) {
+  StrictMock<Derived> d;
+  EXPECT_CALL(d, Test()).Times(2);
+
+  const WeakHandle<Derived> weak_handle = MakeWeakHandle(d.AsWeakPtr());
+
+  // Should trigger type conversion constructor.
+  const WeakHandle<Base> base_weak_handle(weak_handle);
+  // Should trigger regular copy constructor.
+  const WeakHandle<Derived> derived_weak_handle(weak_handle);
+
+  EXPECT_TRUE(base_weak_handle.IsInitialized());
+  base_weak_handle.Call(FROM_HERE, &Base::Test);
+
+  EXPECT_TRUE(derived_weak_handle.IsInitialized());
+  // Copy constructor shouldn't construct a new |core_|.
+  EXPECT_EQ(weak_handle.core_.get(), derived_weak_handle.core_.get());
+  derived_weak_handle.Call(FROM_HERE, &Base::Test);
+
+  PumpLoop();
+}
+
+TEST_F(WeakHandleTest, TypeConversionConstructorMakeWeakHandle) {
+  const base::WeakPtr<Derived> weak_ptr;
+
+  // Should trigger type conversion constructor after MakeWeakHandle.
+  WeakHandle<Base> base_weak_handle(MakeWeakHandle(weak_ptr));
+  // Should trigger regular copy constructor after MakeWeakHandle.
+  const WeakHandle<Derived> derived_weak_handle(MakeWeakHandle(weak_ptr));
+
+  EXPECT_TRUE(base_weak_handle.IsInitialized());
+  EXPECT_TRUE(derived_weak_handle.IsInitialized());
+}
+
+TEST_F(WeakHandleTest, TypeConversionConstructorAssignment) {
+  const WeakHandle<Derived> weak_handle =
+      MakeWeakHandle(Derived().AsWeakPtr());
+
+  // Should trigger type conversion constructor before the assignment.
+  WeakHandle<Base> base_weak_handle;
+  base_weak_handle = weak_handle;
+  // Should trigger regular copy constructor before the assignment.
+  WeakHandle<Derived> derived_weak_handle;
+  derived_weak_handle = weak_handle;
+
+  EXPECT_TRUE(base_weak_handle.IsInitialized());
+  EXPECT_TRUE(derived_weak_handle.IsInitialized());
+  // Copy constructor shouldn't construct a new |core_|.
+  EXPECT_EQ(weak_handle.core_.get(), derived_weak_handle.core_.get());
+}
+
+TEST_F(WeakHandleTest, TypeConversionConstructorUninitialized) {
+  const WeakHandle<Base> base_weak_handle = WeakHandle<Derived>();
+  EXPECT_FALSE(base_weak_handle.IsInitialized());
+}
+
+TEST_F(WeakHandleTest, TypeConversionConstructorUninitializedAssignment) {
+  WeakHandle<Base> base_weak_handle;
+  base_weak_handle = WeakHandle<Derived>();
+  EXPECT_FALSE(base_weak_handle.IsInitialized());
+}
+
 }  // namespace browser_sync
diff --git a/chrome/browser/ui/webui/sync_internals_ui.cc b/chrome/browser/ui/webui/sync_internals_ui.cc
index bcfd89b..8b03d28 100644
--- a/chrome/browser/ui/webui/sync_internals_ui.cc
+++ b/chrome/browser/ui/webui/sync_internals_ui.cc
@@ -113,7 +113,7 @@ void SyncInternalsUI::OnWebUISend(const GURL& source_url,
     if (js_controller_.get()) {
       js_controller_->ProcessJsMessage(
           name, args,
-          WeakHandle<JsReplyHandler>(weak_ptr_factory_.GetWeakPtr()));
+          MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()));
     } else {
       LOG(WARNING) << "No sync service; dropping message " << name
                    << " with args " << args.ToString();