Fix memory leak with ScriptedIdleTaskController

Fix memory leak with ScriptedIdleTaskController

If there is an untriggered idle task when the document is gone, it
introduces memory leak because IdleRequestCallbackWrapper keeps a
reference to the ScriptedIdleTaskController, making it live longer
than the document.

This patch fixes the issue by letting IdleRequestCallbackWrapper
store a WeakPersistent instead.

BUG=595155
Review-Url: https://codereview.chromium.org/2618893002
Cr-Commit-Position: refs/heads/master@{#441929}
Committed: https://chromium.googlesource.com/chromium/src/+/114c62b99fd57ac1cf7c0da1150608fd5f68d3bf

[Xiaocheng, 2017-01-06 08:03:43]

Description was changed from

==========
Fix memory leak with ScriptedIdleTaskController

BUG=595155
==========

to

==========
Fix memory leak with ScriptedIdleTaskController

If there is an untriggered idle task when the document is gone, it
introduces memory leak because IdleRequestCallbackWrapper keeps a
reference to the ScriptedIdleTaskController, making it live longer
than the document.

This patch fixes the issue by letting IdleRequestCallbackWrapper
store a WeakPtr instead.

BUG=595155
==========

[Xiaocheng, 2017-01-06 08:16:29]

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

[commit-bot: I haz the power, 2017-01-06 08:16:39]

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

[Xiaocheng, 2017-01-06 09:27:40]

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

[Xiaocheng, 2017-01-06 09:27:41]

PTAL.

The failure on mac_chromium_rel_ng doesn't seem relevant. I'll retry later.

[sof, 2017-01-06 09:31:04]

https://codereview.chromium.org/2618893002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ScriptedIdleTaskController.cpp (right):

https://codereview.chromium.org/2618893002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ScriptedIdleTaskController.cpp:66:
WTF::WeakPtr<ScriptedIdleTaskController> m_controller;
WeakPersistent<> is used to hold onto GC-managed objects weakly.

[Xiaocheng, 2017-01-06 09:55:32]

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

[commit-bot: I haz the power, 2017-01-06 09:55:45]

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

[Xiaocheng, 2017-01-06 09:56:28]

Updated with WeakPersistent. PTAL.

https://codereview.chromium.org/2618893002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ScriptedIdleTaskController.cpp (right):

https://codereview.chromium.org/2618893002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ScriptedIdleTaskController.cpp:66:
WTF::WeakPtr<ScriptedIdleTaskController> m_controller;
On 2017/01/06 at 09:31:04, sof wrote:
> WeakPersistent<> is used to hold onto GC-managed objects weakly.

Done.

Thanks!

[Xiaocheng, 2017-01-06 09:56:44]

Description was changed from

==========
Fix memory leak with ScriptedIdleTaskController

If there is an untriggered idle task when the document is gone, it
introduces memory leak because IdleRequestCallbackWrapper keeps a
reference to the ScriptedIdleTaskController, making it live longer
than the document.

This patch fixes the issue by letting IdleRequestCallbackWrapper
store a WeakPtr instead.

BUG=595155
==========

to

==========
Fix memory leak with ScriptedIdleTaskController

If there is an untriggered idle task when the document is gone, it
introduces memory leak because IdleRequestCallbackWrapper keeps a
reference to the ScriptedIdleTaskController, making it live longer
than the document.

This patch fixes the issue by letting IdleRequestCallbackWrapper
store a WeakPersistent instead.

BUG=595155
==========

[sof, 2017-01-06 10:05:28]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2017-01-06 10:05:29]

lgtm

[esprehn, 2017-01-06 10:45:51]

Do we need null checks now? I thought a WeakPersistent can become null when it's
colected so now the code that uses the callback wrapper needs to check.

[sof, 2017-01-06 10:48:09]

On 2017/01/06 10:45:51, esprehn wrote:
> Do we need null checks now? I thought a WeakPersistent can become null when
it's
> colected so now the code that uses the callback wrapper needs to check.

The *Fired() static methods here do check that.

[haraken, 2017-01-06 11:02:19]

LGTM

[Xiaocheng, 2017-01-06 11:05:39]

The CQ bit was unchecked by xiaochengh@chromium.org

[Xiaocheng, 2017-01-06 11:05:49]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2017-01-06 11:05:49]

Thanks for the review!

[commit-bot: I haz the power, 2017-01-06 11:06:04]

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

[commit-bot: I haz the power, 2017-01-06 13:16:01]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483700749375200,
"parent_rev": "c771336bc3e7a8a29e4e0b6268f4b085c5e2da3b", "commit_rev":
"114c62b99fd57ac1cf7c0da1150608fd5f68d3bf"}

[commit-bot: I haz the power, 2017-01-06 13:16:29]

Description was changed from

==========
Fix memory leak with ScriptedIdleTaskController

If there is an untriggered idle task when the document is gone, it
introduces memory leak because IdleRequestCallbackWrapper keeps a
reference to the ScriptedIdleTaskController, making it live longer
than the document.

This patch fixes the issue by letting IdleRequestCallbackWrapper
store a WeakPersistent instead.

BUG=595155
==========

to

==========
Fix memory leak with ScriptedIdleTaskController

If there is an untriggered idle task when the document is gone, it
introduces memory leak because IdleRequestCallbackWrapper keeps a
reference to the ScriptedIdleTaskController, making it live longer
than the document.

This patch fixes the issue by letting IdleRequestCallbackWrapper
store a WeakPersistent instead.

BUG=595155

Review-Url: https://codereview.chromium.org/2618893002
Cr-Commit-Position: refs/heads/master@{#441929}
Committed:
https://chromium.googlesource.com/chromium/src/+/114c62b99fd57ac1cf7c0da11506...
==========

[commit-bot: I haz the power, 2017-01-06 13:16:31]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/114c62b99fd57ac1cf7c0da11506...