Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to //services/shape_detection

Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to //services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


No new code or functionality is intended in the ...Impl.java
files (except a small change in testing for GMS presence).

BUG=718275
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2855203003
Cr-Commit-Position: refs/heads/master@{#469424}
Committed: https://chromium.googlesource.com/chromium/src/+/5b95074ff70c578c5eef6f1201f23acec24f29c8

[mcasas, 2017-05-04 00:48:57]

Description was changed from

==========
Shape Detection: move BarcodeDetectionImpl and TextDetectionImpl.java from
//chrome to //services/shape_detection


Moved BarcodeDetectionImpl.java and TextDetectionImpl.java from //chrome to
//services/shape_detection, and rewired the service

BUG=659138
==========

to

==========
Shape Detection: move BarcodeDetectionImpl and TextDetectionImpl.java from
//chrome to //services/shape_detection


Moved BarcodeDetectionImpl.java and TextDetectionImpl.java from //chrome to
//services/shape_detection, and rewired the service

BUG=659138
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-04 00:55:50]

Description was changed from

==========
Shape Detection: move BarcodeDetectionImpl and TextDetectionImpl.java from
//chrome to //services/shape_detection


Moved BarcodeDetectionImpl.java and TextDetectionImpl.java from //chrome to
//services/shape_detection, and rewired the service

BUG=659138
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


BUG=659138
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-04 01:03:35]

Description was changed from

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


BUG=659138
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


No new code or functionality is intended in the ...Impl.java
files (except a small change in testing for GMS presence).

BUG=659138
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-04 01:04:19]

mcasas@chromium.org changed reviewers:
+ reillyg@chromium.org, rockot@chromium.org

[mcasas, 2017-05-04 01:04:20]

reillyg@/ rockot@  PTAL

[Reilly Grant (use Gerrit), 2017-05-04 01:24:36]

This looks reasonable to me but I'm not familiar with the details of service
registration or GMSCore usage in the Device Service so I'd like someone with
that domain knowledge to comment.

[mcasas, 2017-05-04 01:57:39]

Description was changed from

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


No new code or functionality is intended in the ...Impl.java
files (except a small change in testing for GMS presence).

BUG=659138
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


No new code or functionality is intended in the ...Impl.java
files (except a small change in testing for GMS presence).

BUG=718275
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[Ken Rockot(use gerrit already), 2017-05-04 17:19:50]

lgtm

https://codereview.chromium.org/2855203003/diff/1/services/shape_detection/sh...
File services/shape_detection/shape_detection_service.h (right):

https://codereview.chromium.org/2855203003/diff/1/services/shape_detection/sh...
services/shape_detection/shape_detection_service.h:39:
service_manager::InterfaceProvider java_interface_provider_;
nit: Just use a unique_ptr<InterfaceProvider> instead of inlining and having a
separate bool "initialized" flag?

[mcasas, 2017-05-04 17:58:50]

https://codereview.chromium.org/2855203003/diff/1/services/shape_detection/sh...
File services/shape_detection/shape_detection_service.h (right):

https://codereview.chromium.org/2855203003/diff/1/services/shape_detection/sh...
services/shape_detection/shape_detection_service.h:39:
service_manager::InterfaceProvider java_interface_provider_;
On 2017/05/04 17:19:50, Ken Rockot wrote:
> nit: Just use a unique_ptr<InterfaceProvider> instead of inlining and having a
> separate bool "initialized" flag?

Yeah!  I thought I recall somewhere (maybe "Do's and
Don'ts in Chromium"?) to not do that, but it still sgtm,
will do soon.

[mcasas, 2017-05-04 17:58:55]

mcasas@chromium.org changed reviewers:
+ avi@chromium.org, tedchoc@chromium.org

[mcasas, 2017-05-04 17:58:56]

tedchoc@ RS plz code removal from chrome/android

avi@ RS plz small reshuffling in render_process_host_impl.cc

[Ken Rockot(use gerrit already), 2017-05-04 18:02:06]

I don't think any such advice should exist anywhere in our documentation.
The only reason to avoid unique_ptr in cases like that is where the
performance impact of heap allocations is relevant. That wouldn't be the
case here. Of course another option, which avoids heap allocation, would be
to use base::Optional instead of std::unique_ptr. unique_ptr is more
idiomatic so I'd still prefer it if performance isn't a concern.

On Thu, May 4, 2017 at 10:58 AM, <mcasas@chromium.org> wrote:

> tedchoc@ RS plz code removal from chrome/android
>
> avi@ RS plz small reshuffling in render_process_host_impl.cc
>
>
>
> https://codereview.chromium.org/2855203003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Avi (use Gerrit), 2017-05-04 18:02:28]

rph lgtm

[Ted C, 2017-05-04 18:12:53]

On 2017/05/04 18:02:28, Avi (ping after 24h) wrote:
> rph lgtm

chrome/android lgtm

[mcasas, 2017-05-04 18:15:55]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 18:16:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 19:18:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 19:18:28]

Dry run: This issue passed the CQ dry run.

[mcasas, 2017-05-04 19:26:42]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2017-05-04 19:26:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, rockot@chromium.org, avi@chromium.org
Link to the patchset: https://codereview.chromium.org/2855203003/#ps20001
(title: "Make |java_interface_provider_| a std::unique_ptr<>")

[commit-bot: I haz the power, 2017-05-04 19:27:18]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 19:34:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493926002393550,
"parent_rev": "47e0be3e13416190fe9038e1e2f23b0be50bb8b7", "commit_rev":
"5b95074ff70c578c5eef6f1201f23acec24f29c8"}

[commit-bot: I haz the power, 2017-05-04 19:35:06]

Description was changed from

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


No new code or functionality is intended in the ...Impl.java
files (except a small change in testing for GMS presence).

BUG=718275
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: move {BarcodeDetectionImpl,TextDetectionImpl}.java to
//services/shape_detection

This CL:

- moves those two fellas (BarcodeDetectionImpl.java and
TextDetectionImpl.java) from //chrome to //services/shape_detection,
with some tiny adaptations.

- changes render_process_host_impl.cc to send requests to the
said service.

- adds a needed InterfaceRegistrar.java to //services/shape_detection
and some android-specific code to shape_detection_service.cc, 
following the  //services/device model (BatteryMonitor, concretely).


No new code or functionality is intended in the ...Impl.java
files (except a small change in testing for GMS presence).

BUG=718275
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2855203003
Cr-Commit-Position: refs/heads/master@{#469424}
Committed:
https://chromium.googlesource.com/chromium/src/+/5b95074ff70c578c5eef6f1201f2...
==========

[commit-bot: I haz the power, 2017-05-04 19:35:07]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5b95074ff70c578c5eef6f1201f2...