Move ScriptStreamerThread::shutdown to CoreInitializer::shutdown

Move ScriptStreamerThread::shutdown to CoreInitializer::shutdown

ScriptStreamerThread::initialize is called in CoreInitializer::initialize.
Thus ScriptStreamerThread::shutdown should be called in CoreInitializer::shutdown.

BUG=593269
Committed: https://crrev.com/ce367ff2ad0cd2d99ded533b1260028fa57e6554
Cr-Commit-Position: refs/heads/master@{#380309}

[haraken, 2016-03-09 12:55:52]

haraken@chromium.org changed reviewers:
+ sigbjornf@opera.com

[haraken, 2016-03-09 12:55:53]

PTAL

[haraken, 2016-03-09 13:02:37]

https://codereview.chromium.org/1776183004/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebKit.cpp (right):

https://codereview.chromium.org/1776183004/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebKit.cpp:185: if (ThreadState* threadState =
ThreadState::current()) {

Sigbjorn: Not related to this CL, do you think we can move this code to
Heap::shutdownMainThread?

I don't fully understand the part "a full v8 GC cycle is needed to flush out all
garbage" because no V8 GC will be scheduled after this point. V8 is shut down at
line 205.

Anyways, if we call Heap::collectAllGarbage in Heap::shutdownMainThread, it's
called after unregisterTraceDOMWrappers is called. This means that the Oilpan GC
will ignore all references from V8 wrappers, which is a desirable thing for us.

[sof, 2016-03-09 16:53:20]

lgtm, makes sense - balanced calls.

https://codereview.chromium.org/1776183004/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebKit.cpp (right):

https://codereview.chromium.org/1776183004/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebKit.cpp:185: if (ThreadState* threadState =
ThreadState::current()) {
On 2016/03/09 13:02:36, haraken wrote:
> 
> Sigbjorn: Not related to this CL, do you think we can move this code to
> Heap::shutdownMainThread?
> 
> I don't fully understand the part "a full v8 GC cycle is needed to flush out
all
> garbage" because no V8 GC will be scheduled after this point. V8 is shut down
at
> line 205.
> 
> Anyways, if we call Heap::collectAllGarbage in Heap::shutdownMainThread, it's
> called after unregisterTraceDOMWrappers is called. This means that the Oilpan
GC
> will ignore all references from V8 wrappers, which is a desirable thing for
us.

The comment is just out of date -- getting all the LSan-reported leaks flushed
out needed some attempts to get it right & it used to call
RequestGarbageCollectionForTesting() for some time to trigger a full v8 GC wash.

=> It can plausibly be moved.

[haraken, 2016-03-09 17:14:42]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2016-03-09 17:14:54]

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

[commit-bot: I haz the power, 2016-03-09 17:16:50]

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

[commit-bot: I haz the power, 2016-03-09 17:16:51]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[haraken, 2016-03-09 23:54:20]

The CQ bit was checked by haraken@chromium.org

[haraken, 2016-03-09 23:54:20]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com
Link to the patchset: https://codereview.chromium.org/1776183004/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-03-09 23:54:53]

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

[commit-bot: I haz the power, 2016-03-10 02:20:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-10 02:22:00]

Description was changed from

==========
Move ScriptStreamerThread::shutdown to CoreInitializer::shutdown

ScriptStreamerThread::initialize is called in CoreInitializer::initialize.
Thus ScriptStreamerThread::shutdown should be called in
CoreInitializer::shutdown.

BUG=593269
==========

to

==========
Move ScriptStreamerThread::shutdown to CoreInitializer::shutdown

ScriptStreamerThread::initialize is called in CoreInitializer::initialize.
Thus ScriptStreamerThread::shutdown should be called in
CoreInitializer::shutdown.

BUG=593269

Committed: https://crrev.com/ce367ff2ad0cd2d99ded533b1260028fa57e6554
Cr-Commit-Position: refs/heads/master@{#380309}
==========

[commit-bot: I haz the power, 2016-03-10 02:22:01]

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