Release Oilpan heap singletons prior to LSan leak detection.

Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

(Reland of r363780.)

R=haraken
BUG=567257
Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}

Committed: https://crrev.com/2e088a7ecaf6c26f44c309b7bab507aeddb41365
Cr-Commit-Position: refs/heads/master@{#363994}

[sof, 2015-12-03 16:10:02]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-03 16:10:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/1

[sof, 2015-12-03 16:21:20]

Description was changed from

==========
Release static Oilpan heap singletons prior to LSan leak detection.

R=
BUG=
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And report no leaks over, hopefully.

A couple of exceptions to the rule of registering these
exceptions are needed -- singletons that we end up touching
as part of doing the leak-tidying GCs cannot be released.
These are instead not registered and exempted from the LSan
root set.

R=
BUG=
==========

[sof, 2015-12-03 16:21:50]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-03 16:22:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/20001

[sof, 2015-12-03 16:23:32]

sigbjornf@opera.com changed reviewers:
+ oilpan-reviews@chromium.org

[sof, 2015-12-03 16:23:33]

please take a look.

Changes split out from https://codereview.chromium.org/1482683002/ -- see it for
full (Oilpan enabled) test results.

[sof, 2015-12-03 16:27:19]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And report no leaks over, hopefully.

A couple of exceptions to the rule of registering these
exceptions are needed -- singletons that we end up touching
as part of doing the leak-tidying GCs cannot be released.
These are instead not registered and exempted from the LSan
root set.

R=
BUG=
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And report no leaks over, hopefully.

A couple of exceptions to the rule of registering these
singletons are needed -- singletons that we end up touching
as part of doing the leak-tidying GCs cannot be released.
These are instead not registered and exempted from the LSan
root set.

R=
BUG=
==========

[commit-bot: I haz the power, 2015-12-03 18:28:11]

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

[commit-bot: I haz the power, 2015-12-03 18:28:12]

Dry run: This issue passed the CQ dry run.

[haraken, 2015-12-04 02:19:51]

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

[haraken, 2015-12-04 02:19:51]

https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebKit.cpp (right):

https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebKit.cpp:209: Heap::collectAllGarbage();

Help me understand: Why do we need to run destructors of the persistents? To
suppress LSan leaks, wouldn't it be enough to just clear m_raw's of the
persistent handles?

If we don't need to run destructors, we can get rid of the complexity of
DEFINE_STATIC_LOCAL_NO_REGISTER.

[sof, 2015-12-04 07:04:08]

https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebKit.cpp (right):

https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebKit.cpp:209: Heap::collectAllGarbage();
On 2015/12/04 02:19:51, haraken wrote:
> 
> Help me understand: Why do we need to run destructors of the persistents? To
> suppress LSan leaks, wouldn't it be enough to just clear m_raw's of the
> persistent handles?
> 
> If we don't need to run destructors, we can get rid of the complexity of
> DEFINE_STATIC_LOCAL_NO_REGISTER.

All the objects reachable from the persistent needs to be freed (=> a GC is
needed.) Otherwise they will be reachable via the (shared) heap.

[haraken, 2015-12-04 08:20:27]

On 2015/12/04 07:04:08, sof wrote:
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/WebKit.cpp (right):
> 
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/WebKit.cpp:209: Heap::collectAllGarbage();
> On 2015/12/04 02:19:51, haraken wrote:
> > 
> > Help me understand: Why do we need to run destructors of the persistents? To
> > suppress LSan leaks, wouldn't it be enough to just clear m_raw's of the
> > persistent handles?
> > 
> > If we don't need to run destructors, we can get rid of the complexity of
> > DEFINE_STATIC_LOCAL_NO_REGISTER.
> 
> All the objects reachable from the persistent needs to be freed (=> a GC is
> needed.) Otherwise they will be reachable via the (shared) heap.

Thanks, makes sense.

What I'm not really happy about is the DEFINE_STATIC_LOCAL_NO_REGISTER -- it's a
bit too hard for blink developers to use the macro correctly.

Just help me understand: If we add WTF_ANNOTATE_SCOPED_MEMORY_LEAK to all
DEFINE_STATIC_LOCALs, will it solve our issues? I mean, can we do something
like:

#define DEFINE_STATIC_LOCAL ...
  WTF_ANNOTATE_SCOPED_MEMORY_LEAK; \
  ...

?

(I'm not intending to object to your fix -- it's important to fix this. I just
want to look for a better solution.)

[sof, 2015-12-04 08:51:01]

On 2015/12/04 08:20:27, haraken wrote:
> On 2015/12/04 07:04:08, sof wrote:
> >
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/web/WebKit.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/web/WebKit.cpp:209: Heap::collectAllGarbage();
> > On 2015/12/04 02:19:51, haraken wrote:
> > > 
> > > Help me understand: Why do we need to run destructors of the persistents?
To
> > > suppress LSan leaks, wouldn't it be enough to just clear m_raw's of the
> > > persistent handles?
> > > 
> > > If we don't need to run destructors, we can get rid of the complexity of
> > > DEFINE_STATIC_LOCAL_NO_REGISTER.
> > 
> > All the objects reachable from the persistent needs to be freed (=> a GC is
> > needed.) Otherwise they will be reachable via the (shared) heap.
> 
> Thanks, makes sense.
> 

If you want to see this in action, change
ThreadState::releaseStaticPersistentNodes() to

  memset(node->self(), 0, sizeof(PersistentNode));

instead of persistentRegion()->freePresistentNode(node), along with not GCing in
blink::shutdown().

(Notice that registerAsStaticReference() already marks the persistent object as
an LSan-ignored object, so this ought to be a no-op.)

> What I'm not really happy about is the DEFINE_STATIC_LOCAL_NO_REGISTER -- it's
a
> bit too hard for blink developers to use the macro correctly.
> 

I agree all the way. All but a few core global caches will require this, but as
far as mitigation if DEFINE_STATIC_LOCAL() isn't appropriate, ASan will
vociferously complain.

> Just help me understand: If we add WTF_ANNOTATE_SCOPED_MEMORY_LEAK to all
> DEFINE_STATIC_LOCALs, will it solve our issues? I mean, can we do something
> like:
> 
> #define DEFINE_STATIC_LOCAL ...
>   WTF_ANNOTATE_SCOPED_MEMORY_LEAK; \
>   ...
> 
> ?
> 
> (I'm not intending to object to your fix -- it's important to fix this. I just
> want to look for a better solution.)

The problem you run into with that is that you also have to cover all the call
sites which add to that static singleton. This is what
https://codereview.chromium.org/1482683002/#msg28 tried to explain.

The only other approach I can think of is to change LSan to have it "flood fill"
ignored-reachable memory before it starts doing its leak traversal. I made such
a change locally without having that plug the reported leaks -- I don't profess
to understand exactly why.

[haraken, 2015-12-04 15:18:28]

On 2015/12/04 08:51:01, sof wrote:
> On 2015/12/04 08:20:27, haraken wrote:
> > On 2015/12/04 07:04:08, sof wrote:
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/web/WebKit.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/web/WebKit.cpp:209: Heap::collectAllGarbage();
> > > On 2015/12/04 02:19:51, haraken wrote:
> > > > 
> > > > Help me understand: Why do we need to run destructors of the
persistents?
> To
> > > > suppress LSan leaks, wouldn't it be enough to just clear m_raw's of the
> > > > persistent handles?
> > > > 
> > > > If we don't need to run destructors, we can get rid of the complexity of
> > > > DEFINE_STATIC_LOCAL_NO_REGISTER.
> > > 
> > > All the objects reachable from the persistent needs to be freed (=> a GC
is
> > > needed.) Otherwise they will be reachable via the (shared) heap.
> > 
> > Thanks, makes sense.
> > 
> 
> If you want to see this in action, change
> ThreadState::releaseStaticPersistentNodes() to
> 
>   memset(node->self(), 0, sizeof(PersistentNode));
> 
> instead of persistentRegion()->freePresistentNode(node), along with not GCing
in
> blink::shutdown().
> 
> (Notice that registerAsStaticReference() already marks the persistent object
as
> an LSan-ignored object, so this ought to be a no-op.)
> 
> > What I'm not really happy about is the DEFINE_STATIC_LOCAL_NO_REGISTER --
it's
> a
> > bit too hard for blink developers to use the macro correctly.
> > 
> 
> I agree all the way. All but a few core global caches will require this, but
as
> far as mitigation if DEFINE_STATIC_LOCAL() isn't appropriate, ASan will
> vociferously complain.
> 
> > Just help me understand: If we add WTF_ANNOTATE_SCOPED_MEMORY_LEAK to all
> > DEFINE_STATIC_LOCALs, will it solve our issues? I mean, can we do something
> > like:
> > 
> > #define DEFINE_STATIC_LOCAL ...
> >   WTF_ANNOTATE_SCOPED_MEMORY_LEAK; \
> >   ...
> > 
> > ?
> > 
> > (I'm not intending to object to your fix -- it's important to fix this. I
just
> > want to look for a better solution.)
> 
> The problem you run into with that is that you also have to cover all the call
> sites which add to that static singleton. This is what
> https://codereview.chromium.org/1482683002/#msg28 tried to explain.
> 
> The only other approach I can think of is to change LSan to have it "flood
fill"
> ignored-reachable memory before it starts doing its leak traversal. I made
such
> a change locally without having that plug the reported leaks -- I don't
profess
> to understand exactly why.

Thanks, I now understand the situation very well.

Honestly speaking, I think it would be too hard to ensure that the heap becomes
empty before LSan runs. To ensure the emptiness, we need to destruct all
persistents in a proper order and for persistents that cannot be cleared in that
order, we need to annotate with WTF_ANNOTATE_SCOPED_MEMORY_LEAK. It seems hard.

BTW, would it be an option to just disable LSan on the entire heap (i.e., do not
register Oilpan's heap to LSan)? Given that we have a leak detector, it wouldn't
be a crazy option than we expect.

[sof, 2015-12-04 15:47:51]

On 2015/12/04 15:18:28, haraken wrote:
> On 2015/12/04 08:51:01, sof wrote:
> > On 2015/12/04 08:20:27, haraken wrote:
> > > On 2015/12/04 07:04:08, sof wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> > > > File third_party/WebKit/Source/web/WebKit.cpp (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/40001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/web/WebKit.cpp:209: Heap::collectAllGarbage();
> > > > On 2015/12/04 02:19:51, haraken wrote:
> > > > > 
> > > > > Help me understand: Why do we need to run destructors of the
> persistents?
> > To
> > > > > suppress LSan leaks, wouldn't it be enough to just clear m_raw's of
the
> > > > > persistent handles?
> > > > > 
> > > > > If we don't need to run destructors, we can get rid of the complexity
of
> > > > > DEFINE_STATIC_LOCAL_NO_REGISTER.
> > > > 
> > > > All the objects reachable from the persistent needs to be freed (=> a GC
> is
> > > > needed.) Otherwise they will be reachable via the (shared) heap.
> > > 
> > > Thanks, makes sense.
> > > 
> > 
> > If you want to see this in action, change
> > ThreadState::releaseStaticPersistentNodes() to
> > 
> >   memset(node->self(), 0, sizeof(PersistentNode));
> > 
> > instead of persistentRegion()->freePresistentNode(node), along with not
GCing
> in
> > blink::shutdown().
> > 
> > (Notice that registerAsStaticReference() already marks the persistent object
> as
> > an LSan-ignored object, so this ought to be a no-op.)
> > 
> > > What I'm not really happy about is the DEFINE_STATIC_LOCAL_NO_REGISTER --
> it's
> > a
> > > bit too hard for blink developers to use the macro correctly.
> > > 
> > 
> > I agree all the way. All but a few core global caches will require this, but
> as
> > far as mitigation if DEFINE_STATIC_LOCAL() isn't appropriate, ASan will
> > vociferously complain.
> > 
> > > Just help me understand: If we add WTF_ANNOTATE_SCOPED_MEMORY_LEAK to all
> > > DEFINE_STATIC_LOCALs, will it solve our issues? I mean, can we do
something
> > > like:
> > > 
> > > #define DEFINE_STATIC_LOCAL ...
> > >   WTF_ANNOTATE_SCOPED_MEMORY_LEAK; \
> > >   ...
> > > 
> > > ?
> > > 
> > > (I'm not intending to object to your fix -- it's important to fix this. I
> just
> > > want to look for a better solution.)
> > 
> > The problem you run into with that is that you also have to cover all the
call
> > sites which add to that static singleton. This is what
> > https://codereview.chromium.org/1482683002/#msg28 tried to explain.
> > 
> > The only other approach I can think of is to change LSan to have it "flood
> fill"
> > ignored-reachable memory before it starts doing its leak traversal. I made
> such
> > a change locally without having that plug the reported leaks -- I don't
> profess
> > to understand exactly why.
> 
> Thanks, I now understand the situation very well.
> 
> Honestly speaking, I think it would be too hard to ensure that the heap
becomes
> empty before LSan runs. To ensure the emptiness, we need to destruct all
> persistents in a proper order and for persistents that cannot be cleared in
that
> order, we need to annotate with WTF_ANNOTATE_SCOPED_MEMORY_LEAK. It seems
hard.
> 

Not trivial, but not super-hard to get the property right that no "registered
static persistent" can be touched by finalizers.

> BTW, would it be an option to just disable LSan on the entire heap (i.e., do
not
> register Oilpan's heap to LSan)? Given that we have a leak detector, it
wouldn't
> be a crazy option than we expect.

LSan was still able to detect some leaks when I experimented with that. But, it
would pretty much mean that LSan has no real use with Blink if we consider the
Oilpan heap a black box, I think. 

Anyway, I'll put this one to the side for a couple of days & just think about it
in the background; spent too much time on it this week.

[sof, 2015-12-04 22:07:25]

I realized that a full V8GCController::collectAllGarbageForTesting() was being
used on https://codereview.chromium.org/1482683002/ when identifying the need
for DEFINE_STATIC_LOCAL_NO_REGISTER().

Not doing any v8 GCs here, hence dropping the uses of
DEFINE_STATIC_LOCAL_NO_REGISTER() in all places but one appears possible (
https://codereview.chromium.org/1501673003/ have the linux_chromium_asan_rel_ng
test results.) The exception being the Resource callback handler singleton.

I've removed DEFINE_STATIC_LOCAL_NO_REGISTER() as a result; a separate mechanism
for disabling LSan registration is provided via Threadstate, should that be
needed later.

[sof, 2015-12-05 16:48:03]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-05 16:48:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/100001

[commit-bot: I haz the power, 2015-12-05 18:50:46]

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

[commit-bot: I haz the power, 2015-12-05 18:50:47]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)

[haraken, 2015-12-06 23:56:21]

Thanks for a lot of work on this! I think the CL now has a good shape.

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>, cache,
new ThreadSpecific<CSSTextCacheWrapper>);

Why do we need to use AtomicallyInitializedStaticReference? Since this is
ThreadSpecific, we won't need to acquire a lock to instantiate the singleton. If
we can just use DEFINE_STATIC_LOCAL, we can get rid of CSSTextCacheWrapper.

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:1300:
ThreadState::current()->enterDisabledStaticReferenceRegistrationScope();

Can we add an if(ThreadState::current()) check? Then we can always use
LSAN_LEAK_SCOPE (and remove WTF_ANNOTATE_LEAK_SCOPE).

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:1341: void
ThreadState::enterDisabledStaticReferenceRegistrationScope()

enterRecordStaticPersistentsScope ?

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:1343:
m_disabledStaticPersistentsCount++;

m_recordStaticPersistentsScopeCount ?

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:1346: void
ThreadState::exitDisabledStaticReferenceRegistrationScope()

leaveRecordStaticPersistentsScope ?

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/LeakAnnotations.h (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/LeakAnnotations.h:136: #define
WTF_ANNOTATE_LEAK_REGISTER_STATIC_LOCAL(Type, Object) Object

It's a bit confusing to have four macros to annotate leaks (including
LSAN_LEAK_SCOPE).

- Can we merge LSAN_LEAK_SCOPE and WTF_ANNOTATE_LEAK_SCOPE?
- Can we replace WTF_ANNOTATE_LEAK_IGNORE_OBJECT with LSAN_LEAK_SCOPE?

[sof, 2015-12-07 06:04:58]

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>, cache,
new ThreadSpecific<CSSTextCacheWrapper>);
On 2015/12/06 23:56:21, haraken wrote:
> 
> Why do we need to use AtomicallyInitializedStaticReference? Since this is
> ThreadSpecific, we won't need to acquire a lock to instantiate the singleton.
If
> we can just use DEFINE_STATIC_LOCAL, we can get rid of CSSTextCacheWrapper.

It is needed due to the parser thread also using this singleton cache -- this is
something that was investigated&changed in
https://codereview.chromium.org/864143006 (I confirmed its "It looks" hypothesis
as valid via the https://codereview.chromium.org/1491193002/ tryruns.)

[haraken, 2015-12-07 06:09:24]

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>, cache,
new ThreadSpecific<CSSTextCacheWrapper>);
On 2015/12/07 06:04:58, sof wrote:
> On 2015/12/06 23:56:21, haraken wrote:
> > 
> > Why do we need to use AtomicallyInitializedStaticReference? Since this is
> > ThreadSpecific, we won't need to acquire a lock to instantiate the
singleton.
> If
> > we can just use DEFINE_STATIC_LOCAL, we can get rid of CSSTextCacheWrapper.
> 
> It is needed due to the parser thread also using this singleton cache -- this
is
> something that was investigated&changed in
> https://codereview.chromium.org/864143006 (I confirmed its "It looks"
hypothesis
> as valid via the https://codereview.chromium.org/1491193002/ tryruns.)

