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

Issue 2825233002: Android: Refactor BuildInfo to use less jni and remove StrictMode exception (Closed)

Created:
3 years, 8 months ago by agrieve
Modified:
3 years, 8 months ago
Reviewers:
Torne
CC:
chromium-reviews, danakj+watch_chromium.org, agrieve+watch_chromium.org, vmpstr+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Android: Refactor BuildInfo to use less jni and remove StrictMode exception No behaviour change expected. Strictly a simplifying refactoring. Review-Url: https://codereview.chromium.org/2825233002 Cr-Commit-Position: refs/heads/master@{#466630} Committed: https://chromium.googlesource.com/chromium/src/+/c4cb0d38f0bd26578120775e9c09be813b5215c0

Patch Set 1 #

Patch Set 2 : rebase #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -134 lines) Patch
M base/android/build_info.h View 1 2 chunks +10 lines, -9 lines 1 comment Download
M base/android/build_info.cc View 1 2 chunks +33 lines, -27 lines 0 comments Download
M base/android/java/src/org/chromium/base/BuildInfo.java View 1 5 chunks +29 lines, -98 lines 2 comments Download

Messages

Total messages: 21 (13 generated)
agrieve
https://codereview.chromium.org/2825233002/diff/20001/base/android/build_info.h File base/android/build_info.h (right): https://codereview.chromium.org/2825233002/diff/20001/base/android/build_info.h#newcode126 base/android/build_info.h:126: const char* const brand_; re-ordered to match the order ...
3 years, 8 months ago (2017-04-21 17:58:37 UTC) #8
agrieve
On 2017/04/21 17:58:37, agrieve wrote: > https://codereview.chromium.org/2825233002/diff/20001/base/android/build_info.h > File base/android/build_info.h (right): > > https://codereview.chromium.org/2825233002/diff/20001/base/android/build_info.h#newcode126 > ...
3 years, 8 months ago (2017-04-21 19:37:00 UTC) #12
Torne
lgtm https://codereview.chromium.org/2825233002/diff/20001/base/android/java/src/org/chromium/base/BuildInfo.java File base/android/java/src/org/chromium/base/BuildInfo.java (right): https://codereview.chromium.org/2825233002/diff/20001/base/android/java/src/org/chromium/base/BuildInfo.java#newcode79 base/android/java/src/org/chromium/base/BuildInfo.java:79: return getAll()[10]; Can we just retrieve all the ...
3 years, 8 months ago (2017-04-21 19:41:25 UTC) #13
agrieve
https://codereview.chromium.org/2825233002/diff/20001/base/android/java/src/org/chromium/base/BuildInfo.java File base/android/java/src/org/chromium/base/BuildInfo.java (right): https://codereview.chromium.org/2825233002/diff/20001/base/android/java/src/org/chromium/base/BuildInfo.java#newcode79 base/android/java/src/org/chromium/base/BuildInfo.java:79: return getAll()[10]; On 2017/04/21 19:41:25, Torne wrote: > Can ...
3 years, 8 months ago (2017-04-21 20:00:33 UTC) #14
Torne
Yeah, okay. LGTM.
3 years, 8 months ago (2017-04-21 20:03:49 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2825233002/20001
3 years, 8 months ago (2017-04-24 13:45:24 UTC) #17
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/c4cb0d38f0bd26578120775e9c09be813b5215c0
3 years, 8 months ago (2017-04-24 14:26:59 UTC) #20
agrieve
3 years, 8 months ago (2017-04-24 15:43:05 UTC) #21
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2835113004/ by agrieve@chromium.org.

The reason for reverting is: Likely causing net_unittest failures.

e.g.:
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Mar....

Powered by Google App Engine
This is Rietveld 408576698