Rename a few Member and Persistent methods to make the transition

Rename a few Member and Persistent methods to make the transition
period simpler.

We will need to have Members and Persistents instead of RefPtrs.
On RefPtrs the methods that correspond to raw and clear are called
get and release. Unify on the RefPtr methods to make the transition
period code simpler.

Fix comparison operators that were unused and did not work because
they accessed private members.

R=haraken@chromium.org, wibling@chromium.org, zerny@chromium.org, erik.corry@gmail.com
BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165110

[wibling-chromium, 2014-01-14 13:52:37]

lgtm

[haraken, 2014-01-14 13:57:10]

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h
File Source/heap/Handle.h (right):

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h#newcode196
Source/heap/Handle.h:196: T* release()

Just to confirm: The places where we want to call Persistent::release() are
exactly the same as the places where we're currently calling RefPtr::release(),
right?

I'm asking this because in the replacement work in our experimental branch,
we've sometimes replaced ptr.release() with ptr and we've sometimes replaces
ptr.release() with ptr.clear().

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h#newcode269
Source/heap/Handle.h:269: T* release()

Ditto.

https://codereview.chromium.org/138203002/diff/1/Source/heap/HeapTest.cpp
File Source/heap/HeapTest.cpp (right):

https://codereview.chromium.org/138203002/diff/1/Source/heap/HeapTest.cpp#new...
Source/heap/HeapTest.cpp:1013: Heap::init();

This will be removed in wibling's CL, which is currently in the commit queue.

https://codereview.chromium.org/138203002/diff/1/Source/heap/HeapTest.cpp#new...
Source/heap/HeapTest.cpp:1023: Heap::shutdown();

Ditto.

[Mads Ager (chromium), 2014-01-14 14:03:13]

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h
File Source/heap/Handle.h (right):

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h#newcode196
Source/heap/Handle.h:196: T* release()
On 2014/01/14 13:57:10, haraken wrote:
> 
> Just to confirm: The places where we want to call Persistent::release() are
> exactly the same as the places where we're currently calling
RefPtr::release(),
> right?
> 
> I'm asking this because in the replacement work in our experimental branch,
> we've sometimes replaced ptr.release() with ptr and we've sometimes replaces
> ptr.release() with ptr.clear().

It seems to me that it should mostly be the case that if we replace a RefPtr
with a Persistent we will want to call release at the same times as well. There
will definitely be cases where we will want to and this patch makes that easier.
If/when we find cases where we do not want to call release on one of them we
will have to either introduce other methods with different meanings depending on
ENABLE_OILPAN or ifdef the code.

https://codereview.chromium.org/138203002/diff/1/Source/heap/HeapTest.cpp
File Source/heap/HeapTest.cpp (right):

https://codereview.chromium.org/138203002/diff/1/Source/heap/HeapTest.cpp#new...
Source/heap/HeapTest.cpp:1013: Heap::init();
On 2014/01/14 13:57:10, haraken wrote:
> 
> This will be removed in wibling's CL, which is currently in the commit queue.

Yes. I'll wait for Gustav's changes, rebase and put in the cq.

[haraken, 2014-01-14 14:23:15]

LGTM.

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h
File Source/heap/Handle.h (right):

https://codereview.chromium.org/138203002/diff/1/Source/heap/Handle.h#newcode196
Source/heap/Handle.h:196: T* release()
On 2014/01/14 14:03:13, Mads Ager (chromium) wrote:
> On 2014/01/14 13:57:10, haraken wrote:
> > 
> > Just to confirm: The places where we want to call Persistent::release() are
> > exactly the same as the places where we're currently calling
> RefPtr::release(),
> > right?
> > 
> > I'm asking this because in the replacement work in our experimental branch,
> > we've sometimes replaced ptr.release() with ptr and we've sometimes replaces
> > ptr.release() with ptr.clear().
> 
> It seems to me that it should mostly be the case that if we replace a RefPtr
> with a Persistent we will want to call release at the same times as well.
There
> will definitely be cases where we will want to and this patch makes that
easier.
> If/when we find cases where we do not want to call release on one of them we
> will have to either introduce other methods with different meanings depending
on
> ENABLE_OILPAN or ifdef the code.

If there are cases where we must not replace RefPtr::release() with
Persistent::release() (I don't know if there are cases where we *must not* do
the replacement), my concern is that this CL will implicitly do the replacement
and cause unexpected, premature releases of persistent handles. I'm wondering
this since we've not always replaced RefPtr::release() with
Persistent::release().

That said, I'm OK with going with this CL and see how it goes.

[zerny-google, 2014-01-14 18:44:43]

> If there are cases where we must not replace RefPtr::release() with
> Persistent::release() (I don't know if there are cases where we *must
> not* do the replacement), my concern is that this CL will implicitly do
> the replacement and cause unexpected, premature releases of persistent
> handles. I'm wondering this since we've not always replaced
> RefPtr::release() with Persistent::release().

I think the places where we removed the release entirely coincide with the
replacement of a RefPtr with a Handle/raw ptr while replacements that keep
release are the ones that end up as Member/Persistent. The difference was
less clear while we still used Handle types since it too had a clear
method.

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

[Mads Ager (chromium), 2014-01-15 07:34:14]

I removed a ton of unneeded friend declarations as well. PTAL.

[haraken, 2014-01-15 07:40:14]

LGTM

[zerny-chromium, 2014-01-15 07:43:15]

lgtm

[Mads Ager (chromium), 2014-01-15 08:24:24]

Committed patchset #3 manually as r165110 (presubmit successful).