Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337, 257943, 612563
Committed: https://crrev.com/11c0c210570c67f7b32ebc706102c9e8833cc9a1
Cr-Commit-Position: refs/heads/master@{#395727}

[scottmg, 2016-05-18 22:57:03]

Description was changed from

==========
Remove dependency of content/browser/power_save* on
content/public/browser/browser_thread.h

BUG=612337
==========

to

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

BUG=612337,257943,612563
==========

[scottmg, 2016-05-18 23:00:50]

Description was changed from

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

BUG=612337,257943,612563
==========

to

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563
==========

[scottmg, 2016-05-19 23:52:26]

scottmg@chromium.org changed reviewers:
+ asargent@chromium.org, hashimoto@chromium.org, nick@chromium.org,
sergeyu@chromium.org

[scottmg, 2016-05-19 23:52:27]

sergeyu: chrome/browser/media
hashimoto: components/drive
nick: content/
asargent: extensions/

[asargent_no_longer_on_chrome, 2016-05-20 00:07:36]

extensions lgtm

[ncarter (slow), 2016-05-23 17:20:39]

lgtm

[hashimoto, 2016-05-24 06:02:43]

components/drive lgtm

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
File content/browser/power_save_blocker_factory.cc (right):

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
content/browser/power_save_blocker_factory.cc:18:
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
How about using BrowserThread::GetBlockingPool()->GetSequencedTaskRunner()
instead of the FILE thread?

The FILE thread was declared dead long ago.
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/OaA-9rnjOqI/H0cDC...

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
File content/browser/power_save_blocker_impl.h (right):

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
content/browser/power_save_blocker_impl.h:59:
scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
This task runner has nothing to do with files.
How about renaming this to something like blocking_task_runner_?

This task runner is used only by power_save_blocker_x11.cc and it's using this
task runner to perform D-Bus method calls.

[scottmg, 2016-05-24 17:17:34]

Description was changed from

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563
==========

to

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563
==========

[scottmg, 2016-05-24 17:17:35]

scottmg@chromium.org changed reviewers:
+ tommi@chromium.org
- sergeyu@chromium.org

[scottmg, 2016-05-24 17:18:04]

Thanks.

sergeyu is ooo, -> tommi for 	chrome/browser/media/cast_transport_host_filter.cc

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
File content/browser/power_save_blocker_factory.cc (right):

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
content/browser/power_save_blocker_factory.cc:18:
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
On 2016/05/24 06:02:42, hashimoto wrote:
> How about using BrowserThread::GetBlockingPool()->GetSequencedTaskRunner()
> instead of the FILE thread?
> 
> The FILE thread was declared dead long ago.
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/OaA-9rnjOqI/H0cDC...

Done.

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
File content/browser/power_save_blocker_impl.h (right):

https://codereview.chromium.org/1991593002/diff/180001/content/browser/power_...
content/browser/power_save_blocker_impl.h:59:
scoped_refptr<base::SequencedTaskRunner> file_task_runner_;
On 2016/05/24 06:02:42, hashimoto wrote:
> This task runner has nothing to do with files.
> How about renaming this to something like blocking_task_runner_?
> 
> This task runner is used only by power_save_blocker_x11.cc and it's using this
> task runner to perform D-Bus method calls.

Done.

[tommi (sloooow) - chröme, 2016-05-24 18:22:26]

lgtm for chrome/browser/media/

[scottmg, 2016-05-24 18:33:22]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-05-24 18:33:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org, hashimoto@chromium.org, nick@chromium.org,
tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/1991593002/#ps220001
(title: ".")

[commit-bot: I haz the power, 2016-05-24 18:34:06]

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

[commit-bot: I haz the power, 2016-05-24 18:54:55]

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

[commit-bot: I haz the power, 2016-05-24 18:54:56]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[scottmg, 2016-05-24 19:30:32]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-05-24 19:30:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
hashimoto@chromium.org, nick@chromium.org, asargent@chromium.org,
tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/1991593002/#ps240001
(title: "mac")

[commit-bot: I haz the power, 2016-05-24 19:31:02]

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

[commit-bot: I haz the power, 2016-05-24 23:21:03]

Description was changed from

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563
==========

to

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563
==========

[commit-bot: I haz the power, 2016-05-24 23:21:04]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-05-24 23:22:27]

