Android: Integrates native and java SystemMonitor.

base/system_monitor/system_monitor.h

Index: base/system_monitor/system_monitor.h
diff --git a/base/system_monitor/system_monitor.h b/base/system_monitor/system_monitor.h
index ea95da89ccbe9fb6635b8ae905b6415ac977ca24..5fb06ece1ed8f52ad9ec7d115e4014852834b520 100644
--- a/base/system_monitor/system_monitor.h
+++ b/base/system_monitor/system_monitor.h
@@ -38,6 +38,10 @@
 #include <objc/runtime.h>
 #endif  // OS_IOS
 
+#if defined(OS_ANDROID)
+#include <jni.h>

[vandebo (ex-Chrome), 2012/10/04 17:55:56]

Can you declare instead of include?

[bulach, 2012/10/04 18:33:33]

unfortunately not.. :( there are some nasty typedefs and indirections for the
JNIEnv..

+#endif
+
 namespace base {
 
 // Class for monitoring various system-related subsystems
@@ -95,6 +99,11 @@ class BASE_EXPORT SystemMonitor {
 #endif  // OS_IOS
 #endif  // OS_MACOSX
 
+#if defined(OS_ANDROID)
+  static bool RegisterSystemMonitor(JNIEnv* env);

[vandebo (ex-Chrome), 2012/10/04 17:55:56]

I'm not familiar with how JNI is being used in Chrome, so please forgive me if
this is a silly question, but why does this need to be on this class?

[bulach, 2012/10/04 18:33:33]

there's never a silly question, just poorly documented code :)
so let me clarify:
we need a way to bind / register the java side methods with the native ones. we
do all this registration via:
http://codereview.chromium.org/11027024/diff/2002/base/android/base_jni_regis...

so this method doesn't _need_ to be here, but since there are other
platform-specific initializations above, it seemed a good idea.
I could of course create a header for system_monitor_android.h and expose this
method there, but that wouldn't be entirely in line with other platforms..

happy to go either way anyways.

[vandebo (ex-Chrome), 2012/10/04 19:19:22]

If it doesn't need to be a member of the class, then doing a separate header
file sounds good.

+  void SetBatteryIsCharging(bool is_battery_charging);
+#endif
+
   // Returns information for attached removable storage.
   std::vector<RemovableStorageInfo> GetAttachedRemovableStorage() const;
 
@@ -224,6 +233,11 @@ class BASE_EXPORT SystemMonitor {
   std::vector<id> notification_observers_;
 #endif
 
+#if defined(OS_ANDROID)
+  // Holds the battery state from the java side.
+  bool is_battery_charging_;

[Satish, 2012/10/04 14:31:45]

Good idea. This could also be used on windows so before sending the power
message this variable can be set by the code in system_monitor_win.cc and the
method IsBatteryPower() can be removed altogether. That way there is lesser
android specific code here.

But I'm fine with leaving this as is if OWNERS are happy with this approach.

[vandebo (ex-Chrome), 2012/10/04 17:55:56]

But adding SetBatteryIsCharging exposes more of the internal  state of
SystemMonitor as public.  I much prefer adding a parameter to the
ProcesssPowerMessage method.  It can be a generic argument that is interpreted
differently depending on the powerevent it.

[bulach, 2012/10/04 18:33:33]

this was all to avoid one extra JNI hop.. since this method shouldn't be in the
critical path, and other platforms already call into the system to get the
state, I changed this to pull from java rather than the other way around..

+#endif
+
   // For manipulating removable_storage_map_ structure.
   mutable base::Lock removable_storage_lock_;
   // Map of all the attached removable storage devices.