Help me understand: Do we want to create two singletons here (one for the main
thread and the other for the parser thread)? Or do we want to create one
singleton but want to point to the singleton by two thread-local storages?

[sof, 2015-12-07 06:21:48]

On 2015/12/07 06:09:24, haraken wrote:
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):
> 
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
> AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>,
cache,
> new ThreadSpecific<CSSTextCacheWrapper>);
> On 2015/12/07 06:04:58, sof wrote:
> > On 2015/12/06 23:56:21, haraken wrote:
> > > 
> > > Why do we need to use AtomicallyInitializedStaticReference? Since this is
> > > ThreadSpecific, we won't need to acquire a lock to instantiate the
> singleton.
> > If
> > > we can just use DEFINE_STATIC_LOCAL, we can get rid of
CSSTextCacheWrapper.
> > 
> > It is needed due to the parser thread also using this singleton cache --
this
> is
> > something that was investigated&changed in
> > https://codereview.chromium.org/864143006 (I confirmed its "It looks"
> hypothesis
> > as valid via the https://codereview.chromium.org/1491193002/ tryruns.)
> 
> Help me understand: Do we want to create two singletons here (one for the main
> thread and the other for the parser thread)? Or do we want to create one
> singleton but want to point to the singleton by two thread-local storages?

The former. A singleton ThreadSpecific<> can be created by (any) thread; one
cache per thread.

