Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
Committed: https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57
Cr-Commit-Position: refs/heads/master@{#414005}

[hiroshige, 2016-08-17 06:40:04]

Description was changed from

==========
Remove Resource from MemoryCache instead of calling SharedBuffer::unlock()

BUG=603791, 569162
==========

to

==========
Remove Resource from MemoryCache instead of calling SharedBuffer::unlock()

[1/2] CL of Step 1 of removing SharedBuffer::unlock() and PurgeableVector:
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...

This CL makes Resource's SharedBuffer always locked and never unlocked by
replacing SharedBuffer::unlock() call with MemoryCache::remove(), and
removes the following:
- Resource::lock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

BUG=603791, 569162
==========

[hiroshige, 2016-08-17 07:24:58]

Description was changed from

==========
Remove Resource from MemoryCache instead of calling SharedBuffer::unlock()

[1/2] CL of Step 1 of removing SharedBuffer::unlock() and PurgeableVector:
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...

This CL makes Resource's SharedBuffer always locked and never unlocked by
replacing SharedBuffer::unlock() call with MemoryCache::remove(), and
removes the following:
- Resource::lock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

BUG=603791, 569162
==========

to

==========
Remove Resource from MemoryCache instead of calling SharedBuffer::unlock()

This CL makes Resource's SharedBuffer always locked and never unlocked by
replacing the SharedBuffer::unlock() call with MemoryCache::remove(), and
propagates the change by removing the following:
- Resource::lock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
==========

[hiroshige, 2016-08-17 07:53:57]

hiroshige@chromium.org changed reviewers:
+ japhet@chromium.org, mkwst@chromium.org, yhirano@chromium.org

[hiroshige, 2016-08-17 07:53:58]

PTAL.

[haraken, 2016-08-17 08:13:30]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-08-17 08:13:31]

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/MemoryCache.cpp (left):

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/MemoryCache.cpp:593: purgeableSize +=
purgeable ? pageSize : 0;

Can you remove the purgeableSize member entirely?

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/Resource.cpp:562:
memoryCache()->remove(this);

Would you help me understand why you need to proactively remove the Resource
from MemoryCache? What happens if you don't call memoryCache()->remove()? (i.e.,
simply remove Resource::unlock)

[hiroshige, 2016-08-17 09:02:38]

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/MemoryCache.cpp (left):

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/MemoryCache.cpp:593: purgeableSize +=
purgeable ? pageSize : 0;
On 2016/08/17 08:13:31, haraken wrote:
> 
> Can you remove the purgeableSize member entirely?

Yes.
Created a separate CL: https://codereview.chromium.org/2254593003/.

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/Resource.cpp:562:
memoryCache()->remove(this);
On 2016/08/17 08:13:31, haraken wrote:
> 
> Would you help me understand why you need to proactively remove the Resource
> from MemoryCache? What happens if you don't call memoryCache()->remove()?
(i.e.,
> simply remove Resource::unlock)
Because the Resource is not expected to be reused frequently, I remove it from
MemoryCache to save memory.
Functionally we can also simply remove the unlock() call here, and the two
choices will be the same after WeakMemoryCache is landed.
(Added "Alternatives Considered" section in the doc:
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...)

[haraken, 2016-08-17 09:10:26]

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/Resource.cpp:562:
memoryCache()->remove(this);
On 2016/08/17 09:02:38, hiroshige wrote:
> On 2016/08/17 08:13:31, haraken wrote:
> > 
> > Would you help me understand why you need to proactively remove the Resource
> > from MemoryCache? What happens if you don't call memoryCache()->remove()?
> (i.e.,
> > simply remove Resource::unlock)
> Because the Resource is not expected to be reused frequently, I remove it from
> MemoryCache to save memory.
> Functionally we can also simply remove the unlock() call here, and the two
> choices will be the same after WeakMemoryCache is landed.
> (Added "Alternatives Considered" section in the doc:
>
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...)

Makes sense.

