The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2887dc41aa23ad6e81c0e61793c518a6db1a1373

commit 2887dc41aa23ad6e81c0e61793c518a6db1a1373
Author: mlippautz <mlippautz@chromium.org>
Date: Tue Nov 22 13:41:40 2016

[wrapper-tracing] Temporarily mark tests as leaking

BUG= chromium:667746 
NOTRY=true

Review-Url: https://codereview.chromium.org/2525643002
Cr-Commit-Position: refs/heads/master@{#433847}

[modify] https://crrev.com/2887dc41aa23ad6e81c0e61793c518a6db1a1373/third_party/WebKit/LayoutTests/LeakExpectations

Tracing of the last GC shows that we have start with a document A in the root set, trace some parts on blink side, and then find another document B on the V8 side.

ScriptWrappableVisitor::TracePrologue
Root: 0x171763330868
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::RemotePlayback]: 0x171763330868
Root: 0x171763336008
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DOMWindow]: 0x171763336008
Root: 0x171763336008
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DOMWindow]: 0x171763336008
Root: 0x321fd7a7e20
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLDocument]: 0x321fd7a7e20
Root: 0x2dd176c29af0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Internals]: 0x2dd176c29af0
  RemotePlayback(0x171763330868)::traceWrappers: m_availabilityCallbacks: 0x3eb3d8413d50
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::RemotePlaybackAvailabilityCallback]: 0x3eb3d8413d50
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::StyleEngine]: 0x171763336278
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Supplement<blink::Document>]: 0x171763336648
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a89f0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a89f0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DocumentStyleSheetCollection]: 0x2dd176c29760
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a8a58
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a8ac0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a7e20
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a89f0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a8ac0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a89f0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a8a58
Root: 0x17176332d5d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DOMWindow]: 0x17176332d5d0
Root: 0x17176332d5d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DOMWindow]: 0x17176332d5d0
Root: 0x321fd7a2538
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLDocument]: 0x321fd7a2538
Root: 0x3b80ea264bc8
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Location]: 0x3b80ea264bc8
Root: 0x3eb3d8412cb8
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DOMStringList]: 0x3eb3d8412cb8
Root: 0x2dd176c24cf0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Internals]: 0x2dd176c24cf0
Root: 0x171763330868
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::RemotePlayback]: 0x171763330868
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::StyleEngine]: 0x17176332d840
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::LiveNodeListBase]: 0x2dd176c25088
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::LiveNodeListBase]: 0x2dd176c25640
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Supplement<blink::Document>]: 0x17176332dc10
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3108
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3168
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::NodeRareData]: 0x3eb3d8412a18
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::DocumentStyleSheetCollection]: 0x2dd176c249e0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a2538
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3168
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3640
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a2538
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3108
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::NodeListsNodeData]: 0x3eb3d8412a40
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3238
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a35a0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a3168
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a35f0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a7d68
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a7d68
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a3168
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a35f0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3288
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3528
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a3168
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3640
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a7dd0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a7dd0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a3640
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a32f8
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a32f8
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3238
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3348
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a34d8
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a35a0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a7d68
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a3288
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3288
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3398
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3460
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3528
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3348
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3410
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3410
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a34d8
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::ContainerNode]: 0x321fd7a31d0
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3398
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::Node]: 0x321fd7a3460
Root: 0x321fd7a3398
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLScriptElement]: 0x321fd7a3398
Root: 0x321fd7a7d68
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLPreElement]: 0x321fd7a7d68
Root: 0x321fd7a3640
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLBodyElement]: 0x321fd7a3640
Root: 0x2dd176c25630
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLCollection]: 0x2dd176c25630
Root: 0x2dd176c25078
void blink::WrapperVisitor::traceWrappers(const T *) const [T = blink::HTMLCollection]: 0x2dd176c25078
[1:0xb4f4bf2a000]      333 ms: Mark-sweep 3.1 (8.5) -> 3.1 (8.5) MB, 29.7 / 0.2 ms  testing GC in old space requested
#LEAK - renderer pid 32869 ({"numberOfLiveActiveDOMObjects":[2,4],"numberOfLiveDocuments":[1,2],"numberOfLiveNodes":[4,98],"numberOfLiveResources":[0,3]})

So, I see that even in the last round of GC RemotePlayback is still alive. Since this playback thingy and its callback are alive, I assume that its global object and the tied document are also kept alive, which is fine. 

The question rather is whether the ActiveScriptWrappable should be somehow canceled (= all callbacks should be canceled) when we navigate away from the page. If yes, than that's a but in RemotePlayback. Otherwise, the leak detector needs to wait/cancel for pending callbacks.

wdyt?

Maybe we need to export this hack (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp?sq=package:chromium&dr=CSs&l=176) to the traceWrapper world (ScriptWrappableVisitor).

A real fix is, as you mentioned, that we need to make RemotePlayback get cancelled when the execution context is gone.

Do we need to assume that these callbacks don't necessarily fire? 

If we could assume that they always fire at some point, then the mess should actually untangle itself (through tracing) and we would only need to block until this happens in the detector. If, however, the callbacks don't always fire (which is what I think is rather the case), then we need to break this cycle with a hack like you mentioned.

I'm not sure about the specific case of RemotePlayback, but historically we had a bunch of cases where hasPendingActivity doesn't return false in a finite time period. So we added the hack to forcibly make all hasPendingActivity's return false after the execution context gets detached.

We should really fix the behavior with the wrapper tracing (that's one of the goals of the wrapper tracing), but at the moment, I'd suggest just porting the hack to the wrapper tracing to avoid changing existing behavior.

(Note: Even if the callback is fired at some later point, it doesn't make much sense because the callback won't run after the execution context gets detached. In that sense, maybe what we should really do is to make RemotePlayback inherit from ContextLifecycleObserver and clear m_availabilityCallbacks in RemotePlayback::contextDestroyed. But let's revisit the issue after stabilizing the wrapper tracing :-)

Not sure if this is known, but Sheriff-o-Matic shows that media/remoteplayback/prompt-twice-throws.html started consistently failing on WebKit Linux Trusty Leak bot around https://crrev.com/83e8ca3114f774182150be1bd2e4d85fd0b2c7ec..e1f92e0175c86cf30b0bc1c0b642f489f75058ba.  I see that this CL range includes r434171 from mlippautz@.

Will have another look today. The CL was going to fix this issue and I verified locally that leak detector was fine with it.

fyi: The bot is failing because I removed the Leak expectations and tried to fix the underlying issue. Apparently one test (media/remoteplayback/prompt-twice-throws.html
) is still remaining.

Fix in flight: https://codereview.chromium.org/2531563002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68e0614944ce7c45de33bc8722df414eddf7a0a8

commit 68e0614944ce7c45de33bc8722df414eddf7a0a8
Author: mlippautz <mlippautz@chromium.org>
Date: Thu Nov 24 19:43:41 2016

[wrapper-tracing] Fix leak in RemotePlayback round 2

BUG= chromium:667746 

Review-Url: https://codereview.chromium.org/2531563002
Cr-Commit-Position: refs/heads/master@{#434382}

[modify] https://crrev.com/68e0614944ce7c45de33bc8722df414eddf7a0a8/third_party/WebKit/Source/modules/remoteplayback/RemotePlayback.cpp

 Issue 668479  has been merged into this issue.