Convert miscellaneous RenderThreadImpl messages to mojom

Convert miscellaneous RenderThreadImpl messages to mojom

  ViewMsg_NetworkConnectionChanged
  ViewMsg_UpdateScrollbarTheme
  ViewMsg_SystemColorsChanged
  ViewMsg_SetWebKitSharedTimersSuspended
  ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types.

We would still like to avoid any support for preproccessing or
platform-specific declarations in mojom, but this CL does add
support for platform-specific (Mac only atm) typemap
configuration.

BUG=612500
Committed: https://crrev.com/a2db0da89dd2726e057e8af9b49b18a8a3a4813a
Cr-Commit-Position: refs/heads/master@{#426003}

[Ken Rockot(use gerrit already), 2016-10-07 22:31:43]

Description was changed from

==========
Convert miscellaneous RenderThreadImpl messages to mojom

BUG=612500
==========

to

==========
Convert miscellaneous RenderThreadImpl messages to mojom

ViewMsg_NetworkConnectionChanged
ViewMsg_UpdateScrollbarTheme
ViewMsg_SystemColorsChanged
ViewMsg_SetWebKitSharedTimersSuspended

Some complexity here due to some messages being Mac only and
using Mac-only enum types. Added support for platform-specific
typemap configuration.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-10-07 22:52:33]

Description was changed from

==========
Convert miscellaneous RenderThreadImpl messages to mojom

ViewMsg_NetworkConnectionChanged
ViewMsg_UpdateScrollbarTheme
ViewMsg_SystemColorsChanged
ViewMsg_SetWebKitSharedTimersSuspended

Some complexity here due to some messages being Mac only and
using Mac-only enum types. Added support for platform-specific
typemap configuration.

BUG=612500
==========

to

==========
Convert miscellaneous RenderThreadImpl messages to mojom

ViewMsg_NetworkConnectionChanged
ViewMsg_UpdateScrollbarTheme
ViewMsg_SystemColorsChanged
ViewMsg_SetWebKitSharedTimersSuspended
ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types. Added support for platform-specific
typemap configuration.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-10-07 22:52:41]

Description was changed from

==========
Convert miscellaneous RenderThreadImpl messages to mojom

ViewMsg_NetworkConnectionChanged
ViewMsg_UpdateScrollbarTheme
ViewMsg_SystemColorsChanged
ViewMsg_SetWebKitSharedTimersSuspended
ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types. Added support for platform-specific
typemap configuration.

BUG=612500
==========

to

==========
Convert miscellaneous RenderThreadImpl messages to mojom

  ViewMsg_NetworkConnectionChanged
  ViewMsg_UpdateScrollbarTheme
  ViewMsg_SystemColorsChanged
  ViewMsg_SetWebKitSharedTimersSuspended
  ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types. Added support for platform-specific
typemap configuration.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-10-07 22:54:02]

Description was changed from

==========
Convert miscellaneous RenderThreadImpl messages to mojom

  ViewMsg_NetworkConnectionChanged
  ViewMsg_UpdateScrollbarTheme
  ViewMsg_SystemColorsChanged
  ViewMsg_SetWebKitSharedTimersSuspended
  ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types. Added support for platform-specific
typemap configuration.

BUG=612500
==========

to

==========
Convert miscellaneous RenderThreadImpl messages to mojom

  ViewMsg_NetworkConnectionChanged
  ViewMsg_UpdateScrollbarTheme
  ViewMsg_SystemColorsChanged
  ViewMsg_SetWebKitSharedTimersSuspended
  ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types.

We would still like to avoid any support for preproccessing or
platform-specific declarations in mojom, but this CL does add
support for platform-specific (Mac only atm) typemap
configuration.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-10-07 22:55:07]

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

[Ken Rockot(use gerrit already), 2016-10-07 22:55:13]

Patchset #4 (id:60001) has been deleted

[Ken Rockot(use gerrit already), 2016-10-07 22:55:16]

Patchset #3 (id:40001) has been deleted

[Ken Rockot(use gerrit already), 2016-10-07 22:55:20]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-10-07 22:55:35]

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

[Ken Rockot(use gerrit already), 2016-10-07 22:55:35]

Patchset #1 (id:1) has been deleted

[Ken Rockot(use gerrit already), 2016-10-07 22:58:45]

rockot@chromium.org changed reviewers:
+ nick@chromium.org, sammc@chromium.org, tsepez@chromium.org

[Ken Rockot(use gerrit already), 2016-10-07 22:58:46]

PTAL

nick@ for content/
sammc@ for mojo/
tsepez@ for mojom+messages

