Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5)

Issue 2214383002: Move registration of Java mojo interfaces to the new InterfaceRegistrar. (Closed)

Created:
4 years, 4 months ago by Sam McNally
Modified:
4 years, 2 months ago
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, chrome-apps-syd-reviews_chromium.org, chromium-reviews, creis+watch_chromium.org, darin (slow to review), darin-cc_chromium.org, nasko+codewatch_chromium.org, qsr+mojo_chromium.org, timvolodine, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@java-content-interface-registry
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move registration of Java mojo interfaces to the new InterfaceRegistrar. Previously, Java-implemented mojo interfaces were registered on the C++ InterfaceRegistries via JNI. This CL migrates those registrations to dedicated Java InterfaceRegistries that C++ code can access via corresponding InterfaceProviders. BUG=634568, 637174 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Committed: https://crrev.com/58dcb20da65389d841cd02d9197fb43efb6e8ee2 Committed: https://crrev.com/5a139ba8b5636e7196714b4132b9240b18579ed3 Cr-Original-Commit-Position: refs/heads/master@{#421403} Cr-Commit-Position: refs/heads/master@{#421711}

Patch Set 1 : #

Patch Set 2 : #

Total comments: 8

Patch Set 3 : rebase #

Patch Set 4 : #

Patch Set 5 : rebase #

Total comments: 1

Patch Set 6 : rebase #

Patch Set 7 : fix vr_shell build #

