[Android] Remove deprecated SysInfo property retrieval

[Android] Remove deprecated SysInfo property retrieval

Android's SysInfo implementation currently uses an internal, hidden API.
This is not safe. Switch all relevant SysInfo methods to use their
BuildInfo counterpart, which uses sanctioned approaches for fetching the
same values.

BUG=392191

[jdduke (slow), 2015-07-20 19:52:52]

Patchset #2 (id:20001) has been deleted

[jdduke (slow), 2015-07-20 19:59:23]

jdduke@chromium.org changed reviewers:
+ thakis@chromium.org

[jdduke (slow), 2015-07-20 19:59:24]

+dfalcantara@ to make sure I'm getting the mappings right.
+thakis@ for base/.

https://codereview.chromium.org/1236283002/diff/40001/base/android/build_info.h
File base/android/build_info.h (left):

https://codereview.chromium.org/1236283002/diff/40001/base/android/build_info...
base/android/build_info.h:67: const char* android_build_id() const {
I'm happy to restore the "android" prefix here but it's a bit redundant and
inconsistent with naming for the rest of the getters.

[jdduke (slow), 2015-08-05 00:03:34]

jdduke@chromium.org changed reviewers:
+ nyquist@chromium.org, thestig@chromium.org
- thakis@chromium.org

[jdduke (slow), 2015-08-05 00:03:35]

+nyquist for base/android
-thakis, +thestig for the rest of base/

[Lei Zhang, 2015-08-05 00:28:24]

base/ lgtm if nyquist@ agrees.

chrome/ and components/crash lgtm

https://codereview.chromium.org/1236283002/diff/40001/base/sys_info_android.cc
File base/sys_info_android.cc (right):

https://codereview.chromium.org/1236283002/diff/40001/base/sys_info_android.c...
base/sys_info_android.cc:54: LOG(ERROR) << "Invalid large memory class: " <<
result;
nit: outputting |result| isn't helpful. We already know it's -1.

[Lei Zhang, 2015-08-05 00:29:21]

BTW, is bug 509018 the right bug? It looks somewhat related but is already
marked as fixed.

[jdduke (slow), 2015-08-05 02:46:24]

On 2015/08/05 00:29:21, Lei Zhang wrote:
> BTW, is bug 509018 the right bug? It looks somewhat related but is already
> marked as fixed.

Well, it's the most relevant bug =/ The change that fixed that issue is what
originally prompted this patch.

[Alexei Svitkine (slow), 2015-08-05 15:21:30]

asvitkine@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2015-08-05 15:21:30]

components/metrics lgtm

[jdduke (slow), 2015-08-05 15:29:20]

jdduke@chromium.org changed reviewers:
+ sievers@chromium.org

[jdduke (slow), 2015-08-05 15:29:20]

+sievers for content/browser/gpu

[jdduke (slow), 2015-08-05 15:32:09]

https://codereview.chromium.org/1236283002/diff/40001/base/sys_info_android.cc
File base/sys_info_android.cc (right):

https://codereview.chromium.org/1236283002/diff/40001/base/sys_info_android.c...
base/sys_info_android.cc:54: LOG(ERROR) << "Invalid large memory class: " <<
result;
On 2015/08/05 00:28:24, Lei Zhang wrote:
> nit: outputting |result| isn't helpful. We already know it's -1.

Done.

[no sievers, 2015-08-05 17:29:13]

On 2015/08/05 15:29:20, jdduke wrote:
> +sievers for content/browser/gpu

lgtm

[nyquist, 2015-08-06 06:42:26]

base/android lgtm

[jdduke (slow), 2015-08-06 13:11:43]

The CQ bit was checked by jdduke@chromium.org

[jdduke (slow), 2015-08-06 13:11:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1236283002/#ps60001
(title: "Code review")

[commit-bot: I haz the power, 2015-08-06 13:11:53]

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

[commit-bot: I haz the power, 2015-08-06 13:51:58]

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

[commit-bot: I haz the power, 2015-08-06 13:51:59]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)

[jdduke (slow), 2015-08-06 15:25:02]

Hmm. Looks like there's a remaining __system_property_get dependency in
dns_config_service_posix.cc, so we'll either need to move the dlsym workaround
there or modify ReadDnsConfig to get the values differently.

[jdduke (slow), 2015-08-06 15:25:02]

Hmm. Looks like there's a remaining __system_property_get dependency in
dns_config_service_posix.cc, so we'll either need to move the dlsym workaround
there or modify ReadDnsConfig to get the values differently.

[jdduke (slow), 2015-08-06 15:25:04]

Hmm. Looks like there's a remaining __system_property_get dependency in
dns_config_service_posix.cc, so we'll either need to move the dlsym workaround
there or modify ReadDnsConfig to get the values differently.

[jdduke (slow), 2015-08-06 15:25:04]

Hmm. Looks like there's a remaining __system_property_get dependency in
dns_config_service_posix.cc, so we'll either need to move the dlsym workaround
there or modify ReadDnsConfig to get the values differently.

[jdduke (slow), 2015-08-06 15:26:45]

Hmm. Looks like there's a remaining __system_property_get dependency in
dns_config_service_posix.cc, so we'll either need to move the dlsym workaround
there or modify ReadDnsConfig to get the values differently.

ttuttle@: I notice you have a TODO there to fix the usage, any thoughts?

[jdduke (slow), 2015-08-06 15:34:47]

On 2015/08/06 15:26:45, jdduke wrote:
> Hmm. Looks like there's a remaining __system_property_get dependency in
> dns_config_service_posix.cc, so we'll either need to move the dlsym workaround
> there or modify ReadDnsConfig to get the values differently.
> 
> ttuttle@: I notice you have a TODO there to fix the usage, any thoughts?

I guess we could built a lightweight JNI wrapper for the Java System class,
letting us use the static 'System.getProperty' directly. Not ideal, but should
be fine for infrequent use.

[Deprecated (see juliatuttle), 2015-08-07 20:56:05]

On 2015/08/06 15:34:47, jdduke wrote:
> On 2015/08/06 15:26:45, jdduke wrote:
> > Hmm. Looks like there's a remaining __system_property_get dependency in
> > dns_config_service_posix.cc, so we'll either need to move the dlsym
workaround
> > there or modify ReadDnsConfig to get the values differently.
> > 
> > ttuttle@: I notice you have a TODO there to fix the usage, any thoughts?
> 
> I guess we could built a lightweight JNI wrapper for the Java System class,
> letting us use the static 'System.getProperty' directly. Not ideal, but should
> be fine for infrequent use.

My plan was to hook into Android's networking stuff in a similar way, but that
doesn't work on older versions (the data isn't available for non-wifi networks).
I think your idea sounds good if we still need this data. It's useful for the
improved DNS error pages, at least.