Always get device conditions from Java for every attempt.

chrome/browser/android/offline_pages/background_scheduler_bridge.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/android/callback_android.h"
 #include "base/android/scoped_java_ref.h"
 #include "chrome/browser/android/offline_pages/background_scheduler_bridge.h"
 #include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
 #include "chrome/browser/android/offline_pages/request_coordinator_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 38 matching lines @@
       GetForBrowserContext(profile);
   DVLOG(2) << "resource_coordinator: " << coordinator;
   DeviceConditions device_conditions(
       j_power_connected, j_battery_percentage,
       static_cast<net::NetworkChangeNotifier::ConnectionType>(
           j_net_connection_type));
   return coordinator->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();
   ScopedJavaLocalRef<jobject> j_conditions =
       CreateTriggerConditions(env, trigger_conditions.require_power_connected,
                               trigger_conditions.minimum_battery_percentage,
                               trigger_conditions.require_unmetered_network);
   Java_BackgroundSchedulerBridge_schedule(env, j_conditions);
 }
 
@@ skipping 16 matching lines @@
 ScopedJavaLocalRef<jobject> BackgroundSchedulerBridge::CreateTriggerConditions(
     JNIEnv* env,
     bool require_power_connected,
     int minimum_battery_percentage,
     bool require_unmetered_network) const {
   return Java_BackgroundSchedulerBridge_createTriggerConditions(
       env, require_power_connected, minimum_battery_percentage,
       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);
 }
 
 }  // namespace android
 }  // namespace offline_pages