I'm feeling that whether we call memoryCache()->remove() or not is an orthogonal
issue to this CL (I guess calling memoryCache()->remove() changes the cache hit
rate of MemoryCache). Would it make more sense to simply remove
Resource::unlock() in this CL and then improving the lifetime of MemoryCache
when you weaken the references in MemoryCache?

[hiroshige, 2016-08-18 05:28:44]

On 2016/08/17 09:10:26, haraken wrote:
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/fetch/Resource.cpp (right):
> 
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/fetch/Resource.cpp:562:
> memoryCache()->remove(this);
> On 2016/08/17 09:02:38, hiroshige wrote:
> > On 2016/08/17 08:13:31, haraken wrote:
> > > 
> > > Would you help me understand why you need to proactively remove the
Resource
> > > from MemoryCache? What happens if you don't call memoryCache()->remove()?
> > (i.e.,
> > > simply remove Resource::unlock)
> > Because the Resource is not expected to be reused frequently, I remove it
from
> > MemoryCache to save memory.
> > Functionally we can also simply remove the unlock() call here, and the two
> > choices will be the same after WeakMemoryCache is landed.
> > (Added "Alternatives Considered" section in the doc:
> >
>
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...)
> 
> Makes sense.
> 
> I'm feeling that whether we call memoryCache()->remove() or not is an
orthogonal
> issue to this CL (I guess calling memoryCache()->remove() changes the cache
hit
> rate of MemoryCache). Would it make more sense to simply remove
> Resource::unlock() in this CL and then improving the lifetime of MemoryCache
> when you weaken the references in MemoryCache?

Simply removing Resource::unlock() will make the code simpler and moving the
lifetime change to WeakMemoryCache makes sense.
I'm fine with it, if memory team is fine with potential memory pressure
increase.
Uploaded Patch Set 9.

[hiroshige, 2016-08-18 05:31:21]

Description was changed from

==========
Remove Resource from MemoryCache instead of calling SharedBuffer::unlock()

This CL makes Resource's SharedBuffer always locked and never unlocked by
replacing the SharedBuffer::unlock() call with MemoryCache::remove(), and
propagates the change by removing the following:
- Resource::lock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
==========

to

==========
Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
==========

[haraken, 2016-08-18 06:22:51]

On 2016/08/18 05:28:44, hiroshige wrote:
> On 2016/08/17 09:10:26, haraken wrote:
> >
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/fetch/Resource.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/fetch/Resource.cpp:562:
> > memoryCache()->remove(this);
> > On 2016/08/17 09:02:38, hiroshige wrote:
> > > On 2016/08/17 08:13:31, haraken wrote:
> > > > 
> > > > Would you help me understand why you need to proactively remove the
> Resource
> > > > from MemoryCache? What happens if you don't call
memoryCache()->remove()?
> > > (i.e.,
> > > > simply remove Resource::unlock)
> > > Because the Resource is not expected to be reused frequently, I remove it
> from
> > > MemoryCache to save memory.
> > > Functionally we can also simply remove the unlock() call here, and the two
> > > choices will be the same after WeakMemoryCache is landed.
> > > (Added "Alternatives Considered" section in the doc:
> > >
> >
>
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...)
> > 
> > Makes sense.
> > 
> > I'm feeling that whether we call memoryCache()->remove() or not is an
> orthogonal
> > issue to this CL (I guess calling memoryCache()->remove() changes the cache
> hit
> > rate of MemoryCache). Would it make more sense to simply remove
> > Resource::unlock() in this CL and then improving the lifetime of MemoryCache
> > when you weaken the references in MemoryCache?
> 
> Simply removing Resource::unlock() will make the code simpler and moving the
> lifetime change to WeakMemoryCache makes sense.
> I'm fine with it, if memory team is fine with potential memory pressure
> increase.
> Uploaded Patch Set 9.

How much do you think we should worry about the potential increase? My feeling
is it's just potential :)

[yhirano, 2016-08-18 09:28:28]

https://codereview.chromium.org/2253853002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/2253853002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/Resource.cpp:822:
dump->addScalar("live_size", "bytes", m_encodedSize);
Do these live/dead concepts make sense without isLocked()?