[haraken, 2015-12-07 06:23:51]

On 2015/12/07 06:21:48, sof wrote:
> On 2015/12/07 06:09:24, haraken wrote:
> >
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
> > AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>,
> cache,
> > new ThreadSpecific<CSSTextCacheWrapper>);
> > On 2015/12/07 06:04:58, sof wrote:
> > > On 2015/12/06 23:56:21, haraken wrote:
> > > > 
> > > > Why do we need to use AtomicallyInitializedStaticReference? Since this
is
> > > > ThreadSpecific, we won't need to acquire a lock to instantiate the
> > singleton.
> > > If
> > > > we can just use DEFINE_STATIC_LOCAL, we can get rid of
> CSSTextCacheWrapper.
> > > 
> > > It is needed due to the parser thread also using this singleton cache --
> this
> > is
> > > something that was investigated&changed in
> > > https://codereview.chromium.org/864143006 (I confirmed its "It looks"
> > hypothesis
> > > as valid via the https://codereview.chromium.org/1491193002/ tryruns.)
> > 
> > Help me understand: Do we want to create two singletons here (one for the
main
> > thread and the other for the parser thread)? Or do we want to create one
> > singleton but want to point to the singleton by two thread-local storages?
> 
> The former. A singleton ThreadSpecific<> can be created by (any) thread; one
> cache per thread.

