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

Issue 1996193003: Add support for calling the java Callback from native. (Closed)

Created:
4 years, 7 months ago by Ted C
Modified:
4 years, 7 months ago
Reviewers:
Lei Zhang, nyquist, Yaron
CC:
chromium-reviews, mvanouwerkerk+watch_chromium.org, mcwilliams+watch_chromium.org, dgn+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add support for calling the java Callback from native. Attempting to reduce the boilerplate interfaces that need to be added just to pass back single params from native to java. BUG=None Committed: https://crrev.com/7efe979a9f4ca0755a535bde1cc4d5a5492f4277 Cr-Commit-Position: refs/heads/master@{#395218}

Patch Set 1 #

Patch Set 2 : Sure, sure, I'll add all the files #

Total comments: 4

Patch Set 3 : Add support for calling callbacks with boolean primitives #

Patch Set 4 : Fix base.gypi missing trailing ' #

Patch Set 5 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+137 lines, -93 lines) Patch
M base/BUILD.gn View 2 chunks +3 lines, -0 lines 0 comments Download
M base/android/base_jni_registrar.cc View 2 chunks +2 lines, -0 lines 0 comments Download
A base/android/callback_android.h View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
A base/android/callback_android.cc View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
M base/android/java/src/org/chromium/base/Callback.java View 1 2 1 chunk +16 lines, -2 lines 0 comments Download
M base/base.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M base/base.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/notifications/WebApkNotificationClient.java View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java View 1 2 3 4 2 chunks +1 line, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java View 1 2 3 4 4 chunks +2 lines, -18 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePermissionsFetcher.java View 2 chunks +32 lines, -32 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java View 3 chunks +3 lines, -18 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/services/AndroidEduAndChildAccountHelper.java View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/android/webapk/libs/client/junit/src/org/chromium/webapk/lib/client/WebApkServiceConnectionManagerTest.java View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/ntp/ntp_snippets_bridge.cc View 1 2 3 4 3 chunks +3 lines, -8 lines 0 comments Download
M chrome/browser/android/preferences/website_preference_bridge.cc View 3 chunks +3 lines, -5 lines 0 comments Download

Messages

Total messages: 37 (15 generated)
Ted C
PTAL Personally, conflicted. I saw people adding more of these interfaces recently and took a ...
4 years, 7 months ago (2016-05-19 23:29:46 UTC) #2
Michael van Ouwerkerk
Drive-by notes :-) This seems like a nice cleanup. Aren't there more places that would ...
4 years, 7 months ago (2016-05-20 09:01:41 UTC) #3
Yaron
lgtm When I saw the CL I was concerned about possible memory leaks because it ...
4 years, 7 months ago (2016-05-20 14:27:25 UTC) #4
nyquist
https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/org/chromium/base/Callback.java File base/android/java/src/org/chromium/base/Callback.java (right): https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/org/chromium/base/Callback.java#newcode14 base/android/java/src/org/chromium/base/Callback.java:14: public abstract class Callback<T> { Do you think it ...
4 years, 7 months ago (2016-05-20 15:59:09 UTC) #5
Ted C
On 2016/05/20 09:01:41, Michael van Ouwerkerk wrote: > Drive-by notes :-) > > This seems ...
4 years, 7 months ago (2016-05-20 16:20:04 UTC) #6
Ted C
https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/org/chromium/base/Callback.java File base/android/java/src/org/chromium/base/Callback.java (right): https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/org/chromium/base/Callback.java#newcode14 base/android/java/src/org/chromium/base/Callback.java:14: public abstract class Callback<T> { On 2016/05/20 15:59:08, nyquist ...
4 years, 7 months ago (2016-05-20 16:21:23 UTC) #7
nyquist
lgtm. Agree that adding the one boolean-version will show the intent of possibly adding more ...
4 years, 7 months ago (2016-05-20 16:23:54 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996193003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996193003/40001
4 years, 7 months ago (2016-05-20 16:28:14 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/187398) linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, ...
4 years, 7 months ago (2016-05-20 16:32:20 UTC) #13
Ted C
+thestig for base gn/gyp files owners
4 years, 7 months ago (2016-05-20 16:38:04 UTC) #15
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996193003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996193003/60001
4 years, 7 months ago (2016-05-20 16:38:32 UTC) #17
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 7 months ago (2016-05-20 18:26:35 UTC) #19
Lei Zhang
lgtm
4 years, 7 months ago (2016-05-20 21:02:03 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996193003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996193003/60001
4 years, 7 months ago (2016-05-20 22:02:23 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/187705)
4 years, 7 months ago (2016-05-20 22:11:06 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996193003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996193003/60001
4 years, 7 months ago (2016-05-20 23:04:23 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/187767)
4 years, 7 months ago (2016-05-20 23:12:38 UTC) #29
Lei Zhang
On 2016/05/20 23:12:38, commit-bot: I haz the power wrote: > Try jobs failed on following ...
4 years, 7 months ago (2016-05-20 23:15:31 UTC) #30
Ted C
On 2016/05/20 23:15:31, Lei Zhang wrote: > On 2016/05/20 23:12:38, commit-bot: I haz the power ...
4 years, 7 months ago (2016-05-20 23:33:03 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996193003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1996193003/80001
4 years, 7 months ago (2016-05-20 23:33:33 UTC) #34
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 7 months ago (2016-05-21 00:36:01 UTC) #35
commit-bot: I haz the power
4 years, 7 months ago (2016-05-21 00:37:46 UTC) #37
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/7efe979a9f4ca0755a535bde1cc4d5a5492f4277
Cr-Commit-Position: refs/heads/master@{#395218}

Powered by Google App Engine
This is Rietveld 408576698