Add missing Pass() calls for passing scoped_ptrs on Windows.

Add missing Pass() calls for passing scoped_ptrs on Windows.

BUG=82385
TBR=awong@chromium.org
R=joaodasilva@chromium.org,sergeyu@chromium.org
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281366

[Nico, 2014-07-03 22:28:31]

That's how this is supposed to work, right?

(Look for MOVE_ONLY_TYPE_FOR_CPP_03 in
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20Clang/buil...
. A few of those complain about lines that do "return local_var.Pass();", not
sure why yet)

[Nico, 2014-07-03 22:37:48]

Ah, these seem to all need PassAs<>() calls. Updating patch...


On Thu, Jul 3, 2014 at 3:28 PM, <thakis@chromium.org> wrote:

> Reviewers: awong,
>
> Message:
> That's how this is supposed to work, right?
>
> (Look for MOVE_ONLY_TYPE_FOR_CPP_03 in
> http://build.chromium.org/p/chromium.fyi/builders/
> Chromium%20Win%20Clang/builds/2288/steps/compile/logs/stdio
> . A few of those complain about lines that do "return local_var.Pass();",
> not
> sure why yet)
>
> Description:
> Add missing Pass() calls for passing scoped_ptrs on Windows.
>
> BUG=82385
>
> Please review this at https://codereview.chromium.org/367403002/
>
> SVN Base: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+49, -42 lines):
>   M chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc
>   M chrome/installer/util/chrome_app_host_distribution.cc
>   M chrome/installer/util/chrome_frame_distribution.cc
>   M chrome/installer/util/chromium_binaries_distribution.cc
>   M chrome/installer/util/google_chrome_binaries_distribution.cc
>   M chrome/installer/util/google_chrome_distribution.cc
>   M chrome/installer/util/google_chrome_distribution_dummy.cc
>   M chrome/installer/util/google_chrome_sxs_distribution.cc
>   M components/policy/core/common/registry_dict_win_unittest.cc
>
>
> Index: chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc
> diff --git a/chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc
> b/chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc
> index d302690acfaf55f94ad90a876e83d9b623a7d838..
> dc99fd2f590409f0b82ab8b2961a17a575de0776 100644
> --- a/chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc
> +++ b/chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc
> @@ -135,7 +135,7 @@ TEST(StatusTrayWinTest, EnsureVisibleTest) {
>
>    FakeStatusTrayStateChangerProxy* proxy =
>        new FakeStatusTrayStateChangerProxy();
> -  tray.SetStatusTrayStateChangerProxyForTest(make_scoped_ptr(proxy));
> +  tray.SetStatusTrayStateChangerProxyForTest(make_scoped_ptr(proxy)
> .Pass());
>
>    StatusIconWin* icon = static_cast<StatusIconWin*>(
> tray.CreateStatusIcon(
>        StatusTray::OTHER_ICON, *image, base::ASCIIToUTF16("tool tip")));
> Index: chrome/installer/util/chrome_app_host_distribution.cc
> diff --git a/chrome/installer/util/chrome_app_host_distribution.cc
> b/chrome/installer/util/chrome_app_host_distribution.cc
> index d69d84dc1310f9a1cf5a62fa50cfed3f75a6513b..
> 20c072db8fc76ee9a89cba177beeb81d8ea93ef5 100644
> --- a/chrome/installer/util/chrome_app_host_distribution.cc
> +++ b/chrome/installer/util/chrome_app_host_distribution.cc
> @@ -29,8 +29,8 @@ const wchar_t kChromeAppHostGuid[] =
> L"{FDA71E6F-AC4C-4a00-8B70-9958A68906BF}";
>
>  ChromeAppHostDistribution::ChromeAppHostDistribution()
>      : BrowserDistribution(CHROME_APP_HOST,
> -          make_scoped_ptr(
> -              new UpdatingAppRegistrationData(kChromeAppHostGuid))) {
> +                          make_scoped_ptr(new UpdatingAppRegistrationData(
> +
>  kChromeAppHostGuid)).Pass()) {
>  }
>
>  base::string16 ChromeAppHostDistribution::GetBaseAppName() {
> Index: chrome/installer/util/chrome_frame_distribution.cc
> diff --git a/chrome/installer/util/chrome_frame_distribution.cc
> b/chrome/installer/util/chrome_frame_distribution.cc
> index 385ba2f986f2eddd388f0e6de8d31c9b094e3cc8..
> 67dc86cd40a71246742cd1a28809115b32e3af0d 100644
> --- a/chrome/installer/util/chrome_frame_distribution.cc
> +++ b/chrome/installer/util/chrome_frame_distribution.cc
> @@ -27,7 +27,8 @@ const wchar_t kChromeFrameGuid[] =
> L"{8BA986DA-5100-405E-AA35-86F34A02ACBF}";
>
>  ChromeFrameDistribution::ChromeFrameDistribution()
>      : BrowserDistribution(CHROME_FRAME,
> -          make_scoped_ptr(new
UpdatingAppRegistrationData(kChromeFrameGuid)))
> {
> +                          make_scoped_ptr(new UpdatingAppRegistrationData(
> +                                              kChromeFrameGuid)).Pass()) {
>  }
>
>  base::string16 ChromeFrameDistribution::GetBaseAppName() {
> Index: chrome/installer/util/chromium_binaries_distribution.cc
> diff --git a/chrome/installer/util/chromium_binaries_distribution.cc
> b/chrome/installer/util/chromium_binaries_distribution.cc
> index 2783c51630228d36239bb255c474db3d07af5fbe..
> 0a61d40a4611ffff3050b72d13d064d64a8db63a 100644
> --- a/chrome/installer/util/chromium_binaries_distribution.cc
> +++ b/chrome/installer/util/chromium_binaries_distribution.cc
> @@ -17,9 +17,11 @@ const wchar_t kChromiumBinariesName[] = L"Chromium
> Binaries";
>  }  // namespace
>
>  ChromiumBinariesDistribution::ChromiumBinariesDistribution()
> -    : BrowserDistribution(CHROME_BINARIES,
> +    : BrowserDistribution(
> +          CHROME_BINARIES,
>            make_scoped_ptr(new NonUpdatingAppRegistrationData(
> -              base::string16(L"Software\\").
> append(kChromiumBinariesName)))),
> +                              base::string16(L"Software\\")
> +                                  .append(kChromiumBinariesName)
> )).Pass()),
>        browser_distribution_(
>            BrowserDistribution::GetSpecificDistribution(CHROME_BROWSER)) {
>  }
> Index: chrome/installer/util/google_chrome_binaries_distribution.cc
> diff --git a/chrome/installer/util/google_chrome_binaries_distribution.cc
> b/chrome/installer/util/google_chrome_binaries_distribution.cc
> index bdf1bf6f092e7fe8fe39014be89753f45ae2978d..
> b2a33fa4f4b8b30558a7479b34b9219659f3081a 100644
> --- a/chrome/installer/util/google_chrome_binaries_distribution.cc
> +++ b/chrome/installer/util/google_chrome_binaries_distribution.cc
> @@ -20,8 +20,8 @@ const wchar_t kChromeBinariesName[] = L"Google Chrome
> binaries";
>
>  GoogleChromeBinariesDistribution::GoogleChromeBinariesDistribution()
>      : ChromiumBinariesDistribution(
> -          make_scoped_ptr(
> -              new UpdatingAppRegistrationData(kChromeBinariesGuid))) {
> +          make_scoped_ptr(new UpdatingAppRegistrationData(
> kChromeBinariesGuid))
> +              .Pass()) {
>  }
>
>  base::string16 GoogleChromeBinariesDistribution::GetDisplayName() {
> Index: chrome/installer/util/google_chrome_distribution.cc
> diff --git a/chrome/installer/util/google_chrome_distribution.cc
> b/chrome/installer/util/google_chrome_distribution.cc
> index 3d1522d6a17b0310f42a21dd9c98c2696c34e8ed..
> 7030003a5c930299c505e6af9046a41a50b453f5 100644
> --- a/chrome/installer/util/google_chrome_distribution.cc
> +++ b/chrome/installer/util/google_chrome_distribution.cc
> @@ -63,8 +63,8 @@ base::string16 GetUninstallSurveyUrl() {
>
>  GoogleChromeDistribution::GoogleChromeDistribution()
>      : BrowserDistribution(CHROME_BROWSER,
> -          make_scoped_ptr(
> -              new UpdatingAppRegistrationData(kChromeGuid))) {
> +                          make_scoped_ptr(new UpdatingAppRegistrationData(
> +                                              kChromeGuid)).Pass()) {
>  }
>
>  GoogleChromeDistribution::GoogleChromeDistribution(
> Index: chrome/installer/util/google_chrome_distribution_dummy.cc
> diff --git a/chrome/installer/util/google_chrome_distribution_dummy.cc
> b/chrome/installer/util/google_chrome_distribution_dummy.cc
> index 012b72d6661d851439b4d562a94a212bc8aa28e2..
> 0c3c648cbc00e27b915d43a53df73c6504ab2a5d 100644
> --- a/chrome/installer/util/google_chrome_distribution_dummy.cc
> +++ b/chrome/installer/util/google_chrome_distribution_dummy.cc
> @@ -20,8 +20,8 @@
>
>  GoogleChromeDistribution::GoogleChromeDistribution()
>      : BrowserDistribution(CHROME_BROWSER,
> -          make_scoped_ptr(
> -              new NonUpdatingAppRegistrationData(base::string16()))) {
> +                          make_scoped_ptr(new
> NonUpdatingAppRegistrationData(
> +                                              base::string16())).Pass()) {
>  }
>
>  GoogleChromeDistribution::GoogleChromeDistribution(
> Index: chrome/installer/util/google_chrome_sxs_distribution.cc
> diff --git a/chrome/installer/util/google_chrome_sxs_distribution.cc
> b/chrome/installer/util/google_chrome_sxs_distribution.cc
> index 570766071219843fca46db5a7c45fdd1f1bdbb57..
> b3b7ad852093b562d3a00742dc2d92e24c78c0f3 100644
> --- a/chrome/installer/util/google_chrome_sxs_distribution.cc
> +++ b/chrome/installer/util/google_chrome_sxs_distribution.cc
> @@ -26,9 +26,8 @@ const wchar_t kCommandExecuteImplUuid[] =
>  }  // namespace
>
>  GoogleChromeSxSDistribution::GoogleChromeSxSDistribution()
> -    : GoogleChromeDistribution(
> -          make_scoped_ptr(
> -              new UpdatingAppRegistrationData(kChromeSxSGuid))) {
> +    : GoogleChromeDistribution(make_scoped_ptr(new
> UpdatingAppRegistrationData(
> +
> kChromeSxSGuid)).Pass()) {
>  }
>
>  base::string16 GoogleChromeSxSDistribution::GetBaseAppName() {
> Index: components/policy/core/common/registry_dict_win_unittest.cc
> diff --git a/components/policy/core/common/registry_dict_win_unittest.cc
> b/components/policy/core/common/registry_dict_win_unittest.cc
> index 45df76eacacf519504084441f6e6a3d1ccf94a9e..
> 9c6af6ce39833e0eefe8fa788bfcbffccf57852a 100644
> --- a/components/policy/core/common/registry_dict_win_unittest.cc
> +++ b/components/policy/core/common/registry_dict_win_unittest.cc
> @@ -19,12 +19,12 @@ TEST(RegistryDictTest, SetAndGetValue) {
>    base::FundamentalValue int_value(42);
>    base::StringValue string_value("fortytwo");
>
> -  test_dict.SetValue("one", make_scoped_ptr(int_value.DeepCopy()));
> +  test_dict.SetValue("one", make_scoped_ptr(int_value.
> DeepCopy()).Pass());
>    EXPECT_EQ(1, test_dict.values().size());
>    EXPECT_TRUE(base::Value::Equals(&int_value,
> test_dict.GetValue("one")));
>    EXPECT_FALSE(test_dict.GetValue("two"));
>
> -  test_dict.SetValue("two", make_scoped_ptr(string_value.DeepCopy()));
> +  test_dict.SetValue("two", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    EXPECT_EQ(2, test_dict.values().size());
>    EXPECT_TRUE(base::Value::Equals(&int_value,
> test_dict.GetValue("one")));
>    EXPECT_TRUE(base::Value::Equals(&string_value,
> test_dict.GetValue("two")));
> @@ -47,7 +47,7 @@ TEST(RegistryDictTest,
CaseInsensitiveButPreservingValueNames)
> {
>    base::FundamentalValue int_value(42);
>    base::StringValue string_value("fortytwo");
>
> -  test_dict.SetValue("One", make_scoped_ptr(int_value.DeepCopy()));
> +  test_dict.SetValue("One", make_scoped_ptr(int_value.
> DeepCopy()).Pass());
>    EXPECT_EQ(1, test_dict.values().size());
>    EXPECT_TRUE(base::Value::Equals(&int_value,
> test_dict.GetValue("oNe")));
>
> @@ -55,7 +55,7 @@ TEST(RegistryDictTest,
CaseInsensitiveButPreservingValueNames)
> {
>    ASSERT_NE(entry, test_dict.values().end());
>    EXPECT_EQ("One", entry->first);
>
> -  test_dict.SetValue("ONE", make_scoped_ptr(string_value.DeepCopy()));
> +  test_dict.SetValue("ONE", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    EXPECT_EQ(1, test_dict.values().size());
>    EXPECT_TRUE(base::Value::Equals(&string_value,
> test_dict.GetValue("one")));
>
> @@ -71,7 +71,7 @@ TEST(RegistryDictTest, SetAndGetKeys) {
>    base::StringValue string_value("fortytwo");
>
>    scoped_ptr<RegistryDict> subdict(new RegistryDict());
> -  subdict->SetValue("one", make_scoped_ptr(int_value.DeepCopy()));
> +  subdict->SetValue("one", make_scoped_ptr(int_value.DeepCopy()).Pass());
>    test_dict.SetKey("two", subdict.Pass());
>    EXPECT_EQ(1, test_dict.keys().size());
>    RegistryDict* actual_subdict = test_dict.GetKey("two");
> @@ -79,7 +79,7 @@ TEST(RegistryDictTest, SetAndGetKeys) {
>    EXPECT_TRUE(base::Value::Equals(&int_value,
> actual_subdict->GetValue("one")));
>
>    subdict.reset(new RegistryDict());
> -  subdict->SetValue("three", make_scoped_ptr(string_value.DeepCopy()));
> +  subdict->SetValue("three", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    test_dict.SetKey("four", subdict.Pass());
>    EXPECT_EQ(2, test_dict.keys().size());
>    actual_subdict = test_dict.GetKey("two");
> @@ -101,7 +101,7 @@ TEST(RegistryDictTest,
CaseInsensitiveButPreservingKeyNames)
> {
>
>    base::FundamentalValue int_value(42);
>
> -  test_dict.SetKey("One", make_scoped_ptr(new RegistryDict()));
> +  test_dict.SetKey("One", make_scoped_ptr(new RegistryDict()).Pass());
>    EXPECT_EQ(1, test_dict.keys().size());
>    RegistryDict* actual_subdict = test_dict.GetKey("One");
>    ASSERT_TRUE(actual_subdict);
> @@ -112,7 +112,7 @@ TEST(RegistryDictTest,
CaseInsensitiveButPreservingKeyNames)
> {
>    EXPECT_EQ("One", entry->first);
>
>    scoped_ptr<RegistryDict> subdict(new RegistryDict());
> -  subdict->SetValue("two", make_scoped_ptr(int_value.DeepCopy()));
> +  subdict->SetValue("two", make_scoped_ptr(int_value.DeepCopy()).Pass());
>    test_dict.SetKey("ONE", subdict.Pass());
>    EXPECT_EQ(1, test_dict.keys().size());
>    actual_subdict = test_dict.GetKey("One");
> @@ -134,17 +134,17 @@ TEST(RegistryDictTest, Merge) {
>    base::FundamentalValue int_value(42);
>    base::StringValue string_value("fortytwo");
>
> -  dict_a.SetValue("one", make_scoped_ptr(int_value.DeepCopy()));
> +  dict_a.SetValue("one", make_scoped_ptr(int_value.DeepCopy()).Pass());
>    scoped_ptr<RegistryDict> subdict(new RegistryDict());
> -  subdict->SetValue("two", make_scoped_ptr(string_value.DeepCopy()));
> +  subdict->SetValue("two", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    dict_a.SetKey("three", subdict.Pass());
>
> -  dict_b.SetValue("four", make_scoped_ptr(string_value.DeepCopy()));
> +  dict_b.SetValue("four", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    subdict.reset(new RegistryDict());
> -  subdict->SetValue("two", make_scoped_ptr(int_value.DeepCopy()));
> +  subdict->SetValue("two", make_scoped_ptr(int_value.DeepCopy()).Pass());
>    dict_b.SetKey("three", subdict.Pass());
>    subdict.reset(new RegistryDict());
> -  subdict->SetValue("five", make_scoped_ptr(int_value.DeepCopy()));
> +  subdict->SetValue("five", make_scoped_ptr(int_value.
> DeepCopy()).Pass());
>    dict_b.SetKey("six", subdict.Pass());
>
>    dict_a.Merge(dict_b);
> @@ -167,9 +167,9 @@ TEST(RegistryDictTest, Swap) {
>    base::FundamentalValue int_value(42);
>    base::StringValue string_value("fortytwo");
>
> -  dict_a.SetValue("one", make_scoped_ptr(int_value.DeepCopy()));
> -  dict_a.SetKey("two", make_scoped_ptr(new RegistryDict()));
> -  dict_b.SetValue("three", make_scoped_ptr(string_value.DeepCopy()));
> +  dict_a.SetValue("one", make_scoped_ptr(int_value.DeepCopy()).Pass());
> +  dict_a.SetKey("two", make_scoped_ptr(new RegistryDict()).Pass());
> +  dict_b.SetValue("three", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>
>    dict_a.Swap(&dict_b);
>
> @@ -190,20 +190,25 @@ TEST(RegistryDictTest, ConvertToJSON) {
>    base::StringValue string_zero("0");
>    base::StringValue string_dict("{ \"key\": [ \"value\" ] }");
>
> -  test_dict.SetValue("one", make_scoped_ptr(int_value.DeepCopy()));
> +  test_dict.SetValue("one", make_scoped_ptr(int_value.
> DeepCopy()).Pass());
>    scoped_ptr<RegistryDict> subdict(new RegistryDict());
> -  subdict->SetValue("two", make_scoped_ptr(string_value.DeepCopy()));
> +  subdict->SetValue("two", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    test_dict.SetKey("three", subdict.Pass());
>    scoped_ptr<RegistryDict> list(new RegistryDict());
> -  list->SetValue("1", make_scoped_ptr(string_value.DeepCopy()));
> +  list->SetValue("1", make_scoped_ptr(string_value.DeepCopy()).Pass());
>    test_dict.SetKey("dict-to-list", list.Pass());
> -  test_dict.SetValue("int-to-bool", make_scoped_ptr(int_value.
> DeepCopy()));
> -  test_dict.SetValue("int-to-double", make_scoped_ptr(int_value.
> DeepCopy()));
> -  test_dict.SetValue("string-to-bool", make_scoped_ptr(string_zero.
> DeepCopy()));
> +  test_dict.SetValue("int-to-bool",
> +                     make_scoped_ptr(int_value.DeepCopy()).Pass());
> +  test_dict.SetValue("int-to-double",
> +                     make_scoped_ptr(int_value.DeepCopy()).Pass());
> +  test_dict.SetValue("string-to-bool",
> +                     make_scoped_ptr(string_zero.DeepCopy()).Pass());
>    test_dict.SetValue("string-to-double",
> -                     make_scoped_ptr(string_zero.DeepCopy()));
> -  test_dict.SetValue("string-to-int", make_scoped_ptr(string_zero.
> DeepCopy()));
> -  test_dict.SetValue("string-to-dict", make_scoped_ptr(string_dict.
> DeepCopy()));
> +                     make_scoped_ptr(string_zero.DeepCopy()).Pass());
> +  test_dict.SetValue("string-to-int",
> +                     make_scoped_ptr(string_zero.DeepCopy()).Pass());
> +  test_dict.SetValue("string-to-dict",
> +                     make_scoped_ptr(string_dict.DeepCopy()).Pass());
>
>    std::string error;
>    Schema schema = Schema::Parse(
> @@ -255,9 +260,9 @@ TEST(RegistryDictTest, KeyValueNameClashes) {
>    base::FundamentalValue int_value(42);
>    base::StringValue string_value("fortytwo");
>
> -  test_dict.SetValue("one", make_scoped_ptr(int_value.DeepCopy()));
> +  test_dict.SetValue("one", make_scoped_ptr(int_value.
> DeepCopy()).Pass());
>    scoped_ptr<RegistryDict> subdict(new RegistryDict());
> -  subdict->SetValue("two", make_scoped_ptr(string_value.DeepCopy()));
> +  subdict->SetValue("two", make_scoped_ptr(string_value.
> DeepCopy()).Pass());
>    test_dict.SetKey("one", subdict.Pass());
>
>    EXPECT_TRUE(base::Value::Equals(&int_value,
> test_dict.GetValue("one")));
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-07-03 22:50:10]

Ok, added PassAs<> calls where appropriate.

joao: components/policy
sergeyu: remoting
awong: rest

[Sergey Ulanov, 2014-07-04 01:02:30]

lgtm

[grt (UTC plus 2), 2014-07-04 01:32:28]

https://codereview.chromium.org/367403002/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc (right):

https://codereview.chromium.org/367403002/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc:138:
tray.SetStatusTrayStateChangerProxyForTest(make_scoped_ptr(proxy).Pass());
since SetStatusTrayStateChangerProxyForTest takes a
scoped_ptr<StatusTrayStateChangerProxy>, doesn't this need to be either:
  make_scoped_ptr(proxy).PassAs<StatusTrayStateChangerProxy>()
or:
  scoped_ptr<StatusTrayStateChangerProxy>(proxy)?
for what it's worth, i prefer the latter since it's shorter.

ah, i now see that you've done the same thing elsewhere, so i suppose that what
you have works. i find make_scoped_ptr(instanceOfDerivedType).Pass() odd, so i
would prefer that you replace it with
scoped_ptr<RealType>(instanceOfDerivedType) here and elsewhere since it's more
explicit and less magical.

note that i have not consulted the rest of the codebase to see if what you have
here is actually more idomatic than what i propose. if that's the case, then
this will be my "today i learned...".

cheers

[Nico, 2014-07-04 02:20:12]

https://codereview.chromium.org/367403002/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc (right):

https://codereview.chromium.org/367403002/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc:138:
tray.SetStatusTrayStateChangerProxyForTest(make_scoped_ptr(proxy).Pass());
On 2014/07/04 01:32:28, grt wrote:
> since SetStatusTrayStateChangerProxyForTest takes a
> scoped_ptr<StatusTrayStateChangerProxy>, doesn't this need to be either:
>   make_scoped_ptr(proxy).PassAs<StatusTrayStateChangerProxy>()
> or:
>   scoped_ptr<StatusTrayStateChangerProxy>(proxy)?
> for what it's worth, i prefer the latter since it's shorter.

Yes, you're right. Done.

(Actually, I made proxy a scoped_ptr<StatusTrayStateChangerProxy> and then Pass
that, as I think the idea is that there should rarely be raw pointers that are
strong.)

> ah, i now see that you've done the same thing elsewhere,

This CL was written without a local clang build, so it's possible I didn't do
everything right. I went through it again and tried to find all the other places
where I got this wrong, thanks. (Mostly using your preferred approach.)

I left the places where it says make_scoped_ptr(foo).PassAs<subtype>()
correctly. I'm not sure which form is preferred; maybe Albert has an opinion.
The PassAs docs only talk about local variables, not about make_scoped_ptr
calls.

> so i suppose that what
> you have works. i find make_scoped_ptr(instanceOfDerivedType).Pass() odd, so i
> would prefer that you replace it with
> scoped_ptr<RealType>(instanceOfDerivedType) here and elsewhere since it's more
> explicit and less magical.
> 
> note that i have not consulted the rest of the codebase to see if what you
have
> here is actually more idomatic than what i propose. if that's the case, then
> this will be my "today i learned...".
> 
> cheers

[Nico, 2014-07-04 04:20:52]

https://codereview.chromium.org/367403002/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc (right):

https://codereview.chromium.org/367403002/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/status_icons/status_tray_win_unittest.cc:138:
tray.SetStatusTrayStateChangerProxyForTest(make_scoped_ptr(proxy).Pass());
On 2014/07/04 02:20:12, Nico (away) wrote:
> On 2014/07/04 01:32:28, grt wrote:
> > since SetStatusTrayStateChangerProxyForTest takes a
> > scoped_ptr<StatusTrayStateChangerProxy>, doesn't this need to be either:
> >   make_scoped_ptr(proxy).PassAs<StatusTrayStateChangerProxy>()
> > or:
> >   scoped_ptr<StatusTrayStateChangerProxy>(proxy)?
> > for what it's worth, i prefer the latter since it's shorter.
> 
> Yes, you're right. Done.
> 
> (Actually, I made proxy a scoped_ptr<StatusTrayStateChangerProxy> and then
Pass
> that, as I think the idea is that there should rarely be raw pointers that are
> strong.)

…except that that doesn't build, since this test accesses proxy after it's been
passed. That seems…unfortunate. I added a comment that explains why it's fine,
and went with the method you proposed here.

[Joao da Silva, 2014-07-04 06:37:52]

policy lgtm

[Nico, 2014-07-04 06:40:17]

awong: tbr'ing. If you have comments, I'll address them in a follow-up.

[Nico, 2014-07-04 06:40:56]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2014-07-04 06:41:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thakis@chromium.org/367403002/90001

[commit-bot: I haz the power, 2014-07-04 06:42:14]

Change committed as 281366