Then ThreadSpecific<> makes sense, but do we need
AtomicallyInitializeStaticReference? It looks strange that we need to acquire a
lock to create a thread-local variable.

[sof, 2015-12-07 06:36:09]

On 2015/12/07 06:23:51, haraken wrote:
> On 2015/12/07 06:21:48, sof wrote:
> > On 2015/12/07 06:09:24, haraken wrote:
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
> > > AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>,
> > cache,
> > > new ThreadSpecific<CSSTextCacheWrapper>);
> > > On 2015/12/07 06:04:58, sof wrote:
> > > > On 2015/12/06 23:56:21, haraken wrote:
> > > > > 
> > > > > Why do we need to use AtomicallyInitializedStaticReference? Since this
> is
> > > > > ThreadSpecific, we won't need to acquire a lock to instantiate the
> > > singleton.
> > > > If
> > > > > we can just use DEFINE_STATIC_LOCAL, we can get rid of
> > CSSTextCacheWrapper.
> > > > 
> > > > It is needed due to the parser thread also using this singleton cache --
> > this
> > > is
> > > > something that was investigated&changed in
> > > > https://codereview.chromium.org/864143006 (I confirmed its "It looks"
> > > hypothesis
> > > > as valid via the https://codereview.chromium.org/1491193002/ tryruns.)
> > > 
> > > Help me understand: Do we want to create two singletons here (one for the
> main
> > > thread and the other for the parser thread)? Or do we want to create one
> > > singleton but want to point to the singleton by two thread-local storages?
> > 
> > The former. A singleton ThreadSpecific<> can be created by (any) thread; one
> > cache per thread.
> 
> Then ThreadSpecific<> makes sense, but do we need
> AtomicallyInitializeStaticReference? It looks strange that we need to acquire
a
> lock to create a thread-local variable.

