gpu: Replace more chrome ipc with mojom API.

gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So
remove the associated code too. GpuProcessHostUIShim::GetOneInstance is
also no longer used anymore, so this is also removed.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2723013004
Cr-Commit-Position: refs/heads/master@{#454647}
Committed: https://chromium.googlesource.com/chromium/src/+/b428f6b1ca12af16bc8d30470ecac787f425f9f7

[sadrul, 2017-03-01 20:03:57]

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

[commit-bot: I haz the power, 2017-03-01 20:04:32]

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-01 20:05:54]

sadrul@chromium.org changed reviewers:
+ jbauman@chromium.org, tsepez@chromium.org

[sadrul, 2017-03-01 20:05:55]

tsepez@ for ipc changes (mojom, messages)

jbauman@ for all of it.

Thanks!

[Fady Samuel, 2017-03-01 20:10:57]

fsamuel@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2017-03-01 20:10:58]

fsamuel@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2017-03-01 20:10:58]

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
File services/ui/gpu/interfaces/gpu_service.mojom (right):

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:54: Crash();
nit: comment on what this does?

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:55: Hang();
nit: comment on what this does?

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:56: ThrowJavaException();
nit: comment on what this does?

[Fady Samuel, 2017-03-01 20:10:58]

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
File services/ui/gpu/interfaces/gpu_service.mojom (right):

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:54: Crash();
nit: comment on what this does?

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:55: Hang();
nit: comment on what this does?

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:56: ThrowJavaException();
nit: comment on what this does?

[sadrul, 2017-03-01 20:11:56]

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
File services/ui/gpu/interfaces/gpu_service.mojom (right):

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:56: ThrowJavaException();
On 2017/03/01 20:10:58, Fady Samuel wrote:
> nit: comment on what this does?

These should be obvious/unambiguous from the names?

[Fady Samuel, 2017-03-01 20:14:42]

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
File services/ui/gpu/interfaces/gpu_service.mojom (right):

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:56: ThrowJavaException();
On 2017/03/01 20:11:56, sadrul wrote:
> On 2017/03/01 20:10:58, Fady Samuel wrote:
> > nit: comment on what this does?
> 
> These should be obvious/unambiguous from the names?

Fair enough, although it could be argued "DestroyAllChannels" is also obvious
but it has a comment. I guess it would be nice to have a comment for consistency
but I (as a non-OWNER) don't feel too strongly about it.

[sadrul, 2017-03-01 20:18:31]

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
File services/ui/gpu/interfaces/gpu_service.mojom (right):

https://codereview.chromium.org/2723013004/diff/1/services/ui/gpu/interfaces/...
services/ui/gpu/interfaces/gpu_service.mojom:56: ThrowJavaException();
On 2017/03/01 20:14:42, Fady Samuel wrote:
> On 2017/03/01 20:11:56, sadrul wrote:
> > On 2017/03/01 20:10:58, Fady Samuel wrote:
> > > nit: comment on what this does?
> > 
> > These should be obvious/unambiguous from the names?
> 
> Fair enough, although it could be argued "DestroyAllChannels" is also obvious
> but it has a comment. I guess it would be nice to have a comment for
consistency
> but I (as a non-OWNER) don't feel too strongly about it.

I agree that DestroyAllChannels() doesn't need the comment either. My preference
would be to remove that comment.

[commit-bot: I haz the power, 2017-03-01 20:35:05]

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

[commit-bot: I haz the power, 2017-03-01 20:35:06]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tom Sepez, 2017-03-01 20:47:30]

messages LGTM but tidy comments to your liking.  Thanks.

[sadrul, 2017-03-01 21:14:07]

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

[commit-bot: I haz the power, 2017-03-01 21:14:47]

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-01 21:49:26]

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

[commit-bot: I haz the power, 2017-03-01 21:49:27]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[jbauman, 2017-03-01 22:04:26]

This seems to be failing some tests...

https://codereview.chromium.org/2723013004/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host_ui_shim.cc (right):