These are straightforward message conversions, with the only real annoyance
being treatment of platform-specific types.

[commit-bot: I haz the power, 2016-10-07 23:18:35]

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

[commit-bot: I haz the power, 2016-10-07 23:18:36]

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...)

[Tom Sepez, 2016-10-10 17:26:18]

Messages are fine, but it's a pity we're now including code on all platforms --
do the mojom folks have an answer about platform-specific mojom?  A separate
file conditionalized in GN?

[Ken Rockot(use gerrit already), 2016-10-10 17:29:29]

We've tried pretty hard to avoid platform-specific blocks in mojom, but I
think it might be worth doing. There's more platform-specific IPC in Chrome
than I think we had originally anticipated. I'll send a proposal to
chromium-mojo for discussion.

On Mon, Oct 10, 2016 at 10:26 AM, <tsepez@chromium.org> wrote:

> Messages are fine, but it's a pity we're now including code on all
> platforms --
> do the mojom folks have an answer about platform-specific mojom? A separate
> file conditionalized in GN?
>
> https://codereview.chromium.org/2400313002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ncarter (slow), 2016-10-10 18:33:08]

The NetInfoBrowserTest.VerifyNetworkStateInitialized failures look real and I
assume are related to ViewMsg_NetworkConnectionChanged.

[Tom Sepez, 2016-10-10 23:09:13]

Ok, I'll give you an LGTM on the messages modulo the red bots.

[Ken Rockot(use gerrit already), 2016-10-11 17:57:02]

On 2016/10/10 at 18:33:08, nick wrote:
> The NetInfoBrowserTest.VerifyNetworkStateInitialized failures look real and I
assume are related to ViewMsg_NetworkConnectionChanged.

Sorry, I was temporarily pulled away from this CL. You're right, it's a real
failure. Looking into it now.

[Ken Rockot(use gerrit already), 2016-10-11 19:34:31]

On 2016/10/11 at 17:57:02, Ken Rockot wrote:
> On 2016/10/10 at 18:33:08, nick wrote:
> > The NetInfoBrowserTest.VerifyNetworkStateInitialized failures look real and
I assume are related to ViewMsg_NetworkConnectionChanged.
> 
> Sorry, I was temporarily pulled away from this CL. You're right, it's a real
failure. Looking into it now.

Right, so the problem is that -- sort of unavoidably -- it's impossible to
request and use Channel-associated mojom interfaces before the Channel exists.
This test actually causes an IPC to be sent on the RPH before RPHI::Init is
called.

RPHI queues outgoing messages internally if the |channel_| doesn't exist and
Init hasn't been called yet.

Because I recently added queueing logic to the channel implementation to address
similar issues around ordering during process launch (see https://goo.gl/REW75h
if curious), I believe everything should be correct if I completely remove
|queued_messages_| from RPHI and instead ensure that |channel_| is always
non-null. This will require a few small additional RPHI changes to track process
state, since non-null |channel_| is currently interpreted roughly as "RPHI is
initialized the process is still alive." WDYT?

[ncarter (slow), 2016-10-11 19:46:07]

On 2016/10/11 19:34:31, Ken Rockot wrote:
> On 2016/10/11 at 17:57:02, Ken Rockot wrote:
> > On 2016/10/10 at 18:33:08, nick wrote:
> > > The NetInfoBrowserTest.VerifyNetworkStateInitialized failures look real
and
> I assume are related to ViewMsg_NetworkConnectionChanged.
> > 
> > Sorry, I was temporarily pulled away from this CL. You're right, it's a real
> failure. Looking into it now.
> 
> Right, so the problem is that -- sort of unavoidably -- it's impossible to
> request and use Channel-associated mojom interfaces before the Channel exists.
> This test actually causes an IPC to be sent on the RPH before RPHI::Init is
> called.
> 
> RPHI queues outgoing messages internally if the |channel_| doesn't exist and
> Init hasn't been called yet.
> 
> Because I recently added queueing logic to the channel implementation to
address
> similar issues around ordering during process launch (see
https://goo.gl/REW75h
> if curious), I believe everything should be correct if I completely remove
> |queued_messages_| from RPHI and instead ensure that |channel_| is always
> non-null. This will require a few small additional RPHI changes to track
process
> state, since non-null |channel_| is currently interpreted roughly as "RPHI is
> initialized the process is still alive." WDYT?

This sounds reasonable; no reason to have two queueing mechanisms, and since we
just
branched we can probably tolerate whatever risk this entails.

[Ken Rockot(use gerrit already), 2016-10-11 23:59:15]

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

