Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(37)

Issue 2400313002: Convert miscellaneous RenderThreadImpl messages to mojom (Closed)

Created:
4 years, 2 months ago by Ken Rockot(use gerrit already)
Modified:
4 years, 2 months ago
CC:
Aaron Boodman, abarth-chromium, cbentzel+watch_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, jam, mlamouri+watch-content_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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}

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Patch Set 4 : rebase #

Patch Set 5 : . #

Patch Set 6 : rebase #

Total comments: 2

Patch Set 7 : . #

Patch Set 8 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+207 lines, -138 lines) Patch
M content/browser/android/content_view_statics.cc View 1 2 3 4 3 chunks +3 lines, -2 lines 0 comments Download
M content/browser/net/browser_online_state_observer.cc View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M content/browser/plugin_service_impl.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M content/browser/theme_helper_mac.mm View 1 2 3 4 5 6 5 chunks +20 lines, -14 lines 0 comments Download
M content/common/native_types.mojom View 1 chunk +13 lines, -0 lines 0 comments Download
M content/common/native_types.typemap View 3 chunks +3 lines, -0 lines 0 comments Download
A content/common/native_types_mac.typemap View 1 chunk +34 lines, -0 lines 0 comments Download
M content/common/renderer.mojom View 1 2 2 chunks +29 lines, -0 lines 0 comments Download
A content/common/typemaps_mac.gni View 1 chunk +5 lines, -0 lines 0 comments Download
M content/common/view_messages.h View 1 2 3 4 5 6 7 4 chunks +0 lines, -38 lines 0 comments Download
M content/renderer/render_thread_impl.h View 1 2 3 4 5 6 7 3 chunks +11 lines, -17 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 4 5 6 7 3 chunks +55 lines, -60 lines 0 comments Download
M mojo/public/tools/bindings/chromium_bindings_configuration.gni View 1 2 3 4 5 2 chunks +16 lines, -1 line 0 comments Download
M mojo/public/tools/bindings/mojom.gni View 2 chunks +13 lines, -1 line 0 comments Download

Messages

Total messages: 65 (49 generated)
Ken Rockot(use gerrit already)
PTAL nick@ for content/ sammc@ for mojo/ tsepez@ for mojom+messages These are straightforward message conversions, ...
4 years, 2 months ago (2016-10-07 22:58:46 UTC) #12
Tom Sepez
Messages are fine, but it's a pity we're now including code on all platforms -- ...
4 years, 2 months ago (2016-10-10 17:26:18 UTC) #15
Ken Rockot(use gerrit already)
We've tried pretty hard to avoid platform-specific blocks in mojom, but I think it might ...
4 years, 2 months ago (2016-10-10 17:29:29 UTC) #16
ncarter (slow)
The NetInfoBrowserTest.VerifyNetworkStateInitialized failures look real and I assume are related to ViewMsg_NetworkConnectionChanged.
4 years, 2 months ago (2016-10-10 18:33:08 UTC) #17
Tom Sepez
Ok, I'll give you an LGTM on the messages modulo the red bots.
4 years, 2 months ago (2016-10-10 23:09:13 UTC) #18
Ken Rockot(use gerrit already)
On 2016/10/10 at 18:33:08, nick wrote: > The NetInfoBrowserTest.VerifyNetworkStateInitialized failures look real and I assume ...
4 years, 2 months ago (2016-10-11 17:57:02 UTC) #19
Ken Rockot(use gerrit already)
On 2016/10/11 at 17:57:02, Ken Rockot wrote: > On 2016/10/10 at 18:33:08, nick wrote: > ...
4 years, 2 months ago (2016-10-11 19:34:31 UTC) #20
ncarter (slow)
On 2016/10/11 19:34:31, Ken Rockot wrote: > On 2016/10/11 at 17:57:02, Ken Rockot wrote: > ...
4 years, 2 months ago (2016-10-11 19:46:07 UTC) #21
Ken Rockot(use gerrit already)
I think this is ready for content review now.
4 years, 2 months ago (2016-10-17 19:39:17 UTC) #47
ncarter (slow)
lgtm modulo one question https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_helper_mac.mm File content/browser/theme_helper_mac.mm (right): https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_helper_mac.mm#newcode156 content/browser/theme_helper_mac.mm:156: params->redraw = redraw; What happened ...
4 years, 2 months ago (2016-10-17 21:26:22 UTC) #48
Ken Rockot(use gerrit already)
https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_helper_mac.mm File content/browser/theme_helper_mac.mm (right): https://codereview.chromium.org/2400313002/diff/200001/content/browser/theme_helper_mac.mm#newcode156 content/browser/theme_helper_mac.mm:156: params->redraw = redraw; On 2016/10/17 at 21:26:22, ncarter wrote: ...
4 years, 2 months ago (2016-10-18 14:52:19 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2400313002/240001
4 years, 2 months ago (2016-10-18 14:57:29 UTC) #58
commit-bot: I haz the power
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_rel_ng/builds/299898)
4 years, 2 months ago (2016-10-18 16:20:34 UTC) #60
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2400313002/240001
4 years, 2 months ago (2016-10-18 16:22:20 UTC) #62
commit-bot: I haz the power
Committed patchset #8 (id:240001)
4 years, 2 months ago (2016-10-18 17:39:41 UTC) #63
commit-bot: I haz the power
4 years, 2 months ago (2016-10-18 18:52:31 UTC) #65
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/a2db0da89dd2726e057e8af9b49b18a8a3a4813a
Cr-Commit-Position: refs/heads/master@{#426003}

Powered by Google App Engine
This is Rietveld 408576698