[hiroshige, 2016-08-19 04:56:23]

On 2016/08/18 06:22:51, haraken wrote:
> On 2016/08/18 05:28:44, hiroshige wrote:
> > On 2016/08/17 09:10:26, haraken wrote:
> > >
> >
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/core/fetch/Resource.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/core/fetch/Resource.cpp:562:
> > > memoryCache()->remove(this);
> > > On 2016/08/17 09:02:38, hiroshige wrote:
> > > > On 2016/08/17 08:13:31, haraken wrote:
> > > > > 
> > > > > Would you help me understand why you need to proactively remove the
> > Resource
> > > > > from MemoryCache? What happens if you don't call
> memoryCache()->remove()?
> > > > (i.e.,
> > > > > simply remove Resource::unlock)
> > > > Because the Resource is not expected to be reused frequently, I remove
it
> > from
> > > > MemoryCache to save memory.
> > > > Functionally we can also simply remove the unlock() call here, and the
two
> > > > choices will be the same after WeakMemoryCache is landed.
> > > > (Added "Alternatives Considered" section in the doc:
> > > >
> > >
> >
>
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...)
> > > 
> > > Makes sense.
> > > 
> > > I'm feeling that whether we call memoryCache()->remove() or not is an
> > orthogonal
> > > issue to this CL (I guess calling memoryCache()->remove() changes the
cache
> > hit
> > > rate of MemoryCache). Would it make more sense to simply remove
> > > Resource::unlock() in this CL and then improving the lifetime of
MemoryCache
> > > when you weaken the references in MemoryCache?
> > 
> > Simply removing Resource::unlock() will make the code simpler and moving the
> > lifetime change to WeakMemoryCache makes sense.
> > I'm fine with it, if memory team is fine with potential memory pressure
> > increase.
> > Uploaded Patch Set 9.
> 
> How much do you think we should worry about the potential increase? My feeling
> is it's just potential :)

I expect probably OK but not sure because we haven't collected metrics.
It affects "SharedBuffer that would have been purged before destructed"
(No. 3 in the figure in the doc).
We haven't inserted UMAs for that.
However, from the ratio of purged(A):not-purged(B) in Resources that are locked
again, the number of such Resources would be about 0.4% of CSSStyleSheet
requests, which is sufficiently low.

[hiroshige, 2016-08-19 05:37:43]

https://codereview.chromium.org/2253853002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/2253853002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/Resource.cpp:822:
dump->addScalar("live_size", "bytes", m_encodedSize);
On 2016/08/18 09:28:27, yhirano wrote:
> Do these live/dead concepts make sense without isLocked()?

Nice catch.
In fact, there seems multiple definitions of "Live":
1. hasClientsOrObservers() is true (Most of Resource, e.g. callsites of
MemoryCache::makeDead() and makeLive(), allClientsAndObserversRemoved())
2. the Resource is on MemoryCache's live list (i.e. hasClientsOrObservers() is
true and decodedSize() > 0 and some other conditions...)
3. isLocked() (here; 1. is a subset of 3.)

I replaced this here with hasClientsOrObservers() to unify 1. and 3.
(and let unifying 1. and 2. a separate issue)

[haraken, 2016-08-19 06:34:03]