I don't understand - the ThreadSpecific<> singleton object will have to
atomically created if it can be instantiated by any thread.

[haraken, 2015-12-07 06:40:19]

On 2015/12/07 06:36:09, sof wrote:
> On 2015/12/07 06:23:51, haraken wrote:
> > On 2015/12/07 06:21:48, sof wrote:
> > > On 2015/12/07 06:09:24, haraken wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> > > > File third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> > > > third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp:187:
> > > >
AtomicallyInitializedStaticReference(ThreadSpecific<CSSTextCacheWrapper>,
> > > cache,
> > > > new ThreadSpecific<CSSTextCacheWrapper>);
> > > > On 2015/12/07 06:04:58, sof wrote:
> > > > > On 2015/12/06 23:56:21, haraken wrote:
> > > > > > 
> > > > > > Why do we need to use AtomicallyInitializedStaticReference? Since
this
> > is
> > > > > > ThreadSpecific, we won't need to acquire a lock to instantiate the
> > > > singleton.
> > > > > If
> > > > > > we can just use DEFINE_STATIC_LOCAL, we can get rid of
> > > CSSTextCacheWrapper.
> > > > > 
> > > > > It is needed due to the parser thread also using this singleton cache
--
> > > this
> > > > is
> > > > > something that was investigated&changed in
> > > > > https://codereview.chromium.org/864143006 (I confirmed its "It looks"
> > > > hypothesis
> > > > > as valid via the https://codereview.chromium.org/1491193002/ tryruns.)
> > > > 
> > > > Help me understand: Do we want to create two singletons here (one for
the
> > main
> > > > thread and the other for the parser thread)? Or do we want to create one
> > > > singleton but want to point to the singleton by two thread-local
storages?
> > > 
> > > The former. A singleton ThreadSpecific<> can be created by (any) thread;
one
> > > cache per thread.
> > 
> > Then ThreadSpecific<> makes sense, but do we need
> > AtomicallyInitializeStaticReference? It looks strange that we need to
acquire
> a
> > lock to create a thread-local variable.
> 
> I don't understand - the ThreadSpecific<> singleton object will have to
> atomically created if it can be instantiated by any thread.

Sorry, I was confused. You're right.

[sof, 2015-12-07 07:18:20]

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:1300:
ThreadState::current()->enterDisabledStaticReferenceRegistrationScope();
On 2015/12/06 23:56:21, haraken wrote:
> 
> Can we add an if(ThreadState::current()) check? Then we can always use
> LSAN_LEAK_SCOPE (and remove WTF_ANNOTATE_LEAK_SCOPE).

Yes, that would be preferable. But wtf/LeakAnnotations.h is in a tight spot, as
it cannot include platform/heap/, which is why this is split out like it is atm.

So "LeakAnnotations.h" would have to move somewhere into platform/ for any
ThreadState::current() usage to be possible. My preference would really be for
it to be renamed to LeakSanitizer.h (as that's what it only supports) and that
it'd be next to (wtf/)AddressSanitizer.h.

Why do these dependency restrictions have to get in the way so much, some times?
:)

