Shape Detection Android: unmap() and close() SharedBufferHandles explicitly

Shape Detection Android: unmap() and close() SharedBufferHandles explicitly


This CL unmap()s and close()s the handle after it has been used,
preventing the harmless but annoying Log [1]

05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.

and optimising resource usage by not having to wait until
Handle finalize() to release them.


[1] https://cs.chromium.org/chromium/src/mojo/android/system/src/org/chromium/mojo/system/impl/HandleBase.java?type=cs&l=133


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

Review-Url: https://codereview.chromium.org/2873093003
Cr-Commit-Position: refs/heads/master@{#470990}
Committed: https://chromium.googlesource.com/chromium/src/+/672bf2b83170847857eea5ac6d0e2f1f71dacac4

[mcasas, 2017-05-10 19:10:05]

Description was changed from

==========
Shape Detection Android: unmap() and close() explicitly the SharedBufferHandle

unmap and close the handle


05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.


BUG=720234
==========

to

==========
Shape Detection Android: unmap() and close() explicitly the SharedBufferHandle

unmap and close the handle


05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.


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

[mcasas, 2017-05-10 19:12:47]

Description was changed from

==========
Shape Detection Android: unmap() and close() explicitly the SharedBufferHandle

unmap and close the handle


05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.


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

to

==========
Shape Detection Android: unmap() and close() SharedBufferHandles explicitly


This CL unmap()s and close()s the handle after it has been used,
preventing the harmless but annoying Log [1]

05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.

and optimising resource usage by not having to wait until
Handle finalize() to release them.


[1]
https://cs.chromium.org/chromium/src/mojo/android/system/src/org/chromium/moj...


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

[mcasas, 2017-05-10 19:17:21]

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

[mcasas, 2017-05-10 19:17:22]

rockot@  tiny CL, ptal  (or fwd appropriately, thx!)

[Ken Rockot(use gerrit already), 2017-05-11 15:42:08]

https://codereview.chromium.org/2873093003/diff/1/services/shape_detection/an...
File
services/shape_detection/android/java/src/org/chromium/shape_detection/SharedBufferUtils.java
(right):

https://codereview.chromium.org/2873093003/diff/1/services/shape_detection/an...
services/shape_detection/android/java/src/org/chromium/shape_detection/SharedBufferUtils.java:39:
try {
My Mojo-Java-fu is weak. Why is this wrapped in a try block with an empty
exception handler? What can go wrong that we A) need to trap here and B) is
appropriate to ignore?

[mcasas, 2017-05-11 16:02:48]

https://codereview.chromium.org/2873093003/diff/1/services/shape_detection/an...
File
services/shape_detection/android/java/src/org/chromium/shape_detection/SharedBufferUtils.java
(right):

https://codereview.chromium.org/2873093003/diff/1/services/shape_detection/an...
services/shape_detection/android/java/src/org/chromium/shape_detection/SharedBufferUtils.java:39:
try {
On 2017/05/11 15:42:07, Ken Rockot wrote:
> My Mojo-Java-fu is weak. Why is this wrapped in a try block with an empty
> exception handler? What can go wrong that we A) need to trap here and B) is
> appropriate to ignore?

In Java-land, if it can throw, we have to catch it:
https://source.android.com/source/code-style#dont-ignore-exceptions

 and both unmap() and close() end up in code that throws:
https://cs.chromium.org/chromium/src/mojo/android/system/src/org/chromium/moj...
https://cs.chromium.org/chromium/src/mojo/android/system/src/org/chromium/moj...
 
Catching-ignoring is fine (we could add a Log but meh)
vs the alternative of crashing Chrome :-)

[Ken Rockot(use gerrit already), 2017-05-11 16:31:51]

lgtm

[mcasas, 2017-05-11 16:33:56]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2017-05-11 16:34:46]

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-11 17:16:55]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494520436869570, "parent_rev":
"f1610b19b843e8cdc00a215074f9c9e04de9dd17", "commit_rev":
"672bf2b83170847857eea5ac6d0e2f1f71dacac4"}

[commit-bot: I haz the power, 2017-05-11 17:17:04]

Description was changed from

==========
Shape Detection Android: unmap() and close() SharedBufferHandles explicitly


This CL unmap()s and close()s the handle after it has been used,
preventing the harmless but annoying Log [1]

05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.

and optimising resource usage by not having to wait until
Handle finalize() to release them.


[1]
https://cs.chromium.org/chromium/src/mojo/android/system/src/org/chromium/moj...


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

to

==========
Shape Detection Android: unmap() and close() SharedBufferHandles explicitly


This CL unmap()s and close()s the handle after it has been used,
preventing the harmless but annoying Log [1]

05-10 12:05:23.985 30381 30389 W HandleImpl: Handle was not closed.

and optimising resource usage by not having to wait until
Handle finalize() to release them.


[1]
https://cs.chromium.org/chromium/src/mojo/android/system/src/org/chromium/moj...


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

Review-Url: https://codereview.chromium.org/2873093003
Cr-Commit-Position: refs/heads/master@{#470990}
Committed:
https://chromium.googlesource.com/chromium/src/+/672bf2b83170847857eea5ac6d0e...
==========

[commit-bot: I haz the power, 2017-05-11 17:17:05]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/672bf2b83170847857eea5ac6d0e...