Start BindStateBase ref count from 1 instead of 0

Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds `base::AdoptRef` to scoped_refptr for ref counted types whose
ref count start from 1, and converts BindStateBase to use it.

Review-Url: https://codereview.chromium.org/2723423002
Cr-Commit-Position: refs/heads/master@{#461372}
Committed: https://chromium.googlesource.com/chromium/src/+/65f39693c549f42146b26cd35b13fa6a94fa0744

[tzik, 2017-03-02 05:58:16]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 05:59:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-02 06:16:24]

Description was changed from

==========
Start BindStateBase ref count from 1 instead of 0

BUG=
==========

to

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor until first page load from the browser boot, and also
it sometimes hits invalid state.

This CL adds a scoped_refptr constructor that doesn't increment the ref
count, and converts BindStateBase to use it.
==========

[tzik, 2017-03-02 06:22:09]

Description was changed from

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor until first page load from the browser boot, and also
it sometimes hits invalid state.

This CL adds a scoped_refptr constructor that doesn't increment the ref
count, and converts BindStateBase to use it.
==========

to

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor until first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds a scoped_refptr constructor that doesn't increment the ref
count, and converts BindStateBase to use it.
==========

[tzik, 2017-03-02 06:32:47]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 06:33:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 06:46:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 06:46:54]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[tzik, 2017-03-02 06:56:39]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 06:57:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 08:24:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 08:24:52]

Dry run: This issue passed the CQ dry run.

[tzik, 2017-03-02 12:06:24]

Description was changed from

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor until first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds a scoped_refptr constructor that doesn't increment the ref
count, and converts BindStateBase to use it.
==========

to

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds a scoped_refptr constructor that doesn't increment the ref
count, and converts BindStateBase to use it.
==========

[tzik, 2017-03-06 13:03:42]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-06 13:03:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-06 13:07:44]

Description was changed from

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds a scoped_refptr constructor that doesn't increment the ref
count, and converts BindStateBase to use it.
==========

to

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds `AdoptRef` to scoped_refptr for ref counted types whose ref
count start from 1, and converts BindStateBase to use it.
==========

[tzik, 2017-03-06 13:08:05]

Description was changed from

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds `AdoptRef` to scoped_refptr for ref counted types whose ref
count start from 1, and converts BindStateBase to use it.
==========

to

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds `base::AdoptRef` to scoped_refptr for ref counted types whose
ref count start from 1, and converts BindStateBase to use it.
==========

[tzik, 2017-03-06 13:42:10]

tzik@chromium.org changed reviewers:
+ dcheng@chromium.org

[tzik, 2017-03-06 13:42:10]

PTAL

[commit-bot: I haz the power, 2017-03-06 14:41:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-06 14:41:50]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-03-06 22:09:19]

"also it sometimes hits invalid state."

Can you explain what this means?

In general, I'm a little nervous about changes like this, since it goes against
how all the other refcounting types in Chromium work. Are you planning more uses
of this (i.e. for the swap() work?) outside of callback?

https://codereview.chromium.org/2723423002/diff/60001/base/memory/ref_counted.h
File base/memory/ref_counted.h (right):

https://codereview.chromium.org/2723423002/diff/60001/base/memory/ref_counted...
base/memory/ref_counted.h:391: struct AdoptRefTag {};
FWIW, the usual trick I've seen in the past is to use an enum here: for example,
see
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/webor...

[tzik, 2017-03-07 07:18:07]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 07:19:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-07 08:59:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 08:59:10]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[tzik, 2017-03-07 13:35:23]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-03-07 13:35:24]

The CQ bit was unchecked by tzik@chromium.org

[tzik, 2017-03-07 13:35:31]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[tzik, 2017-03-07 13:35:55]

On 2017/03/06 22:09:19, dcheng wrote:
> "also it sometimes hits invalid state."

There are a number of crash report on the callback destruction such as:
http://crash/?q=stable_signature%20LIKE%20%27base%3A%3Ainternal%3A%3ACallback...
http://crash/?q=stable_signature%20LIKE%20%27base%3A%3Ainternal%3A%3ABindStat...

There might be various reason each, but I think some of them are UAF of callback
objects that we can detect it in AddRef() as a 0 to 1 increment. Then, I think
we can get actionable stack trace on the crash dashboard.

> 
> Can you explain what this means?
> 
> In general, I'm a little nervous about changes like this, since it goes
against
> how all the other refcounting types in Chromium work. Are you planning more
uses
> of this (i.e. for the swap() work?) outside of callback?

I currently don't have enough time to migrate all use of ref count to this. But
I think it's better to use 1-start ref count broadly, so that we can trap more
ref count related UAF in the wild.
# This is not related to swap() work. That was just a drive by clean up.

[commit-bot: I haz the power, 2017-03-07 13:36:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-07 13:39:42]

https://codereview.chromium.org/2723423002/diff/60001/base/memory/ref_counted.h
File base/memory/ref_counted.h (right):

