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

Issue 345753003: [Android] Java Bridge with Gin: implement Java Bridge dispatcher (Closed)

Created:
6 years, 6 months ago by mnaganov (inactive)
Modified:
6 years, 5 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam
Project:
chromium
Visibility:
Public.

Description

[Android] Java Bridge with Gin: implement Java Bridge dispatcher This patch adds implementation for GinJavaBridgeDispatcherHost class, which is responsible for serving JB requests from renderers, and fulfilling them with help of GinJavaBoundObject instances. This patch also enables passing a error message for method invocation errors back to renderers, so they can use it when raising JavaScript exceptions. BUG=355644 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=280298

Patch Set 1 #

Total comments: 4

Patch Set 2 : Comments addressed #

Total comments: 2

Patch Set 3 : Use an enum for passing error codes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+780 lines, -40 lines) Patch
A content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.h View 1 chunk +123 lines, -0 lines 0 comments Download
A content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc View 1 2 1 chunk +497 lines, -0 lines 0 comments Download
M content/browser/renderer_host/java/gin_java_method_invocation_helper.h View 1 2 4 chunks +4 lines, -3 lines 0 comments Download
M content/browser/renderer_host/java/gin_java_method_invocation_helper.cc View 1 2 9 chunks +13 lines, -19 lines 0 comments Download
M content/browser/renderer_host/java/gin_java_method_invocation_helper_unittest.cc View 1 2 4 chunks +62 lines, -8 lines 0 comments Download
M content/browser/renderer_host/java/java_method.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
A content/common/android/gin_java_bridge_errors.h View 1 2 1 chunk +25 lines, -0 lines 0 comments Download
A content/common/android/gin_java_bridge_errors.cc View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
M content/common/gin_java_bridge_messages.h View 1 2 3 chunks +8 lines, -3 lines 0 comments Download
M content/content_browser.gypi View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M content/content_common.gypi View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/java/gin_java_bridge_dispatcher.h View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M content/renderer/java/gin_java_bridge_dispatcher.cc View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M content/renderer/java/gin_java_bridge_object.cc View 1 2 2 chunks +5 lines, -3 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
mnaganov (inactive)
Hi Torne, This is the last big patch completing the new implementation. Nothing is hooked ...
6 years, 6 months ago (2014-06-19 12:26:33 UTC) #1
Torne
https://codereview.chromium.org/345753003/diff/1/content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc File content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc (right): https://codereview.chromium.org/345753003/diff/1/content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc#newcode207 content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc:207: // java object is alive. no longer alive? https://codereview.chromium.org/345753003/diff/1/content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc#newcode259 ...
6 years, 6 months ago (2014-06-23 10:53:10 UTC) #2
mnaganov (inactive)
https://codereview.chromium.org/345753003/diff/1/content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc File content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc (right): https://codereview.chromium.org/345753003/diff/1/content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc#newcode207 content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc:207: // java object is alive. On 2014/06/23 10:53:10, Torne ...
6 years, 6 months ago (2014-06-23 12:13:06 UTC) #3
Torne
On 2014/06/23 12:13:06, Mikhail Naganov (Cr) wrote: > https://codereview.chromium.org/345753003/diff/1/content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc > File content/browser/renderer_host/java/gin_java_bridge_dispatcher_host.cc > (right): > ...
6 years, 6 months ago (2014-06-23 12:58:17 UTC) #4
mnaganov (inactive)
The CQ bit was checked by mnaganov@chromium.org
6 years, 6 months ago (2014-06-23 14:21:06 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mnaganov@chromium.org/345753003/20001
6 years, 6 months ago (2014-06-23 14:22:18 UTC) #6
mnaganov (inactive)
The CQ bit was unchecked by mnaganov@chromium.org
6 years, 6 months ago (2014-06-23 14:22:26 UTC) #7
mnaganov (inactive)
palmer@: Chris, can you please take a look at the gin_java_bridge_messages.h change?
6 years, 6 months ago (2014-06-23 14:23:52 UTC) #8
palmer
https://codereview.chromium.org/345753003/diff/20001/content/common/gin_java_bridge_messages.h File content/common/gin_java_bridge_messages.h (right): https://codereview.chromium.org/345753003/diff/20001/content/common/gin_java_bridge_messages.h#newcode56 content/common/gin_java_bridge_messages.h:56: std::string /* error_message */) It's generally better to send ...
6 years, 6 months ago (2014-06-25 22:22:33 UTC) #9
mnaganov (inactive)
https://codereview.chromium.org/345753003/diff/20001/content/common/gin_java_bridge_messages.h File content/common/gin_java_bridge_messages.h (right): https://codereview.chromium.org/345753003/diff/20001/content/common/gin_java_bridge_messages.h#newcode56 content/common/gin_java_bridge_messages.h:56: std::string /* error_message */) On 2014/06/25 22:22:33, Chromium Palmer ...
6 years, 6 months ago (2014-06-26 13:41:35 UTC) #10
mnaganov (inactive)
benm@: After addressing palmer@'s comment, I now need your approval for content/common/android. PTAL!
6 years, 6 months ago (2014-06-26 13:42:58 UTC) #11
palmer
LGTM. Thanks!
6 years, 6 months ago (2014-06-26 19:25:07 UTC) #12
benm (inactive)
lgtm
6 years, 6 months ago (2014-06-26 19:39:12 UTC) #13
mnaganov (inactive)
The CQ bit was checked by mnaganov@chromium.org
6 years, 5 months ago (2014-06-27 08:32:37 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mnaganov@chromium.org/345753003/40001
6 years, 5 months ago (2014-06-27 08:35:22 UTC) #15
commit-bot: I haz the power
6 years, 5 months ago (2014-06-27 10:47:33 UTC) #16
Message was sent while issue was closed.
Change committed as 280298

Powered by Google App Engine
This is Rietveld 408576698