https://codereview.chromium.org/2723013004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host_ui_shim.cc:141:
GpuProcessHost::CallOnIO(GpuProcessHost::GPU_PROCESS_KIND_UNSANDBOXED,
We should have a version of CallOnIO that takes host_id_, so we can be sure that
the GpuProcessHost that receives the message is the same that was around when
these calls are made.

[jbauman, 2017-03-01 22:43:04]

https://codereview.chromium.org/2723013004/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host_ui_shim.cc (right):

https://codereview.chromium.org/2723013004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host_ui_shim.cc:141:
GpuProcessHost::CallOnIO(GpuProcessHost::GPU_PROCESS_KIND_UNSANDBOXED,
On 2017/03/01 22:04:26, jbauman wrote:
> We should have a version of CallOnIO that takes host_id_, so we can be sure
that
> the GpuProcessHost that receives the message is the same that was around when
> these calls are made.

For example, these should all be done on the SANDBOXED gpu process host, so
that's why they're not working.

[sadrul, 2017-03-02 01:05:14]

Description was changed from

==========
gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So remove
the associated code too.

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

to

==========
gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So remove
the associated code too.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[sadrul, 2017-03-02 01:05:18]

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

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

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-02 01:23:03]

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

[commit-bot: I haz the power, 2017-03-02 01:23:04]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[sadrul, 2017-03-02 01:30:47]

Patchset #3 (id:40001) has been deleted

[sadrul, 2017-03-02 01:59:58]

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

[commit-bot: I haz the power, 2017-03-02 02:00:43]

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-02 02:11:27]

Description was changed from

==========
gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So remove
the associated code too.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So
remove the associated code too. GpuProcessHostUIShim::GetOneInstance is
also no longer used anymore, so this is also removed.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[sadrul, 2017-03-02 03:36:54]

https://codereview.chromium.org/2723013004/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host_ui_shim.cc (right):

https://codereview.chromium.org/2723013004/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host_ui_shim.cc:141:
GpuProcessHost::CallOnIO(GpuProcessHost::GPU_PROCESS_KIND_UNSANDBOXED,
On 2017/03/01 22:43:04, jbauman wrote:
> On 2017/03/01 22:04:26, jbauman wrote:
> > We should have a version of CallOnIO that takes host_id_, so we can be sure
> that
> > the GpuProcessHost that receives the message is the same that was around
when
> > these calls are made.
> 
> For example, these should all be done on the SANDBOXED gpu process host, so
> that's why they're not working.

Fixed, thanks for catching these!

I did a little more cleanup at the same time: turns out, these functions are
called from just one place: debug_urls.cc (and Crash() is also called in a
test). So I updated those code to directly call GpuProcessHost::CallOnIO()
instead. This also allowed the removal of GpuProcessHostUIShim::GetOneInstance()
(which I find somewhat weird)

[commit-bot: I haz the power, 2017-03-02 03:50:53]

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

[commit-bot: I haz the power, 2017-03-02 03:50:53]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-03 03:56:42]

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

[sadrul, 2017-03-03 03:56:43]

+boliu@ for content/browser/frame_host

[jbauman, 2017-03-03 06:21:22]

lgtm

[boliu, 2017-03-03 17:29:01]

lgtm

[sadrul, 2017-03-03 18:30:05]

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

[commit-bot: I haz the power, 2017-03-03 18:30:23]

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-03 18:51:23]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2017-03-03 18:51:23]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
boliu@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2723013004/#ps80001
(title: "tot merge")

[commit-bot: I haz the power, 2017-03-03 18:51:29]

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-03 19:28:35]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1488567083097920,
"parent_rev": "d80646f535fa6368ab78b802206b0b2f50dd33e5", "commit_rev":
"b428f6b1ca12af16bc8d30470ecac787f425f9f7"}

[commit-bot: I haz the power, 2017-03-03 19:29:11]

Description was changed from

==========
gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So
remove the associated code too. GpuProcessHostUIShim::GetOneInstance is
also no longer used anymore, so this is also removed.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
gpu: Replace more chrome ipc with mojom API.

List of chrome ipc messages replaced in this CL:
. GpuMsg_Clean
. GpuMsg_Crash
. GpuMsg_Hang
. GpuMsg_JavaCrash

GpuProcessHostUIShim no longer needs to be an IPC::Sender anymore. So
remove the associated code too. GpuProcessHostUIShim::GetOneInstance is
also no longer used anymore, so this is also removed.

BUG=643746
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2723013004
Cr-Commit-Position: refs/heads/master@{#454647}
Committed:
https://chromium.googlesource.com/chromium/src/+/b428f6b1ca12af16bc8d30470eca...
==========

[commit-bot: I haz the power, 2017-03-03 19:29:12]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/b428f6b1ca12af16bc8d30470eca...