https://codereview.chromium.org/2723423002/diff/60001/base/memory/ref_counted...
base/memory/ref_counted.h:391: struct AdoptRefTag {};
On 2017/03/06 22:09:18, dcheng wrote:
> FWIW, the usual trick I've seen in the past is to use an enum here: for
example,
> see
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/webor...

Done.

[commit-bot: I haz the power, 2017-03-07 15:05:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 15:05:31]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-03-08 07:39:43]

Is it possible to scope this to Callback only? I'm a bit uncomfortable with
exposing AdoptRef, since:
- the default RefCounted implementation can't be used safely with it
- yet it's available in the base namespace

Also acceptable would be figuring out a way to get Blink's DCHECKs (which verify
that the reference is adopted, etc, but that feels even more complicated...)

[tzik, 2017-03-21 12:17:15]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 12:17:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 13:01:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 13:01:52]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[tzik, 2017-03-21 13:10:00]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 13:10:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-21 13:51:05]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 13:51:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-21 14:05:18]

On 2017/03/08 07:39:43, dcheng wrote:
> Is it possible to scope this to Callback only? I'm a bit uncomfortable with
> exposing AdoptRef, since:
> - the default RefCounted implementation can't be used safely with it
> - yet it's available in the base namespace
> 
> Also acceptable would be figuring out a way to get Blink's DCHECKs (which
verify
> that the reference is adopted, etc, but that feels even more complicated...)

How about make non-zero initial value generally available rather than limit to
Callback as PS8?
RefCounted in PS8 provides an optional initial value of the reference count,
which can be used with AdoptRef.

[commit-bot: I haz the power, 2017-03-21 15:36:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-21 15:36:01]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-03-30 04:26:16]

Sorry for the review delay =(

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counted.h
File base/memory/ref_counted.h (right):

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:39: explicit RefCountedBase(size_t initial_ref_count =
0)
I think I would prefer that we have a default ctor which starts at 0 and then a
single-argument constructor which takes a special tag to tell the refcount to
start at 1.

(We DCHECK so this should be noticed fairly quickly, but I think it would be
better if subclasses couldn't specify the refcount directly)

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:84: return ref_count_ == 0;
What is the purpose / necessity for these changes?

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:265: DCHECK(!t || t->HasOneRef());
Hmm... why would we want to handle a null |t| here? In general, it seems like we
want this to always succeed.

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:278: return scoped_refptr<T>(obj);
Not sure if it's possible, but it would be nice if we didn't have to do runtime
selection. Is it possible to be clever here?

For example, maybe we could do something like what C++03 move support used to
do: it provided a macro that movable things would include in their class
declaration, and we would use the presence of a typedef on T to determine if it
should be treated as move-only or not. Perhaps we could do that here? Not sure
if we could make the constructors work with that too... but if that's possible,
that might be a nice safety improvement.

[tzik, 2017-03-30 12:21:51]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 12:22:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-30 13:00:12]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 13:00:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-30 13:08:06]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 13:08:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-30 13:16:28]

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counted.h
File base/memory/ref_counted.h (right):

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:84: return ref_count_ == 0;
On 2017/03/30 04:26:15, dcheng wrote:
> What is the purpose / necessity for these changes?

Oops, this is a contamination from another CL: http://crrev.com/2666423002.
Reverted from this CL.

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:265: DCHECK(!t || t->HasOneRef());
On 2017/03/30 04:26:16, dcheng wrote:
> Hmm... why would we want to handle a null |t| here? In general, it seems like
we
> want this to always succeed.

OK, updated to force nun-null.

https://codereview.chromium.org/2723423002/diff/140001/base/memory/ref_counte...
base/memory/ref_counted.h:278: return scoped_refptr<T>(obj);
On 2017/03/30 04:26:16, dcheng wrote:
> Not sure if it's possible, but it would be nice if we didn't have to do
runtime
> selection. Is it possible to be clever here?
> 
> For example, maybe we could do something like what C++03 move support used to
> do: it provided a macro that movable things would include in their class
> declaration, and we would use the presence of a typedef on T to determine if
it
> should be treated as move-only or not. Perhaps we could do that here? Not sure
> if we could make the constructors work with that too... but if that's
possible,
> that might be a nice safety improvement.

Trying that.

https://codereview.chromium.org/2723423002/diff/180001/base/memory/ref_counte...
File base/memory/ref_counted_unittest.cc (right):

https://codereview.chromium.org/2723423002/diff/180001/base/memory/ref_counte...
base/memory/ref_counted_unittest.cc:129: base::subtle::kStartRefCountFromOneTag;
We should probably replace this with a macro.

[commit-bot: I haz the power, 2017-03-30 15:24:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 15:24:34]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-03-30 21:15:25]

LGTM

https://codereview.chromium.org/2723423002/diff/180001/base/memory/ref_counte...
File base/memory/ref_counted_unittest.cc (right):