Description was changed from

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563
==========

to

==========
Pass SequencedTaskRunner to PowerSaveBlocker for ui/file operations

Beginning of moving PowerSaveBlocker to //device/power_save_blocker.

power_save_blocker_factory is a wrapper to pass down the required
SequencedTaskRunners. It will stay in content when the rest of the
power_save_blocker files move to //device.

BUG=612337,257943,612563

Committed: https://crrev.com/11c0c210570c67f7b32ebc706102c9e8833cc9a1
Cr-Commit-Position: refs/heads/master@{#395727}
==========

[commit-bot: I haz the power, 2016-05-24 23:22:28]

Patchset 13 (id:??) landed as
https://crrev.com/11c0c210570c67f7b32ebc706102c9e8833cc9a1
Cr-Commit-Position: refs/heads/master@{#395727}

[tasak, 2016-05-25 08:43:15]

On 2016/05/24 23:22:28, commit-bot: I haz the power wrote:
> Patchset 13 (id:??) landed as
> https://crrev.com/11c0c210570c67f7b32ebc706102c9e8833cc9a1
> Cr-Commit-Position: refs/heads/master@{#395727}

This CL causes chrome crash when pushing record button shown in tracing UI.
I locally reverted this commit. So the local chrome doesn't crash when pushing
the button.

The following is chrome stacktrace when crashing:

[132643:132658:0525/165702:FATAL:bus.cc(842)] Check failed:
origin_task_runner_.get(). 
#0 0x7f83ac26fd2e base::debug::StackTrace::StackTrace()
#1 0x7f83ac2cd88f logging::LogMessage::~LogMessage()
#2 0x7f83a606b5f4 dbus::Bus::GetOriginTaskRunner()
#3 0x7f83a606b4e6 dbus::Bus::GetDBusTaskRunner()
#4 0x7f83a60b53a5 dbus::ObjectProxy::CallMethodWithErrorCallback()
#5 0x7f83a60b4f7f dbus::ObjectProxy::CallMethod()
#6 0x7f83a72d0cae content::PowerSaveBlockerImpl::Delegate::ApplyBlock()
#7 0x7f83a6a9be2d
_ZN4base8internal15RunnableAdapterIMN7content29BrowserAccessibilityStateImplEFvvEE3RunIRK13scoped_refptrIS3_EJEEEvOT_DpOT0_
#8 0x7f83a72d25f9
_ZN4base8internal12InvokeHelperILb0EvNS0_15RunnableAdapterIMN7content20PowerSaveBlockerImpl8DelegateEFvvEEEE8MakeItSoIJRK13scoped_refptrIS5_EEEEvS8_DpOT_
#9 0x7f83a72d25c0
_ZN4base8internal7InvokerINS_13IndexSequenceIJLm0EEEENS0_9BindStateINS0_15RunnableAdapterIMN7content20PowerSaveBlockerImpl8DelegateEFvvEEEFvPS8_EJSC_EEENS0_12InvokeHelperILb0EvSB_EEFvvEE3RunEPNS0_13BindStateBaseE
#10 0x7f83ac25718e base::Callback<>::Run()
#11 0x7f83ac3fbd5f base::SequencedWorkerPool::Inner::ThreadLoop() 
#12 0x7f83ac3fb725 base::SequencedWorkerPool::Worker::Run()
#13 0x7f83ac409e51 base::SimpleThread::ThreadMain()
#14 0x7f83ac3f96da base::(anonymous namespace)::ThreadFunc()
#15 0x7f839d32d182 start_thread
#16 0x7f839901547d clone

Aborted (core dumped)

[hajimehoshi, 2016-05-25 09:34:28]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/2006143006/ by hajimehoshi@chromium.org.

The reason for reverting is: This causes flaky test failures (e.g.
https://build.chromium.org/p/chromium.gpu/builders/Linux%20Release%20%28NVIDI...).

[hashimoto, 2016-05-25 10:17:29]

Oops, I din't notice that power_save_blocker_x11.cc uses
dbus::ObjectProxy::CallMethod which requires the caller thread to be associated
with a SingleThreadTaskRunner.

This can be fixed by passing a SingleThreadTaskRunner to the implementation, or
by changing it to use dbus::ObjectProxy::CallMethodAndBlock which doesn't
require a SingleThreadTaskRunner.