On 2016/08/19 04:56:23, hiroshige wrote:
> On 2016/08/18 06:22:51, haraken wrote:
> > On 2016/08/18 05:28:44, hiroshige wrote:
> > > On 2016/08/17 09:10:26, haraken wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> > > > File third_party/WebKit/Source/core/fetch/Resource.cpp (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2253853002/diff/100001/third_party/WebKit/Sou...
> > > > third_party/WebKit/Source/core/fetch/Resource.cpp:562:
> > > > memoryCache()->remove(this);
> > > > On 2016/08/17 09:02:38, hiroshige wrote:
> > > > > On 2016/08/17 08:13:31, haraken wrote:
> > > > > > 
> > > > > > Would you help me understand why you need to proactively remove the
> > > Resource
> > > > > > from MemoryCache? What happens if you don't call
> > memoryCache()->remove()?
> > > > > (i.e.,
> > > > > > simply remove Resource::unlock)
> > > > > Because the Resource is not expected to be reused frequently, I remove
> it
> > > from
> > > > > MemoryCache to save memory.
> > > > > Functionally we can also simply remove the unlock() call here, and the
> two
> > > > > choices will be the same after WeakMemoryCache is landed.
> > > > > (Added "Alternatives Considered" section in the doc:
> > > > >
> > > >
> > >
> >
>
https://docs.google.com/document/d/1Q1G8dZ3k78N7DZaxf4wUAlKsSfwsclmhWqDIEMWZO...)
> > > > 
> > > > Makes sense.
> > > > 
> > > > I'm feeling that whether we call memoryCache()->remove() or not is an
> > > orthogonal
> > > > issue to this CL (I guess calling memoryCache()->remove() changes the
> cache
> > > hit
> > > > rate of MemoryCache). Would it make more sense to simply remove
> > > > Resource::unlock() in this CL and then improving the lifetime of
> MemoryCache
> > > > when you weaken the references in MemoryCache?
> > > 
> > > Simply removing Resource::unlock() will make the code simpler and moving
the
> > > lifetime change to WeakMemoryCache makes sense.
> > > I'm fine with it, if memory team is fine with potential memory pressure
> > > increase.
> > > Uploaded Patch Set 9.
> > 
> > How much do you think we should worry about the potential increase? My
feeling
> > is it's just potential :)
> 
> I expect probably OK but not sure because we haven't collected metrics.
> It affects "SharedBuffer that would have been purged before destructed"
> (No. 3 in the figure in the doc).
> We haven't inserted UMAs for that.
> However, from the ratio of purged(A):not-purged(B) in Resources that are
locked
> again, the number of such Resources would be about 0.4% of CSSStyleSheet
> requests, which is sufficiently low.

Thanks, then I'm fine with simply dropping SharedBuffer::unlock().

[haraken, 2016-08-19 06:36:43]

LGTM on my side.

[Mike West, 2016-08-19 09:19:15]

LGTM 2, FWIW.

[yhirano, 2016-08-23 04:59:05]

lgtm

[hiroshige, 2016-08-24 04:25:44]

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

[commit-bot: I haz the power, 2016-08-24 04:26:07]

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, 2016-08-24 06:09:21]

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

[commit-bot: I haz the power, 2016-08-24 06:09:22]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-08-24 07:05:29]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2016-08-24 07:06:00]

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, 2016-08-24 07:11:04]

Description was changed from

==========
Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
==========

to

==========
Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
==========

[commit-bot: I haz the power, 2016-08-24 07:11:06]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-08-24 07:12:41]

Description was changed from

==========
Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162
==========

to

==========
Remove SharedBuffer::unlock() and keep Resource's SharedBuffer always locked

This CL makes Resource's SharedBuffer always locked and never unlocked by
removing SharedBuffer::unlock(), and
propagates the change by removing the following:
- Resource::lock(), unlock() (does nothing and returns always true)
- Resource::isPurgeable() (always false)
- Resource::isSafeToUnlock() (no longer called)
- SharedBuffer::lock(), unlock(), isLocked() (always true)
- PurgeableVector::lock(), unlock(), isLocked() (always true)
- Unit tests related to SharedBuffer/PurgeableVector::lock()/unlock() from
  PurgeableVectorTest and ResourceTest.

After this CL, the SharedBuffer is still allocated as discardable memory.
The SharedBuffer will be turned into non-discardable by
https://codereview.chromium.org/2247073007/.

BUG=603791, 569162

Committed: https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57
Cr-Commit-Position: refs/heads/master@{#414005}
==========

[commit-bot: I haz the power, 2016-08-24 07:12:42]

Patchset 10 (id:??) landed as
https://crrev.com/18d61e56cf1b2c1deaa0dec1a4cda1f1fd825f57
Cr-Commit-Position: refs/heads/master@{#414005}