PSA: scoped_refptr<> is now movable.

[Kibeom Kim]

tl;dr we made scoped_refptr<> movable http://crrev.com/324910, which is analogous to scoped_ptr<>::Pass().

Benefits:

• Clear expression of intent, i.e., “the assigning variable is not used anymore”.
  
• Reduce unnecessary reference counter updates. Micro benchmark
  

Example:

{

  scoped_refptr<Foo> local_ptr;

  ...

  member_ptr_ = local_ptr;

  ...

}

This would call a copy assignment operator, and |local_ptr|’s destructor at the end of the scope, that increments and decrements the reference counter. Now we can do this instead.

member_ptr_ = local_ptr.Pass();

Then a move assignment operator is called: |member_ptr_| will have |local_ptr|’s pointer and |local_ptr| will be |nullptr|. And this doesn’t involve reference counter updates.  This appears to be particularly efficient when dealing with |scoped_refptrs| to |ThreadSafeRefCounted| objects, which has relatively more expensive reference counter updates.

Notes:

• We don’t need .Pass() explicitly if it’s already an rvalue. So some existing code will benefit from this automatically.  Please don’t use .Pass() if it’s an rvalue already or it’s clear from the context that RVO is applied because .Pass() can prevent RVO.
  
• Blink equivalent is PassRefPtr. https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/wtf/PassRefPtr.h&q=passrefptr&sq=package:chromium&type=cs&l=21 https://www.webkit.org/coding/RefPtr.html
  

Thanks to dcheng@, danakj@, mgiuca@, rsleevi@, and trchen@ for reviewing and help completing the CL.

Kibeom

[Dale Curtis]

Nice work! I just tried to use it with a ScopedComPtr (which inherits scoped_refptr) and it worked in most cases, but this fails to compile:

// in .h file

ScopedComPtr<IStream> com_stream_;

// In a function in the .cc file.

ScopedComPtr<IStream> com_stream = com_stream_.Pass();

e:\src\chrome\src\media\audio\win\audio_low_latency_output_win.cc(700) : error C2440: 'initializing' : cannot convert from 'scoped_refptr<Interface>' to 'base::win::ScopedComPtr<IStream,&_GUID_0000000c_0000_0000_c000_000000000046>'

        with

        [

            Interface=IStream

        ]

        No constructor could take the source type, or constructor overload resolution was ambiguous

ninja: build stopped: subcommand failed.

// This works though:

ScopedComPtr<IStream> com_stream;

com_stream = com_stream_.Pass();

Is this expected?

- dale

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[James Robinson]

ScopedComPtr pulls in operator= from its parent: https://code.google.com/p/chromium/codesearch#chromium/src/base/win/scoped_comptr.h&q=ScopedComPtr&sq=package:chromium&type=cs&l=154 but not constructors.  I expect that's why assignment works but not construction.  Implementation inheritance from scoped_refptr<> seems pretty dodgy, perhaps this class should compose a scoped_refptr<> member and expose the COM-specific functionality explicitly.

- James

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

[Kibeom Kim]

Since .Pass(); is not implemented in ScopedComPtr,  ScopedComPtr<>::Pass() is scoped_ptr<>::Pass(), which is effectively { return static_cast<scoped_ptr<>&&>(*this); }. Also ScopedComPtr only has a copy constructor, not a move constructor. I think a quick consistent fix is adding those two.