Always get device conditions from Java for every attempt.

chrome/browser/android/offline_pages/background_scheduler_bridge.cc

Index: chrome/browser/android/offline_pages/background_scheduler_bridge.cc
diff --git a/chrome/browser/android/offline_pages/background_scheduler_bridge.cc b/chrome/browser/android/offline_pages/background_scheduler_bridge.cc
index 9c59ebb6180918612532f3ed7eb8948f81d7268f..dadfd27fb365aaeb8cdec3665409ce435572ea6e 100644
--- a/chrome/browser/android/offline_pages/background_scheduler_bridge.cc
+++ b/chrome/browser/android/offline_pages/background_scheduler_bridge.cc
@@ -56,6 +56,10 @@ static jboolean StartScheduledProcessing(
       device_conditions, base::Bind(&ProcessingDoneCallback, j_callback_ref));
 }
 
+BackgroundSchedulerBridge::BackgroundSchedulerBridge() {}
+
+BackgroundSchedulerBridge::~BackgroundSchedulerBridge() {}
+
 void BackgroundSchedulerBridge::Schedule(
     const TriggerConditions& trigger_conditions) {
   JNIEnv* env = base::android::AttachCurrentThread();
@@ -92,6 +96,49 @@ ScopedJavaLocalRef<jobject> BackgroundSchedulerBridge::CreateTriggerConditions(
       require_unmetered_network);
 }
 
+DeviceConditions& BackgroundSchedulerBridge::GetCurrentDeviceConditions() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  // Convert return value from Java to C++

[dougarnett, 2017/01/27 23:15:56]

nit - period

+  // Call the Java function, put the returned object into a scoped local ref.
+  const ScopedJavaLocalRef<jobject>& java_conditions_object =
+      Java_BackgroundSchedulerBridge_getCurrentDeviceConditions(env);
+  if (java_conditions_object.is_null()) {

[dewittj, 2017/01/27 23:03:43]

nit: no braces

[dougarnett, 2017/01/27 23:15:56]

nit: keep braces if comment stays 

[Pete Williamson, 2017/01/28 00:24:46]

Moved comment above if statement, and lost the braces.

[Pete Williamson, 2017/01/28 00:24:46]

Done.

+    // JNI call failed somehow, so use the last known conditions.
+    return *(device_conditions_.get());
+  }
+  jobject java_conditions =
+      reinterpret_cast<jobject>(java_conditions_object.obj());

[dewittj, 2017/01/27 23:03:43]

this cast is not necessary.

[Pete Williamson, 2017/01/28 00:24:46]

Done.

+
+  // Get the corrensponding class object so we can do introspection.
+  ScopedJavaLocalRef<jclass> device_conditions_clazz = base::android::GetClass(
+      env, "org/chromium/chrome/browser/offlinepages/DeviceConditions");
+
+  // Find the field ID to use to get the member variables.
+  jfieldID power_field_id =
+      env->GetFieldID(device_conditions_clazz.obj(), "mPowerConnected", "Z");

[dewittj, 2017/01/27 23:03:43]

I'd like to recommend you use a more standard Chromium pattern, such as in this
file:
https://cs.chromium.org/chromium/src/base/android/unguessable_token_android.c...

The primary advantage is type checking on both sides - the implementation here
will only fail at runtime if the java fields are renamed.

This is the first usage of this call in all of Chrome besides third_party.  I'd
like you to get someone more experienced with JNI than me to review if you want
to retain bare JNI.  Additionally this is prone to runtime failure if these
members are ever renamed.

[Pete Williamson, 2017/01/28 00:24:46]

Done.

+  jfieldID battery_field_id =
+      env->GetFieldID(device_conditions_clazz.obj(), "mBatteryPercentage", "I");
+  jfieldID network_field_id =
+      env->GetFieldID(device_conditions_clazz.obj(), "mNetConnectionType", "I");
+
+  // Using the field ID, get the member variables from the object.
+  jboolean jpower = env->GetBooleanField(java_conditions, power_field_id);
+  jint jbattery = env->GetIntField(java_conditions, battery_field_id);
+  jint jnetwork = env->GetIntField(java_conditions, network_field_id);
+
+  // Cast the java types back to the types we use.
+  bool power = static_cast<bool>(jpower);
+  int battery = static_cast<int>(jbattery);
+  int network_int = static_cast<int>(jnetwork);

[dewittj, 2017/01/27 23:03:43]

just do the single cast to ConnectionType

[Pete Williamson, 2017/01/28 00:24:46]

Done.

+  net::NetworkChangeNotifier::ConnectionType network_connection_type =
+      static_cast<net::NetworkChangeNotifier::ConnectionType>(network_int);
+
+  // Now return the current conditions.
+  DeviceConditions device_conditions(power, battery, network_connection_type);
+  device_conditions_.reset(new DeviceConditions(device_conditions));

[dewittj, 2017/01/27 23:03:43]

device_conditions_ = base::MakeUnique<DeviceConditions>(power, battery,
network_connection_type)

Saves an extra construction and copy

[Pete Williamson, 2017/01/28 00:24:46]

Done.

+  return *(device_conditions_.get());
+}
+
 bool RegisterBackgroundSchedulerBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }