Create policy ReportArcStatus

chrome/browser/chromeos/policy/device_status_collector_browsertest.cc

Index: chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
diff --git a/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc b/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
index 284cd44022242aa2ff0f0d13c78c92866411dd33..cfbd7fd568ba5267bf8b1ce22ab0dbaf64d4052c 100644
--- a/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
+++ b/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
@@ -27,6 +27,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_data.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
+#include "chrome/browser/chromeos/arc/arc_auth_service.h"
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
 #include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
 #include "chrome/browser/chromeos/ownership/fake_owner_settings_service.h"
@@ -225,6 +226,17 @@ void CallAndroidStatusReceiver(
   receiver.Run(status, droid_guard_info);
 }
 
+bool GetFakeAndroidStatus(
+    mojo::String status,
+    mojo::String droid_guard_info,
+    const policy::DeviceStatusCollector::AndroidStatusReceiver& receiver) {
+  // Post it to the thread because this call is expected to be asynchronous.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, Bind(&CallAndroidStatusReceiver, receiver,

[bartfab (slow), 2016/10/10 15:49:16]

Nit: s/Bind/base::Bind/ (why does this even compile? Who is defining Bind in the
global namespace? :( )

[phweiss, 2016/10/11 12:14:39]

Done.

+          status, droid_guard_info));
+  return true;
+}
+
 bool GetEmptyAndroidStatus(
     const policy::DeviceStatusCollector::AndroidStatusReceiver& receiver) {
   // Post it to the thread because this call is expected to be asynchronous.
@@ -982,9 +994,52 @@ TEST_F(DeviceStatusCollectorTest, TestCPUTemp) {
   EXPECT_EQ(0, device_status_.cpu_temp_info_size());
 }
 
-TEST_F(DeviceStatusCollectorTest, NoSessionStatusIfNotKioskMode) {
+TEST_F(DeviceStatusCollectorTest, TestAndroidReporting) {
+  mojo::String status = "{\"applications\":[ { "

[bartfab (slow), 2016/10/10 15:49:16]

Here and below:

Nit 1: const.
Nit 2: You can use an std::string here. It will be automatically converted to a
mojo::String as needed.

[phweiss, 2016/10/11 12:14:39]

Done.

+      "\"packageName\":\"com.android.providers.telephony\","
+      "\"versionName\":\"6.0.1\","
+      "\"permissions\": [\"android.permission.INTERNET\"] }],"
+      "\"userEmail\":\"xxx@google.com\"}";
+  mojo::String droid_guard_info = "{\"droid_guard_info\":42}";
+  RestartStatusCollector(base::Bind(&GetEmptyVolumeInfo),
+                         base::Bind(&GetEmptyCPUStatistics),
+                         base::Bind(&GetEmptyCPUTempInfo),
+                         base::Bind(&GetFakeAndroidStatus, status,
+                             droid_guard_info));
+  GetStatus();
+  EXPECT_EQ(status, session_status_.android_status().status_payload());
+  EXPECT_EQ(droid_guard_info,
+      session_status_.android_status().droid_guard_info());
+}
+
+TEST_F(DeviceStatusCollectorTest, NoAndroidReportingWhenDisabled) {
+  mojo::String status = "{\"applications\":[ { "

[bartfab (slow), 2016/10/10 15:49:16]

Nit: Can you move these lengthy constants to an anonymous namespace at the top
of the file?

[phweiss, 2016/10/11 12:14:39]

Done.

+      "\"packageName\":\"com.android.providers.telephony\","
+      "\"versionName\":\"6.0.1\","
+      "\"permissions\": [\"android.permission.INTERNET\"] }],"
+      "\"userEmail\":\"xxx@google.com\"}";
+  mojo::String droid_guard_info = "{\"droid_guard_info\":42}";
+  RestartStatusCollector(base::Bind(&GetEmptyVolumeInfo),
+                         base::Bind(&GetEmptyCPUStatistics),
+                         base::Bind(&GetEmptyCPUTempInfo),
+                         base::Bind(&GetFakeAndroidStatus, status,
+                             droid_guard_info));
+
+  prefs_.SetBoolean(prefs::kReportArcStatus, false);
+  // Mock Kiosk app, so some session status is reported
+  status_collector_->set_kiosk_account(
+      base::MakeUnique<policy::DeviceLocalAccount>(fake_device_local_account_));

[bartfab (slow), 2016/10/10 15:49:16]

Nit: s/policy:://

[phweiss, 2016/10/11 12:14:39]

Done.

+  MockRunningKioskApp(fake_device_local_account_);
+
+  GetStatus();
+  EXPECT_TRUE(got_session_status_);
+  EXPECT_FALSE(session_status_.has_android_status());
+}
+
+TEST_F(DeviceStatusCollectorTest, NoSessionStatusIfNotKioskAndNoArcReporting) {
   // Should not report session status if we don't have an active kiosk app.
   settings_helper_.SetBoolean(chromeos::kReportDeviceSessionStatus, true);
+  prefs_.SetBoolean(prefs::kReportArcStatus, false);
   GetStatus();
   EXPECT_FALSE(got_session_status_);
 }
@@ -992,6 +1047,7 @@ TEST_F(DeviceStatusCollectorTest, NoSessionStatusIfNotKioskMode) {
 TEST_F(DeviceStatusCollectorTest, NoSessionStatusIfSessionReportingDisabled) {
   // Should not report session status if session status reporting is disabled.
   settings_helper_.SetBoolean(chromeos::kReportDeviceSessionStatus, false);
+  prefs_.SetBoolean(prefs::kReportArcStatus, false);

[bartfab (slow), 2016/10/10 15:49:16]

Nit: Even if this was true, reproting should return nothing because it is off
overall. Is it worth having one more test case for this?

[phweiss, 2016/10/11 12:14:39]

In the current implementation, kReportDeviceSessionStatus only controls the
kiosk session reporting (as it says in its documentation), and is independent of
ARC reporting. You need to turn both off to receive nothing. I added a comment.

   status_collector_->set_kiosk_account(
       base::MakeUnique<policy::DeviceLocalAccount>(fake_device_local_account_));
   // Set up a device-local account for single-app kiosk mode.