[haraken, 2015-12-07 07:23:36]

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:1300:
ThreadState::current()->enterDisabledStaticReferenceRegistrationScope();
On 2015/12/07 07:18:19, sof wrote:
> On 2015/12/06 23:56:21, haraken wrote:
> > 
> > Can we add an if(ThreadState::current()) check? Then we can always use
> > LSAN_LEAK_SCOPE (and remove WTF_ANNOTATE_LEAK_SCOPE).
> 
> Yes, that would be preferable. But wtf/LeakAnnotations.h is in a tight spot,
as
> it cannot include platform/heap/, which is why this is split out like it is
atm.
> 
> So "LeakAnnotations.h" would have to move somewhere into platform/ for any
> ThreadState::current() usage to be possible. My preference would really be for
> it to be renamed to LeakSanitizer.h (as that's what it only supports) and that
> it'd be next to (wtf/)AddressSanitizer.h.
> 
> Why do these dependency restrictions have to get in the way so much, some
times?
> :)

Yeah, the dependency issue between platform/ and wtf/ should be resolved
throughout the Onion Soup project. yutak@ is looking into it.

At the moment, let's do whatever makes sense at the moment. If it's hard to move
LeakSanitizer.h to platform/ (because LeakSanitizer is used in wtf/), you can
pass a function pointer for ThreadState::current() to wtf/LeakSanitizer.h.

[sof, 2015-12-07 10:37:20]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-07 10:37:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/120001

[sof, 2015-12-07 10:51:42]

On 2015/12/07 07:23:36, haraken wrote:
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/platform/heap/Handle.h (right):
> 
>
https://codereview.chromium.org/1491253004/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/platform/heap/Handle.h:1300:
> ThreadState::current()->enterDisabledStaticReferenceRegistrationScope();
> On 2015/12/07 07:18:19, sof wrote:
> > On 2015/12/06 23:56:21, haraken wrote:
> > > 
> > > Can we add an if(ThreadState::current()) check? Then we can always use
> > > LSAN_LEAK_SCOPE (and remove WTF_ANNOTATE_LEAK_SCOPE).
> > 
> > Yes, that would be preferable. But wtf/LeakAnnotations.h is in a tight spot,
> as
> > it cannot include platform/heap/, which is why this is split out like it is
> atm.
> > 
> > So "LeakAnnotations.h" would have to move somewhere into platform/ for any
> > ThreadState::current() usage to be possible. My preference would really be
for
> > it to be renamed to LeakSanitizer.h (as that's what it only supports) and
that
> > it'd be next to (wtf/)AddressSanitizer.h.
> > 
> > Why do these dependency restrictions have to get in the way so much, some
> times?
> > :)
> 
> Yeah, the dependency issue between platform/ and wtf/ should be resolved
> throughout the Onion Soup project. yutak@ is looking into it.
> 
> At the moment, let's do whatever makes sense at the moment. If it's hard to
move
> LeakSanitizer.h to platform/ (because LeakSanitizer is used in wtf/), you can
> pass a function pointer for ThreadState::current() to wtf/LeakSanitizer.h.

Moved around definitions to hopefully make it tidier (+ renamed some static
helper methods on ThreadState, as requested.)

[commit-bot: I haz the power, 2015-12-07 11:53:22]

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

[commit-bot: I haz the power, 2015-12-07 11:53:23]

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

[haraken, 2015-12-07 14:46:10]

LGTM!

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp:71:
LEAK_SANITIZER_IGNORE_OBJECT(sheet.get());

This is the only use case of WTF_ANNOTATE_LEAKING_OBJECT_PTR. Shall we just
replace it with LEAK_SANITIZER_DISABLED_SCOPE (and remove
WTF_ANNOTATE_LEAKING_OBJECT_PTR)?

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:194:
ThreadState::current()->registerStaticPersistentNode(m_persistentNode);

Add ASSERT(ThreadState::current()).

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:566:
ThreadState::current()->registerStaticPersistentNode(m_persistentNode);

Ditto.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:1301: class
LeakSanitizerDisableScope {

Add STACK_ALLOCATED.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:1336:
persistentRegion()->freePersistentNode(node);

After calling this, all static Persistent handles become stale pointers (i.e.,
pointing to freed persistent nodes). This is intentional, right?

I think this would be okay because this happens only when we shut down a
renderer with LSan enabled. It won't cause any practical issue.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/LeakAnnotations.h (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/LeakAnnotations.h:88: // result being a tidied
heap that the LeakSanitizer can then scan to report real leaks.

It's not really nice to have an Oilpan-related comment in wtf/. Can we move (the
Oilpan part of) this comment to ThreadState.h?

[sof, 2015-12-07 15:09:28]

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp:71:
LEAK_SANITIZER_IGNORE_OBJECT(sheet.get());
On 2015/12/07 14:46:10, haraken wrote:
> 
> This is the only use case of WTF_ANNOTATE_LEAKING_OBJECT_PTR. Shall we just
> replace it with LEAK_SANITIZER_DISABLED_SCOPE (and remove
> WTF_ANNOTATE_LEAKING_OBJECT_PTR)?

It is also used by Persistent*::registerAsStaticReference(), but those can be
avoided.

Actually, I'm not so sure we need to mark this object as individually ignored
here as all of these UA sheet objects belong to the
CSSDefaultStyleSheet::instance() singleton. Will investigate.

