📤 Upstream SafeBrowsingApiBridge.

Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207
Committed: https://crrev.com/917bb8e15aaf0a392819aa2ef6b2229805450f89
Cr-Commit-Position: refs/heads/master@{#390939}

[Yaron, 2016-04-29 18:58:23]

yfriedman@chromium.org changed reviewers:
+ nparker@chromium.org, nyquist@chromium.org

[Yaron, 2016-04-29 19:02:47]

Description was changed from

==========
Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207
==========

to

==========
Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207
==========

[nyquist, 2016-04-29 20:01:34]

lgtm

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiBridge.java
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiBridge.java:17:
public class SafeBrowsingApiBridge {
I thinks this amazing class definition deserves the 'final' keyword.

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiBridge.java:29:
* @param handler
Nit: This @param isn't really helpful.

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
File chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h:35: 
Nit: remove empty line?

[Nathan Parker, 2016-04-29 20:42:04]

lgtm

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiHandler.java
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiHandler.java:21:
* Observer to be notified when the SafeBrowsingApiHandler computes the score for
a url.
s/computes the score/determines the verdict

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiHandler.java:28:
@Retention(RetentionPolicy.SOURCE)
What does Retention and IntDef do?

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/chro...
File chrome/browser/android/chrome_jni_registrar.cc (right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/chro...
chrome/browser/android/chrome_jni_registrar.cc:354: #if
defined(SAFE_BROWSING_DB_REMOTE)
This should always be defined on Android, so you can remove the ifdef.

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
File chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.cc
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.cc:155:
AttachCurrentThread(), GetApplicationContext());
On public-android builds, this will return null... yes?

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
File chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h:29: //
|threat_types| is currently ignored.
rm second line of comment (since threat_types is not ignored.)

[Yaron, 2016-05-02 13:25:07]

thanks

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiBridge.java
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiBridge.java:17:
public class SafeBrowsingApiBridge {
On 2016/04/29 20:01:34, nyquist wrote:
> I thinks this amazing class definition deserves the 'final' keyword.

Done.

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiBridge.java:29:
* @param handler
On 2016/04/29 20:01:34, nyquist wrote:
> Nit: This @param isn't really helpful.

Done.

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiHandler.java
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiHandler.java:21:
* Observer to be notified when the SafeBrowsingApiHandler computes the score for
a url.
On 2016/04/29 20:42:04, Nathan Parker wrote:
> s/computes the score/determines the verdict

Done.

https://codereview.chromium.org/1940443002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/safe_browsing/SafeBrowsingApiHandler.java:28:
@Retention(RetentionPolicy.SOURCE)
On 2016/04/29 20:42:04, Nathan Parker wrote:
> What does Retention and IntDef do?

Retention source indicates that it shouldn't be kept in runtime .class files.
This allows us to specify it for type-checking but not impact runtime size.

@IntDef is effectively an int enum but without the overhead

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/chro...
File chrome/browser/android/chrome_jni_registrar.cc (right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/chro...
chrome/browser/android/chrome_jni_registrar.cc:354: #if
defined(SAFE_BROWSING_DB_REMOTE)
On 2016/04/29 20:42:04, Nathan Parker wrote:
> This should always be defined on Android, so you can remove the ifdef.

Done.

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
File chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.cc
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.cc:155:
AttachCurrentThread(), GetApplicationContext());
On 2016/04/29 20:42:04, Nathan Parker wrote:
> On public-android builds, this will return null... yes?

yep, the java side of the bridge needs to be given a .class to use

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
File chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h
(right):

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h:29: //
|threat_types| is currently ignored.
On 2016/04/29 20:42:04, Nathan Parker wrote:
> rm second line of comment (since threat_types is not ignored.)

Done.

https://codereview.chromium.org/1940443002/diff/1/chrome/browser/android/safe...
chrome/browser/android/safe_browsing/safe_browsing_api_handler_bridge.h:35: 
On 2016/04/29 20:01:34, nyquist wrote:
> Nit: remove empty line?

Done.

[Yaron, 2016-05-02 13:25:14]

The CQ bit was checked by yfriedman@chromium.org

[Yaron, 2016-05-02 13:25:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, nparker@chromium.org
Link to the patchset: https://codereview.chromium.org/1940443002/#ps20001
(title: "comments")

[commit-bot: I haz the power, 2016-05-02 13:25:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1940443002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1940443002/20001

[commit-bot: I haz the power, 2016-05-02 14:50:42]

Description was changed from

==========
Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207
==========

to

==========
Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207
==========

[commit-bot: I haz the power, 2016-05-02 14:50:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-02 14:52:41]

Description was changed from

==========
Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207
==========

to

==========
Upstream SafeBrowsingApiBridge.

Implements a JNI bridge for the SafeBrowsingApiHandler. When in use,
a pure-java handler can implement the SafeBrowsing interface.
BUG=607207

Committed: https://crrev.com/917bb8e15aaf0a392819aa2ef6b2229805450f89
Cr-Commit-Position: refs/heads/master@{#390939}
==========

[commit-bot: I haz the power, 2016-05-02 14:52:46]

Patchset 2 (id:??) landed as
https://crrev.com/917bb8e15aaf0a392819aa2ef6b2229805450f89
Cr-Commit-Position: refs/heads/master@{#390939}