Unified diffs Side-by-side diffs Delta from patch set Stats (+212 lines, -152 lines) Patch
M chrome/android/BUILD.gn View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/android/java/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/mojo/ChromeInterfaceRegistrar.java View 1 2 3 4 1 chunk +12 lines, -6 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestFactory.java View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webshare/ShareServiceImplementationFactory.java View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/DEPS View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/android/mojo/chrome_interface_registrar_android.h View 1 chunk +1 line, -20 lines 0 comments Download
M chrome/browser/android/mojo/chrome_interface_registrar_android.cc View 1 chunk +5 lines, -18 lines 0 comments Download
M chrome/browser/chrome_browser_main_android.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 2 3 4 5 3 chunks +14 lines, -3 lines 0 comments Download
M content/browser/BUILD.gn View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/DEPS View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 2 3 4 5 2 chunks +6 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 2 chunks +6 lines, -1 line 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download
M content/public/android/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java View 1 2 3 4 1 chunk +0 lines, -94 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java View 1 2 3 4 3 chunks +37 lines, -0 lines 0 comments Download
A content/public/android/java/src/org/chromium/content/browser/NfcFactory.java View 1 2 3 4 1 chunk +61 lines, -0 lines 0 comments Download
M device/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M device/battery/android/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M device/battery/android/java/src/org/chromium/device/battery/BatteryMonitorFactory.java View 1 2 3 4 3 chunks +4 lines, -2 lines 0 comments Download
M device/vibration/android/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M device/vibration/android/java/src/org/chromium/device/vibration/VibrationManagerImpl.java View 1 2 3 4 2 chunks +16 lines, -0 lines 0 comments Download
M services/shell/public/cpp/interface_provider.h View 1 2 chunks +14 lines, -0 lines 0 comments Download
M services/shell/public/java/src/org/chromium/services/shell/InterfaceRegistry.java View 1 2 3 4 2 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 100 (79 generated)
Sam McNally
4 years, 3 months ago (2016-09-20 22:22:47 UTC) #50
Ken Rockot(use gerrit already)
LGTM - this is great. My only nit would be maybe to call it MakeInterfaceFactory ...
4 years, 3 months ago (2016-09-21 14:11:39 UTC) #51
Sam McNally
+jam for //chrome and //content On 2016/09/21 14:11:39, Ken Rockot wrote: > LGTM - this ...
4 years, 3 months ago (2016-09-22 00:01:23 UTC) #55
jam
On 2016/09/22 00:01:23, Sam McNally wrote: > +jam for //chrome and //content I'm not really ...
4 years, 3 months ago (2016-09-22 03:48:47 UTC) #58
Sam McNally
+boliu for //content/browser +dtrainor for *android*
4 years, 3 months ago (2016-09-22 04:00:09 UTC) #61
boliu
can you write a better CL description? https://codereview.chromium.org/2214383002/diff/220001/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java File content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java (right): https://codereview.chromium.org/2214383002/diff/220001/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java#newcode17 content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java:17: class InterfaceRegistrar ...
4 years, 3 months ago (2016-09-22 14:32:18 UTC) #62
Sam McNally
https://codereview.chromium.org/2214383002/diff/220001/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java File content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java (right): https://codereview.chromium.org/2214383002/diff/220001/content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java#newcode17 content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrar.java:17: class InterfaceRegistrar { On 2016/09/22 14:32:18, boliu wrote: > ...
4 years, 3 months ago (2016-09-23 01:38:50 UTC) #71
boliu
content lgtm
4 years, 3 months ago (2016-09-23 02:02:40 UTC) #72
David Trainor- moved to gerrit
*android* lgtm. Thanks! Sorry for the delayed review.
4 years, 2 months ago (2016-09-26 18:44:00 UTC) #75
Sam McNally
+jam for chrome/browser/DEPS
4 years, 2 months ago (2016-09-26 23:40:24 UTC) #77
jam
lgtm
4 years, 2 months ago (2016-09-27 16:02:12 UTC) #78
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/2214383002/300001
4 years, 2 months ago (2016-09-28 00:09:59 UTC) #84
commit-bot: I haz the power
Committed patchset #5 (id:300001)
4 years, 2 months ago (2016-09-28 00:53:22 UTC) #85
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/58dcb20da65389d841cd02d9197fb43efb6e8ee2 Cr-Commit-Position: refs/heads/master@{#421403}
4 years, 2 months ago (2016-09-28 00:56:37 UTC) #87
David Trainor- moved to gerrit
A revert of this CL (patchset #5 id:300001) has been created in https://codereview.chromium.org/2374213002/ by [email protected]. ...
4 years, 2 months ago (2016-09-28 21:21:53 UTC) #88
boliu
https://codereview.chromium.org/2214383002/diff/300001/content/browser/BUILD.gn File content/browser/BUILD.gn (right): https://codereview.chromium.org/2214383002/diff/300001/content/browser/BUILD.gn#newcode1763 content/browser/BUILD.gn:1763: deps -= [ "//device/battery" ] ahh, should remove this ...
4 years, 2 months ago (2016-09-28 21:29:53 UTC) #89
David Trainor- moved to gerrit
On 2016/09/28 21:29:53, boliu wrote: > https://codereview.chromium.org/2214383002/diff/300001/content/browser/BUILD.gn > File content/browser/BUILD.gn (right): > > https://codereview.chromium.org/2214383002/diff/300001/content/browser/BUILD.gn#newcode1763 > ...
4 years, 2 months ago (2016-09-28 21:52:29 UTC) #90
Sam McNally
On 2016/09/28 21:52:29, David Trainor wrote: > On 2016/09/28 21:29:53, boliu wrote: > > > ...
4 years, 2 months ago (2016-09-29 00:17:38 UTC) #93
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/2214383002/360001
4 years, 2 months ago (2016-09-29 00:18:11 UTC) #96
commit-bot: I haz the power
Committed patchset #7 (id:360001)
4 years, 2 months ago (2016-09-29 01:49:37 UTC) #98
commit-bot: I haz the power
4 years, 2 months ago (2016-09-29 01:51:58 UTC) #100
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/5a139ba8b5636e7196714b4132b9240b18579ed3
Cr-Commit-Position: refs/heads/master@{#421711}

Powered by Google App Engine
This is Rietveld 408576698