[commit-bot: I haz the power, 2016-10-11 23:59:46]

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, 2016-10-12 00:12:24]

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

[commit-bot: I haz the power, 2016-10-12 00:12:25]

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

[Ken Rockot(use gerrit already), 2016-10-14 16:55:06]

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

[commit-bot: I haz the power, 2016-10-14 16:55:39]

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, 2016-10-14 17:18:09]

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

[commit-bot: I haz the power, 2016-10-14 17:18:10]

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...)

[Ken Rockot(use gerrit already), 2016-10-14 17:25:51]

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

[commit-bot: I haz the power, 2016-10-14 17:26:09]

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

[Ken Rockot(use gerrit already), 2016-10-14 17:26:12]

Patchset #3 (id:120001) has been deleted

[commit-bot: I haz the power, 2016-10-14 18:44:57]

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

[commit-bot: I haz the power, 2016-10-14 18:44:59]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-10-14 21:07:41]

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

[commit-bot: I haz the power, 2016-10-14 21:08:24]

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, 2016-10-14 21:32:49]

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

[commit-bot: I haz the power, 2016-10-14 21:32:51]

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...)

[Ken Rockot(use gerrit already), 2016-10-14 21:35:10]

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

[commit-bot: I haz the power, 2016-10-14 21:35:35]

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, 2016-10-14 22:32:57]

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

[commit-bot: I haz the power, 2016-10-14 22:32:58]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-10-17 16:28:44]

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

[commit-bot: I haz the power, 2016-10-17 16:29:02]

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, 2016-10-17 17:39:24]

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

[commit-bot: I haz the power, 2016-10-17 17:39:26]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-10-17 19:39:17]

I think this is ready for content review now.

[ncarter (slow), 2016-10-17 21:26:22]

lgtm modulo one question

https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_...
File content/browser/theme_helper_mac.mm (right):

https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_...
content/browser/theme_helper_mac.mm:156: params->redraw = redraw;
What happened to the rphi->RecomputeAndUpdateWebKitPreferences() ?

[Ken Rockot(use gerrit already), 2016-10-18 14:52:05]

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

[Ken Rockot(use gerrit already), 2016-10-18 14:52:19]

https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_...
File content/browser/theme_helper_mac.mm (right):

https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_...
content/browser/theme_helper_mac.mm:156: params->redraw = redraw;
On 2016/10/17 at 21:26:22, ncarter wrote:
> What happened to the rphi->RecomputeAndUpdateWebKitPreferences() ?

Hmm. Seems to have disappeared. I have un-disappeared it.

[Ken Rockot(use gerrit already), 2016-10-18 14:52:21]

The CQ bit was unchecked by rockot@chromium.org

[commit-bot: I haz the power, 2016-10-18 14:52:25]

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

[Ken Rockot(use gerrit already), 2016-10-18 14:54:52]

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

[commit-bot: I haz the power, 2016-10-18 14:55:12]

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

[Ken Rockot(use gerrit already), 2016-10-18 14:57:12]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-10-18 14:57:14]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-10-18 14:57:15]

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

[commit-bot: I haz the power, 2016-10-18 14:57: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, 2016-10-18 16:20:32]

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

[commit-bot: I haz the power, 2016-10-18 16:20:34]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Ken Rockot(use gerrit already), 2016-10-18 16:21:54]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-10-18 16:22:20]

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, 2016-10-18 17:39:41]

Committed patchset #8 (id:240001)

[commit-bot: I haz the power, 2016-10-18 18:51:54]

Description was changed from

==========
Convert miscellaneous RenderThreadImpl messages to mojom

  ViewMsg_NetworkConnectionChanged
  ViewMsg_UpdateScrollbarTheme
  ViewMsg_SystemColorsChanged
  ViewMsg_SetWebKitSharedTimersSuspended
  ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types.

We would still like to avoid any support for preproccessing or
platform-specific declarations in mojom, but this CL does add
support for platform-specific (Mac only atm) typemap
configuration.

BUG=612500
==========

to

==========
Convert miscellaneous RenderThreadImpl messages to mojom

  ViewMsg_NetworkConnectionChanged
  ViewMsg_UpdateScrollbarTheme
  ViewMsg_SystemColorsChanged
  ViewMsg_SetWebKitSharedTimersSuspended
  ViewMsg_PurgePluginListCache

Some complexity here due to some messages being Mac only and
using Mac-only enum types.

We would still like to avoid any support for preproccessing or
platform-specific declarations in mojom, but this CL does add
support for platform-specific (Mac only atm) typemap
configuration.

BUG=612500

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

[commit-bot: I haz the power, 2016-10-18 18:52:31]

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