https://codereview.chromium.org/2723423002/diff/180001/base/memory/ref_counte...
base/memory/ref_counted_unittest.cc:129: base::subtle::kStartRefCountFromOneTag;
On 2017/03/30 13:16:28, tzik wrote:
> We should probably replace this with a macro.

Sounds good to me.

(To give some background context: I think my original thought was we could use
the absence of the tag to use the original start-from-zero behavior. In theory,
this would let us incrementally migrate the codebase to using adoption
semantics. But this way should be fine too: if a subclass declares a preference,
it should still work.)

https://codereview.chromium.org/2723423002/diff/200001/base/callback_internal.cc
File base/callback_internal.cc (right):

https://codereview.chromium.org/2723423002/diff/200001/base/callback_internal...
base/callback_internal.cc:78: : bind_state_(bind_state ? AdoptRef(bind_state) :
nullptr) {
Can you remind me when we would bind a null state here? Codesearch doesn't work
for this.

[tzik, 2017-03-31 13:24:50]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 13:25:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-03-31 13:31:00]

Added document and a macro to opt-in. Could you take another look to them?

https://codereview.chromium.org/2723423002/diff/200001/base/callback_internal.cc
File base/callback_internal.cc (right):

https://codereview.chromium.org/2723423002/diff/200001/base/callback_internal...
base/callback_internal.cc:78: : bind_state_(bind_state ? AdoptRef(bind_state) :
nullptr) {
On 2017/03/30 21:15:25, dcheng wrote:
> Can you remind me when we would bind a null state here? Codesearch doesn't
work
> for this.

It's null when it's called from the default constructor of Callback.
https://cs.chromium.org/chromium/src/base/callback.h?q=callback.h&dr=CSs&l=47

[commit-bot: I haz the power, 2017-03-31 15:31:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 15:31:28]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-04-03 00:07:30]

LGTM

https://codereview.chromium.org/2723423002/diff/220001/base/memory/ref_counted.h
File base/memory/ref_counted.h (right):

https://codereview.chromium.org/2723423002/diff/220001/base/memory/ref_counte...
base/memory/ref_counted.h:186: #define MAKE_REF_COUNT_START_FROM_ONE            
         \
Two nits
1) Make this a function-style macro, so that a typo fails to compile.
2) Maybe name it something like START_REFCOUNT_AT_ONE() or
REQUIRE_ADOPTION_FOR_REFCOUNTED_TYPE().

[tzik, 2017-04-03 02:23:06]

The CQ bit was checked by tzik@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 02:23:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-04-03 03:48:23]

https://codereview.chromium.org/2723423002/diff/220001/base/memory/ref_counted.h
File base/memory/ref_counted.h (right):

https://codereview.chromium.org/2723423002/diff/220001/base/memory/ref_counte...
base/memory/ref_counted.h:186: #define MAKE_REF_COUNT_START_FROM_ONE            
         \
On 2017/04/03 00:07:28, dcheng wrote:
> Two nits
> 1) Make this a function-style macro, so that a typo fails to compile.
> 2) Maybe name it something like START_REFCOUNT_AT_ONE() or
> REQUIRE_ADOPTION_FOR_REFCOUNTED_TYPE().

Done. Updated its name to REQUIRE_ADOPTION_FOR_REFCOUNTED_TYPE.

[commit-bot: I haz the power, 2017-04-03 04:20:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 04:20:06]

Dry run: This issue passed the CQ dry run.

[tzik, 2017-04-03 05:22:46]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-04-03 05:22:47]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2723423002/#ps240001
(title:
"s/MAKE_REF_COUNT_START_FROM_ONE/REQUIRE_ADOPTION_FOR_REFCOUNTED_TYPE()/")

[commit-bot: I haz the power, 2017-04-03 05:23:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 05:27:38]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1491196966573940,
"parent_rev": "f98f8cb0517d4b28943102a007528063252e0ce0", "commit_rev":
"65f39693c549f42146b26cd35b13fa6a94fa0744"}

[commit-bot: I haz the power, 2017-04-03 05:29:18]

Description was changed from

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds `base::AdoptRef` to scoped_refptr for ref counted types whose
ref count start from 1, and converts BindStateBase to use it.
==========

to

==========
Start BindStateBase ref count from 1 instead of 0

The first atomic increments of ref counts are generally unneeded if the
ref count starts from 1 instead of 0, and we can detect an invalid AddRef
from 0 to 1 in that case.
Especially, the ref count in BindStateBase is incremented 300k times in
its constructor by the first page load from the browser boot, and also
it sometimes hits invalid state.
Note that 300k atomic increments are probably not so time consuming as
we can measure.

This CL adds `base::AdoptRef` to scoped_refptr for ref counted types whose
ref count start from 1, and converts BindStateBase to use it.

Review-Url: https://codereview.chromium.org/2723423002
Cr-Commit-Position: refs/heads/master@{#461372}
Committed:
https://chromium.googlesource.com/chromium/src/+/65f39693c549f42146b26cd35b13...
==========

[commit-bot: I haz the power, 2017-04-03 05:29:20]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/65f39693c549f42146b26cd35b13...