[sof, 2015-12-07 16:59:17]

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp:71:
LEAK_SANITIZER_IGNORE_OBJECT(sheet.get());
On 2015/12/07 15:09:28, sof wrote:
> On 2015/12/07 14:46:10, haraken wrote:
> > 
> > This is the only use case of WTF_ANNOTATE_LEAKING_OBJECT_PTR. Shall we just
> > replace it with LEAK_SANITIZER_DISABLED_SCOPE (and remove
> > WTF_ANNOTATE_LEAKING_OBJECT_PTR)?
> 
> It is also used by Persistent*::registerAsStaticReference(), but those can be
> avoided.
> 
> Actually, I'm not so sure we need to mark this object as individually ignored
> here as all of these UA sheet objects belong to the
> CSSDefaultStyleSheet::instance() singleton. Will investigate.

Ah, this is just right as it is. The StyleSheetContents objects are reachable by
other objects, hence marking them as ignored is what's wanted. And marking the
object itself as ignored, rather than disabling LSan just for the duration of
its (initial) construction, will also let us not flag leaks against anything
that the object exclusively refers to at shutdown.

Same goes for Persistent*::registerAsStaticReference() uses, they're also
needed. Hence leaving this as it is.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/Handle.h (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:194:
ThreadState::current()->registerStaticPersistentNode(m_persistentNode);
On 2015/12/07 14:46:10, haraken wrote:
> 
> Add ASSERT(ThreadState::current()).

Done.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/Handle.h:1301: class
LeakSanitizerDisableScope {
On 2015/12/07 14:46:10, haraken wrote:
> 
> Add STACK_ALLOCATED.

Done.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/heap/ThreadState.cpp (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/heap/ThreadState.cpp:1336:
persistentRegion()->freePersistentNode(node);
On 2015/12/07 14:46:10, haraken wrote:
> 
> After calling this, all static Persistent handles become stale pointers (i.e.,
> pointing to freed persistent nodes). This is intentional, right?
> 
> I think this would be okay because this happens only when we shut down a
> renderer with LSan enabled. It won't cause any practical issue.

Intentional to handle it that way; the sweeping phase will poison what the
persistent points to via m_raw, so any attempted uses will be caught.

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/LeakAnnotations.h (right):

https://codereview.chromium.org/1491253004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/LeakAnnotations.h:88: // result being a tidied
heap that the LeakSanitizer can then scan to report real leaks.
On 2015/12/07 14:46:10, haraken wrote:
> 
> It's not really nice to have an Oilpan-related comment in wtf/. Can we move
(the
> Oilpan part of) this comment to ThreadState.h?

This template needs to live here as StdLibExtras.h is defined in terms of it.
Hence it would be quite unusual to leave out any real motivation as to why this
whole wodge of extra complexity is really needed.

[sof, 2015-12-07 18:37:00]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And report no leaks over, hopefully.

A couple of exceptions to the rule of registering these
singletons are needed -- singletons that we end up touching
as part of doing the leak-tidying GCs cannot be released.
These are instead not registered and exempted from the LSan
root set.

R=
BUG=
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And report no leaks over, hopefully.

A couple of exceptions to the rule of registering these
singletons are needed -- singletons that we end up touching
as part of doing the leak-tidying GCs cannot be released.
These are instead not registered and exempted from the LSan
root set.

R=haraken
BUG=567257
==========

[sof, 2015-12-07 18:44:58]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And report no leaks over, hopefully.

A couple of exceptions to the rule of registering these
singletons are needed -- singletons that we end up touching
as part of doing the leak-tidying GCs cannot be released.
These are instead not registered and exempted from the LSan
root set.

R=haraken
BUG=567257
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:
  
 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

[sof, 2015-12-07 18:51:24]

Going ahead with this one in its current shape.

[sof, 2015-12-07 18:51:33]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-12-07 18:51:34]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1491253004/#ps140001
(title: "add some asserts")

[commit-bot: I haz the power, 2015-12-07 18:52:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/140001

[commit-bot: I haz the power, 2015-12-07 21:45:53]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:
  
 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:
  
 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

[commit-bot: I haz the power, 2015-12-07 21:45:54]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-12-07 21:46:54]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:
  
 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/5699ffefdcb17f5150f0d9342d21f73b3e8958b7
Cr-Commit-Position: refs/heads/master@{#363591}
==========

[commit-bot: I haz the power, 2015-12-07 21:46:56]

Patchset 8 (id:??) landed as
https://crrev.com/5699ffefdcb17f5150f0d9342d21f73b3e8958b7
Cr-Commit-Position: refs/heads/master@{#363591}

[haraken, 2015-12-08 01:53:08]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1505003003/ by haraken@chromium.org.

The reason for reverting is: This caused compile errors on Oilpan + no-LSan
bots.

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Oilpan/...

It doesn't seem to be trivial to fix the error, so let me revert.
.

[sof, 2015-12-08 06:41:25]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/5699ffefdcb17f5150f0d9342d21f73b3e8958b7
Cr-Commit-Position: refs/heads/master@{#363591}
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/5699ffefdcb17f5150f0d9342d21f73b3e8958b7
Cr-Commit-Position: refs/heads/master@{#363591}
==========

[sof, 2015-12-08 06:43:23]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-08 06:43:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/160001

[sof, 2015-12-08 07:41:08]

mea culpa; non-LSan compilation issue addressed.

[haraken, 2015-12-08 07:49:08]

LGTM

[commit-bot: I haz the power, 2015-12-08 09:09:59]

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

[commit-bot: I haz the power, 2015-12-08 09:10:00]

Dry run: This issue passed the CQ dry run.

[sof, 2015-12-08 09:10:54]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/5699ffefdcb17f5150f0d9342d21f73b3e8958b7
Cr-Commit-Position: refs/heads/master@{#363591}
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

[sof, 2015-12-08 10:05:10]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-12-08 10:05:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/160001

[commit-bot: I haz the power, 2015-12-08 10:13:51]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

[commit-bot: I haz the power, 2015-12-08 10:13:53]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-12-08 10:14:57]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

[commit-bot: I haz the power, 2015-12-08 10:14:59]

Patchset 9 (id:??) landed as
https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}

[Lambros, 2015-12-08 22:38:51]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/1502153005/ by lambroslambrou@chromium.org.

The reason for reverting is: Might have caused compile failures on official Win
builders:
https://chromegw.corp.google.com/i/official.desktop/builders/win/builds/268/s...
https://chromegw.corp.google.com/i/official.desktop/builders/win64/builds/264...

FAILED: ninja -t msvc -e environment.x64 --
"C:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
/showIncludes /FC
@obj\third_party\WebKit\Source\core\html\webcore_html_2.HTMLTableElement.obj.rsp
/c ..\..\third_party\WebKit\Source\core\html\HTMLTableElement.cpp
/Foobj\third_party\WebKit\Source\core\html\webcore_html_2.HTMLTableElement.obj
/Fdobj\third_party\WebKit\Source\core\webcore_html_2.cc.pdb 
c:\b\build\slave\win64\build\src\third_party\webkit\source\core\html\htmltableelement.cpp(413)
: error C2039: 'registerAsStaticReference' : is not a member of
'blink::Persistent<blink::StylePropertySet>'
c:\b\build\slave\win64\build\src\third_party\webkit\source\core\html\htmltableelement.cpp(420)
: error C2039: 'registerAsStaticReference' : is not a member of
'blink::Persistent<blink::StylePropertySet>'
c:\b\build\slave\win64\build\src\third_party\webkit\source\core\html\htmltableelement.cpp(423)
: error C2039: 'registerAsStaticReference' : is not a member of
'blink::Persistent<blink::StylePropertySet>'
c:\b\build\slave\win64\build\src\third_party\webkit\source\core\html\htmltableelement.cpp(520)
: error C2039: 'registerAsStaticReference' : is not a member of
'blink::Persistent<blink::StylePropertySet>'
c:\b\build\slave\win64\build\src\third_party\webkit\source\core\html\htmltableelement.cpp(523)
: error C2039: 'registerAsStaticReference' : is not a member of
'blink::Persistent<blink::StylePropertySet>'
ninja: build stopped: subcommand failed.
.

[sof, 2015-12-09 06:19:45]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

[sof, 2015-12-09 06:21:00]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

(Reland of r363780.)

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

[sof, 2015-12-09 06:22:59]

relanding; i have no access to the build logs of the revert, but that branch
ought to have reverted ps#8, if anything. 

Trunk was compiling just fine with r363780.

[sof, 2015-12-09 06:23:10]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-12-09 06:23:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491253004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491253004/160001

[commit-bot: I haz the power, 2015-12-09 06:27:42]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

(Reland of r363780.)

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

(Reland of r363780.)

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

[commit-bot: I haz the power, 2015-12-09 06:27:44]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-12-09 06:28:23]

Description was changed from

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

(Reland of r363780.)

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}
==========

to

==========
Release Oilpan heap singletons prior to LSan leak detection.

Make Oilpan and LSan cooperate better. As Persistent<> references
created via DEFINE_STATIC_LOCAL() and similar will be reachable to
LSan's leak detection pass, the objects they refer to outside of
the Oilpan heap will be reported as leaking.

(This is in contrast to what happens in the non-Oilpan setting,
where the (leaked) pointer created via DEFINE_STATIC_LOCAL() is
stored in a local "static"; such non-global references are not
considered roots to LSan and hence the objects reachable from
those will not be reported as leaking.)

Address the problem on the Oilpan side by having such "static"
Persistent<>ly-held singletons be registered and tracked such
that we're able to release them all just before shutting down
and performing an extra round of GCs. Leaving a cleaner heap
for LSan to work over. And to report no leaks over, ideally.

As part of the changes needed to support this for Oilpan,
wtf/LeakAnnotations.h offerings has been renamed and changed
a bit:

 * WTF_ANNOTATE_MEMORY_LEAK_SCOPE => LEAK_SANITIZER_DISABLED_SCOPE.
   (but see LeakAnnotations.h for macro to use local to wtf/.)
 * WTF_ANNOTATE_IGNORE_OBJECT_PTR => LEAK_SANITIZER_IGNORE_OBJECT.
 * LEAK_SANITIZER_REGISTER_STATIC_LOCAL().

(Reland of r363780.)

R=haraken
BUG=567257

Committed: https://crrev.com/6918d00fae1ab739f89393378fa4adddabacafd2
Cr-Commit-Position: refs/heads/master@{#363780}

Committed: https://crrev.com/2e088a7ecaf6c26f44c309b7bab507aeddb41365
Cr-Commit-Position: refs/heads/master@{#363994}
==========

[commit-bot: I haz the power, 2015-12-09 06:28:25]

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