Minor scoped_refptr cleanup in cc::OutputSurface.

Minor scoped_refptr cleanup in cc::OutputSurface.

Using const scoped_refptr<T>& is more efficient when passing a
scoped_refptr<T> to a function or returning a scoped_refptr<T> from an
accessor. It avoids the creation of a temporary scoped_refptr and
reduces refcount churn.

BUG=none

Committed: https://crrev.com/6afa1700e5019cce52fdb1d498d1d7ece69e3a2c
Cr-Commit-Position: refs/heads/master@{#291955}

[dcheng, 2014-08-26 15:24:53]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org, enne@chromium.org

[dcheng, 2014-08-26 15:24:53]

This was mentioned yesterday in the review of
https://codereview.chromium.org/500123003 (one of the scoped_refptr implicit
conversion cleanup CLs). Obviously, this is probably not a performance
bottleneck for Chrome (even though this idiom is pretty common), but it's
roughly akin to using RefPtr<> in Blink when PassRefPtr<> would have sufficed.

Using const scoped_refptr<T>& still hints to callers that they're dealing with a
refcounted type, but without the refcount churn if the consuming code knows it
doesn't need to keep it alive itself.

It also makes my life a tiny bit easier, since the scoped_refptr cleanup tool
assumes that you won't shoot yourself in the foot and will rewrite things like:
const scoped_refptr<Foo>& GetFoo();
Foo* f = GetFoo();
to:
Foo* f = GetFoo().get();
instead of:
scoped_refptr<Foo> f = GetFoo();
which is what the tool was doing yesterday whenever it encountered
OutputSurface::context_provider.

There is one situation where it doesn't work though--since it's not an actual
class like PassRefPtr<>, things like this won't work since the code would be
returning a ref to a temporary:
const scoped_refptr<T>& Bar::GetFoo() {
  return x_ ? x_->foo() : NULL;
}
or:
const scoped_refptr<U>& Bar::GetFoo() {
  return t_;  // t is a scoped_refptr<T>
}

[danakj, 2014-08-26 15:35:18]

LGTM

Did you try changing this to a ContextProvider*? Was there something that needed
to take a ref after all?

[dcheng, 2014-08-26 16:00:07]

PTAL. I've changed context_provider() to return a raw pointer and it seems
fine--all callsites were unwrapping it with get() anyway.

I didn't do this originally because I misinterpreted your request yesterday, and
const scoped_refptr<T>& is a drop in replacement, while T* is not =)

[danakj, 2014-08-26 16:19:01]

LGTM!

[danakj, 2014-08-26 16:19:16]

danakj@chromium.org changed reviewers:
+ jamesr@chromium.org

[danakj, 2014-08-26 16:19:16]

+jamesr for content/renderer/gpu

[jamesr, 2014-08-26 17:11:01]

lgtm

[dcheng, 2014-08-26 17:15:16]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-26 17:16:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcheng@chromium.org/467183003/20001

[commit-bot: I haz the power, 2014-08-26 18:17:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[commit-bot: I haz the power, 2014-08-26 19:11:49]

Committed patchset #2 (20001) as bf056c37ea2fa74299a203dd9917004d78cc5b05

[commit-bot: I haz the power, 2014-09-10 02:44:34]

Patchset 2 (id:??) landed as
https://crrev.com/6afa1700e5019cce52fdb1d498d1d7ece69e3a2c
Cr-Commit-Position: refs/heads/master@{#291955}