Report app usage up to current time and account for multiple app states

Reports app info activity up to the current time instead of to the end
of the previous day. Account for concurrent states in app instance
updates. Give better messages for app activity matchers.

Bug: 1058453
Change-Id: I06274894768eb5a5c222fcfdea0bce903dc512f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2249271Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Matt Menard <mattme@google.com>
Cr-Commit-Position: refs/heads/master@{#779847}

chrome/browser/chromeos/policy/status_collector/app_info_generator.cc

@@ -99,9 +99,7 @@ const AppInfoGenerator::Result AppInfoGenerator::Generate() const {
     VLOG(1) << "No affiliated user session. Returning empty app list.";
     return base::nullopt;
   }
-
-  auto activity_periods =
-      provider_->activity_storage.GetActivityPeriods(clock_.Now());
+  auto activity_periods = provider_->activity_storage.GetActivityPeriods();
   auto activity_compare = [](const em::TimePeriod& time_period1,
                              const em::TimePeriod& time_period2) {
     return time_period1.start_timestamp() < time_period2.start_timestamp();
@@ -136,7 +134,8 @@ void AppInfoGenerator::OnReportedSuccessfully(const base::Time report_time) {
   if (!provider_) {
     return;
   }
-  provider_->activity_storage.PruneActivityPeriods(report_time);
+  provider_->activity_storage.TrimActivityPeriods(
+      report_time.ToJavaTime(), base::Time::Max().ToJavaTime());
 }
 
 void AppInfoGenerator::OnWillReport() {
@@ -230,7 +229,7 @@ void AppInfoGenerator::SetIdleDurationsToOpen() {
   base::Time start_time = clock_.Now();
   provider_->app_service_proxy.InstanceRegistry().ForEachInstance(
       [this, start_time](const apps::InstanceUpdate& update) {
-        if (update.State() == apps::InstanceState::kStarted) {
+        if (update.State() & apps::InstanceState::kStarted) {
           OpenUsageInterval(update.AppId(), update.Window(), start_time);
         }
       });

chrome/browser/chromeos/policy/status_collector/app_info_generator_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "base/strings/stringprintf.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_clock.h"
@@ -39,11 +40,15 @@
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
+using ::testing::ElementsAreArray;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
+using ::testing::MatcherInterface;
 using ::testing::Matches;
+using ::testing::MatchResultListener;
 using ::testing::Property;
+using ::testing::ResultOf;
 
 using web_app::ProviderType;
 
@@ -51,30 +56,65 @@ namespace em = enterprise_management;
 
 namespace {
 
-MATCHER_P(EqualsProto,
-          message,
-          "Match a proto Message equal to the matcher's argument.") {
-  std::string expected_serialized, actual_serialized;
-  message.SerializeToString(&expected_serialized);
-  arg.SerializeToString(&actual_serialized);
-  return expected_serialized == actual_serialized;
-}
+class TimePeriodMatcher : public MatcherInterface<const em::TimePeriod&> {
+ public:
+  explicit TimePeriodMatcher(const em::TimePeriod& time_period)
+      : start_(base::Time::FromJavaTime(time_period.start_timestamp())),
+        end_(base::Time::FromJavaTime(time_period.end_timestamp())) {}
+
+  bool MatchAndExplain(const em::TimePeriod& time_period,
+                       MatchResultListener* listener) const override {
+    bool start_timestamp_equal =
+        time_period.start_timestamp() == start_.ToJavaTime();
+    if (!start_timestamp_equal) {
+      *listener << " |start_timestamp| is "
+                << base::Time::FromJavaTime(time_period.start_timestamp());
+    }
+    bool end_timestamp_equal = time_period.end_timestamp() == end_.ToJavaTime();
+    if (!end_timestamp_equal) {
+      *listener << " |end_timestamp| is "
+                << base::Time::FromJavaTime(time_period.end_timestamp());
+    }
+    return start_timestamp_equal && end_timestamp_equal;
+  }
+
+  void DescribeTo(std::ostream* os) const override {
+    *os << "equal to TimePeriod"
+        << "(" << start_ << ", " << end_ << "]";
+  }
+
+ private:
+  base::Time start_;
+  base::Time end_;
+};
 
 auto EqApp(const std::string& app_id,
            const std::string& name,
            const em::AppInfo::Status status,
            const std::string& version,
            const em::AppInfo::AppType app_type,
-           const std::vector<em::TimePeriod> app_activity = {}) {
-  em::AppInfo app_info;
-  app_info.set_app_id(app_id);
-  app_info.set_app_name(name);
-  app_info.set_status(status);
-  app_info.set_version(version);
-  app_info.set_app_type(app_type);
-  *app_info.mutable_active_time_periods() = {app_activity.begin(),
-                                             app_activity.end()};
-  return EqualsProto(app_info);
+           const std::vector<em::TimePeriod> app_activities = {}) {
+  auto GetActivity =
+      [](const em::AppInfo& input) -> std::vector<em::TimePeriod> {
+    std::vector<em::TimePeriod> activities;
+    for (const em::TimePeriod& time_period : input.active_time_periods()) {
+      activities.push_back(time_period);
+    }
+    return activities;
+  };
+
+  std::vector<Matcher<const em::TimePeriod&>> activity_matchers;
+  for (const em::TimePeriod& activity : app_activities) {
+    activity_matchers.push_back(
+        Matcher<const em::TimePeriod&>(new TimePeriodMatcher(activity)));
+  }
+
+  return AllOf(Property(&em::AppInfo::app_id, app_id),
+               Property(&em::AppInfo::app_name, name),
+               Property(&em::AppInfo::status, status),
+               Property(&em::AppInfo::version, version),
+               Property(&em::AppInfo::app_type, app_type),
+               ResultOf(GetActivity, ElementsAreArray(activity_matchers)));
 }
 
 auto MakeActivity(const base::Time& start_time, const base::Time& end_time) {
@@ -119,7 +159,7 @@ class AppInfoGeneratorTest : public ::testing::TestWithParam<ProviderType> {
 
   class Instance {
    public:
-    Instance(const std::string& app_id) {
+    explicit Instance(const std::string& app_id) {
       window_ = std::make_unique<aura::Window>(nullptr);
       window_->Init(ui::LAYER_NOT_DRAWN);
       instance_ = std::make_unique<apps::Instance>(app_id, window_.get());
@@ -335,14 +375,17 @@ TEST_P(AppInfoGeneratorTest, OnReportedSuccessfully) {
   generator->OnReportedSuccessfully(report_time);
 
   test_clock().SetNow(MakeLocalTime("31-MAR-2020 5:30pm"));
+  PushAppInstance(app_instance, apps::InstanceState::kStarted);
+  test_clock().SetNow(MakeLocalTime("31-MAR-2020 11:30pm"));
+  PushAppInstance(app_instance, apps::InstanceState::kDestroyed);
   auto result = generator->Generate();
 
   EXPECT_THAT(
       result.value(),
       ElementsAre(EqApp("a", "FirstApp", em::AppInfo_Status_STATUS_DISABLED,
                         "1.1", em::AppInfo_AppType_TYPE_ARC,
-                        {MakeActivity(MakeUTCTime("30-MAR-2020 12:00am"),
-                                      MakeUTCTime("30-MAR-2020 11:30pm"))})));
+                        {MakeActivity(MakeUTCTime("31-MAR-2020 12:00am"),
+                                      MakeUTCTime("31-MAR-2020 6:00am"))})));
 }
 
 TEST_P(AppInfoGeneratorTest, OnWillReport) {
@@ -351,7 +394,9 @@ TEST_P(AppInfoGeneratorTest, OnWillReport) {
           apps::mojom::AppType::kArc);
   Instance app_instance("a");
   test_clock().SetNow(MakeLocalTime("29-MAR-2020 3:30pm"));
-  PushAppInstance(app_instance, apps::InstanceState::kStarted);
+  PushAppInstance(app_instance, static_cast<apps::InstanceState>(
+                                    apps::InstanceState::kStarted |
+                                    apps::InstanceState::kHidden));
 
   test_clock().SetNow(MakeLocalTime("30-MAR-2020 5:30pm"));
   generator->OnWillReport();
@@ -366,7 +411,9 @@ TEST_P(AppInfoGeneratorTest, OnWillReport) {
       ElementsAre(EqApp("a", "FirstApp", em::AppInfo_Status_STATUS_DISABLED,
                         "1.1", em::AppInfo_AppType_TYPE_ARC,
                         {MakeActivity(MakeUTCTime("29-MAR-2020 12:00am"),
-                                      MakeUTCTime("29-MAR-2020 8:30am"))})));
+                                      MakeUTCTime("29-MAR-2020 8:30am")),
+                         MakeActivity(MakeUTCTime("30-MAR-2020 12:00am"),
+                                      MakeUTCTime("30-MAR-2020 5:30pm"))})));
   EXPECT_THAT(
       result2.value(),
       ElementsAre(EqApp("a", "FirstApp", em::AppInfo_Status_STATUS_DISABLED,
@@ -374,7 +421,9 @@ TEST_P(AppInfoGeneratorTest, OnWillReport) {
                         {MakeActivity(MakeUTCTime("29-MAR-2020 12:00am"),
                                       MakeUTCTime("29-MAR-2020 8:30am")),
                          MakeActivity(MakeUTCTime("30-MAR-2020 12:00am"),
-                                      MakeUTCTime("31-MAR-2020 12:00am"))})));
+                                      MakeUTCTime("31-MAR-2020 12:00am")),
+                         MakeActivity(MakeUTCTime("31-MAR-2020 12:00am"),
+                                      MakeUTCTime("31-MAR-2020 8:30pm"))})));
 }
 
 TEST_P(AppInfoGeneratorTest, OnLogoutOnLogin) {

chrome/browser/chromeos/policy/status_collector/device_status_collector_browsertest.cc

@@ -3266,17 +3266,25 @@ TEST_F(DeviceStatusCollectorTest, GenerateAppInfo) {
   test_clock_.SetNow(report_time);
   GetStatus();
 
-  EXPECT_EQ(session_status_.app_infos(0).app_id(), "id");
-  EXPECT_EQ(session_status_.app_infos(0).active_time_periods_size(), 1);
-  auto first = session_status_.app_infos(0).active_time_periods()[0];
   base::Time reported_start_time;
+  base::Time reported_end_time;
+  EXPECT_EQ(session_status_.app_infos(0).app_id(), "id");
+  EXPECT_EQ(session_status_.app_infos(0).active_time_periods_size(), 2);
+  auto first_activity = session_status_.app_infos(0).active_time_periods()[0];
   EXPECT_TRUE(
       base::Time::FromUTCString("29-MAR-2020 12:00am", &reported_start_time));
-  EXPECT_EQ(first.start_timestamp(), reported_start_time.ToJavaTime());
-  base::Time reported_end_time;
   EXPECT_TRUE(
       base::Time::FromUTCString("29-MAR-2020 10:30am", &reported_end_time));
-  EXPECT_EQ(first.end_timestamp(), reported_end_time.ToJavaTime());
+  EXPECT_EQ(first_activity.start_timestamp(), reported_start_time.ToJavaTime());
+  EXPECT_EQ(first_activity.end_timestamp(), reported_end_time.ToJavaTime());
+  auto second_activity = session_status_.app_infos(0).active_time_periods()[1];
+  EXPECT_TRUE(
+      base::Time::FromUTCString("30-MAR-2020 12:00am", &reported_start_time));
+  EXPECT_TRUE(
+      base::Time::FromUTCString("30-MAR-2020 2:30pm", &reported_end_time));
+  EXPECT_EQ(second_activity.start_timestamp(),
+            reported_start_time.ToJavaTime());
+  EXPECT_EQ(second_activity.end_timestamp(), reported_end_time.ToJavaTime());
   EXPECT_EQ(session_status_.app_infos(1).app_id(), "id2");
   EXPECT_EQ(session_status_.app_infos(1).active_time_periods_size(), 0);
 }