mus-gpu: Talk to the GpuHost from the IO thread.

mus-gpu: Talk to the GpuHost from the IO thread.

Bind the GpuHost connection on the IO thread, so that messages
from the gpu process can still be sent to the host process (i.e.
chrome browser) while the main thread is blocked.

BUG=643746
Review-Url: https://codereview.chromium.org/2764753004
Cr-Commit-Position: refs/heads/master@{#458722}
Committed: https://chromium.googlesource.com/chromium/src/+/66d333e3d935e43745ad8ae27b5b76a3a34f4917

[sadrul, 2017-03-21 20:22:29]

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

[commit-bot: I haz the power, 2017-03-21 20:22:54]

Dry run: 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-03-21 22:20:27]

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

[commit-bot: I haz the power, 2017-03-21 22:20:28]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-21 22:25:36]

sadrul@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-03-21 22:40:35]

https://codereview.chromium.org/2764753004/diff/1/services/ui/gpu/gpu_service.cc
File services/ui/gpu/gpu_service.cc (right):

https://codereview.chromium.org/2764753004/diff/1/services/ui/gpu/gpu_service...
services/ui/gpu/gpu_service.cc:121: gpu_host_ =
mojom::ThreadSafeGpuHostPtr::Create(std::move(gpu_host));
The ThreadSafeInterfacePtr itself is thread-safe, but |gpu_host_| storage is
not. This would need to be guarded by a lock now, wouldn't it?

You can avoid this by instead calling, in the constructor:

gpu_host_ = mojom::ThreadSafeGpuHostPtr::Create(gpu_host.PassInterface(),
io_runner_);

I don't think you'd need a CancelableTaskTracker then either?

[sadrul, 2017-03-21 22:43:10]

https://codereview.chromium.org/2764753004/diff/1/services/ui/gpu/gpu_service.cc
File services/ui/gpu/gpu_service.cc (right):

https://codereview.chromium.org/2764753004/diff/1/services/ui/gpu/gpu_service...
services/ui/gpu/gpu_service.cc:121: gpu_host_ =
mojom::ThreadSafeGpuHostPtr::Create(std::move(gpu_host));
On 2017/03/21 22:40:35, Ken Rockot wrote:
> The ThreadSafeInterfacePtr itself is thread-safe, but |gpu_host_| storage is
> not. This would need to be guarded by a lock now, wouldn't it?
> 
> You can avoid this by instead calling, in the constructor:
> 
> gpu_host_ = mojom::ThreadSafeGpuHostPtr::Create(gpu_host.PassInterface(),
> io_runner_);
> 
> I don't think you'd need a CancelableTaskTracker then either?

Ooh, so ThreadSafe* can be take a task-runner of a different thread? That's
awesome :) Yep, that simplifies this a lot. I will make that change. Thanks!

[sadrul, 2017-03-21 22:48:37]

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

[commit-bot: I haz the power, 2017-03-21 22:49:10]

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

[sadrul, 2017-03-21 22:56:13]

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

[commit-bot: I haz the power, 2017-03-21 22:56:40]

Dry run: 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-03-21 23:13:48]

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

[commit-bot: I haz the power, 2017-03-21 23:13:49]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[sadrul, 2017-03-21 23:14:17]

https://codereview.chromium.org/2764753004/diff/1/services/ui/gpu/gpu_service.cc
File services/ui/gpu/gpu_service.cc (right):

https://codereview.chromium.org/2764753004/diff/1/services/ui/gpu/gpu_service...
services/ui/gpu/gpu_service.cc:121: gpu_host_ =
mojom::ThreadSafeGpuHostPtr::Create(std::move(gpu_host));
On 2017/03/21 22:43:10, sadrul wrote:
> On 2017/03/21 22:40:35, Ken Rockot wrote:
> > The ThreadSafeInterfacePtr itself is thread-safe, but |gpu_host_| storage is
> > not. This would need to be guarded by a lock now, wouldn't it?
> > 
> > You can avoid this by instead calling, in the constructor:
> > 
> > gpu_host_ = mojom::ThreadSafeGpuHostPtr::Create(gpu_host.PassInterface(),
> > io_runner_);
> > 
> > I don't think you'd need a CancelableTaskTracker then either?
> 
> Ooh, so ThreadSafe* can be take a task-runner of a different thread? That's
> awesome :) Yep, that simplifies this a lot. I will make that change. Thanks!

Done.

[Ken Rockot(use gerrit already), 2017-03-21 23:17:45]

lgtm

[sadrul, 2017-03-21 23:50:03]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-03-21 23:50:17]

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-03-22 01:44:25]

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

[commit-bot: I haz the power, 2017-03-22 01:44:26]

Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[sadrul, 2017-03-22 02:18:58]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-03-22 02:19:28]

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-03-22 02:27:17]

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

[commit-bot: I haz the power, 2017-03-22 02:27:18]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[sadrul, 2017-03-22 03:59:22]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-03-22 04:00:09]

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-03-22 04:21:01]

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

[commit-bot: I haz the power, 2017-03-22 04:21:03]

Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[sadrul, 2017-03-22 11:50:32]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-03-22 11:50:50]

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-03-22 11:54:51]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490183432030170,
"parent_rev": "9a3e6e43c4bdce4bdb64cbeb3f5bcf61d35e2ff7", "commit_rev":
"66d333e3d935e43745ad8ae27b5b76a3a34f4917"}

[commit-bot: I haz the power, 2017-03-22 11:55:43]

Description was changed from

==========
mus-gpu: Talk to the GpuHost from the IO thread.

Bind the GpuHost connection on the IO thread, so that messages
from the gpu process can still be sent to the host process (i.e.
chrome browser) while the main thread is blocked.

BUG=643746
==========

to

==========
mus-gpu: Talk to the GpuHost from the IO thread.

Bind the GpuHost connection on the IO thread, so that messages
from the gpu process can still be sent to the host process (i.e.
chrome browser) while the main thread is blocked.

BUG=643746

Review-Url: https://codereview.chromium.org/2764753004
Cr-Commit-Position: refs/heads/master@{#458722}
Committed:
https://chromium.googlesource.com/chromium/src/+/66d333e3d935e43745ad8ae27b5b...
==========

[commit-bot: I haz the power, 2017-03-22 11:55:44]

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