Removed UploadAppInstallReport method

UploadAppInstallReport method was used for AppInstall Reporting so that
DMServerJobConfiguration is created and events are reported to Chrome
Reporting API using DM server as a proxy. Now the events are reported
directly to Chrome Reporting API. UploadAppInstallReport is no longer
used.

Bug: 1051464
Change-Id: Ic13f334fa207499852f52d433b08c083799975fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049861Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Askar Aitzhan <askaraitzhan@google.com>
Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
Cr-Commit-Position: refs/heads/master@{#741403}

chrome/browser/chromeos/policy/app_install_event_log_manager_unittest.cc

@@ -327,7 +327,6 @@ class AppInstallEventLogManagerTest : public testing::Test {
 // Create a manager with an empty log. Verify that no store is scheduled and no
 // upload occurs.
 TEST_F(AppInstallEventLogManagerTest, CreateEmpty) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   FastForwardUntilNoTasksRemain();
@@ -343,7 +342,6 @@ TEST_F(AppInstallEventLogManagerTest, CreateNonEmpty) {
   log.Add(kPackageNames[0], event_);
   log.Store();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
   base::DeleteFile(log_file_path_, false /* recursive */);
 
@@ -357,7 +355,6 @@ TEST_F(AppInstallEventLogManagerTest, CreateNonEmpty) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -366,7 +363,6 @@ TEST_F(AppInstallEventLogManagerTest, CreateNonEmpty) {
 // five seconds and an expedited initial upload occurs after a total of fifteen
 // minutes.
 TEST_F(AppInstallEventLogManagerTest, AddBeforeInitialUpload) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(2);
@@ -389,7 +385,6 @@ TEST_F(AppInstallEventLogManagerTest, AddBeforeInitialUpload) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -398,7 +393,6 @@ TEST_F(AppInstallEventLogManagerTest, AddBeforeInitialUpload) {
 // stores are scheduled after five and eleven seconds and an upload occurs
 // after three hours.
 TEST_F(AppInstallEventLogManagerTest, Add) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -437,7 +431,6 @@ TEST_F(AppInstallEventLogManagerTest, Add) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -446,7 +439,6 @@ TEST_F(AppInstallEventLogManagerTest, Add) {
 // that a store is scheduled after five seconds and an upload occurs after three
 // hours.
 TEST_F(AppInstallEventLogManagerTest, AddForMultipleApps) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -469,7 +461,6 @@ TEST_F(AppInstallEventLogManagerTest, AddForMultipleApps) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -477,7 +468,6 @@ TEST_F(AppInstallEventLogManagerTest, AddForMultipleApps) {
 // Wait twenty minutes. Add an identical log entry for an empty set of apps.
 // Verify that no store is scheduled and no upload occurs.
 TEST_F(AppInstallEventLogManagerTest, AddForZeroApps) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -492,7 +482,6 @@ TEST_F(AppInstallEventLogManagerTest, AddForZeroApps) {
 // threshold for expedited upload. Verify that a store is scheduled after five
 // seconds and an upload occurs after fifteen minutes.
 TEST_F(AppInstallEventLogManagerTest, AddToTriggerMaxSizeExpedited) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -517,7 +506,6 @@ TEST_F(AppInstallEventLogManagerTest, AddToTriggerMaxSizeExpedited) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -526,7 +514,6 @@ TEST_F(AppInstallEventLogManagerTest, AddToTriggerMaxSizeExpedited) {
 // exceeds the threshold for expedited upload. Verify that a store is scheduled
 // after five seconds and an upload occurs after fifteen minutes.
 TEST_F(AppInstallEventLogManagerTest, AddToTriggerTotalSizeExpedited) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -554,7 +541,6 @@ TEST_F(AppInstallEventLogManagerTest, AddToTriggerTotalSizeExpedited) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -565,7 +551,6 @@ TEST_F(AppInstallEventLogManagerTest, AddToTriggerTotalSizeExpedited) {
 // minutes.
 TEST_F(AppInstallEventLogManagerTest,
        AddForMultipleAppsToTriggerTotalSizeExpedited) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -591,7 +576,6 @@ TEST_F(AppInstallEventLogManagerTest,
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -601,7 +585,6 @@ TEST_F(AppInstallEventLogManagerTest,
 // entry. Complete the upload. Verify that the pending log entry is stored.
 // Then, verify that a regular upload occurs three hours later.
 TEST_F(AppInstallEventLogManagerTest, RequestUploadAddUpload) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
   AddLogEntry(0 /* app_index */);
 
@@ -625,7 +608,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestUploadAddUpload) {
   ReportUploadSuccess(std::move(upload_callback));
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardTo(kExpeditedUploadDelay + kUploadInterval - kOneMs);
   Mock::VerifyAndClearExpectations(&cloud_policy_client_);
   EXPECT_FALSE(base::PathExists(log_file_path_));
@@ -636,7 +618,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestUploadAddUpload) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -647,7 +628,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestUploadAddUpload) {
 // the upload. Verify that the pending log entries are stored. Then, verify that
 // an expedited upload occurs fifteen minutes later.
 TEST_F(AppInstallEventLogManagerTest, RequestUploadAddExpeditedUpload) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
   AddLogEntry(0 /* app_index */);
 
@@ -673,7 +653,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestUploadAddExpeditedUpload) {
   ReportUploadSuccess(std::move(upload_callback));
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardTo(kExpeditedUploadDelay + kExpeditedUploadDelay - kOneMs);
   Mock::VerifyAndClearExpectations(&cloud_policy_client_);
   EXPECT_FALSE(base::PathExists(log_file_path_));
@@ -684,7 +663,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestUploadAddExpeditedUpload) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -695,7 +673,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestUploadAddExpeditedUpload) {
 // entry. Complete the upload. Verify that the pending log entry is stored.
 // Then, verify that a regular upload occurs three hours later.
 TEST_F(AppInstallEventLogManagerTest, RequestExpeditedUploadAddUpload) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   const base::TimeDelta offset = base::TimeDelta::FromMinutes(20);
@@ -725,7 +702,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestExpeditedUploadAddUpload) {
   ReportUploadSuccess(std::move(upload_callback));
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardTo(offset + kExpeditedUploadDelay + kUploadInterval - kOneMs);
   Mock::VerifyAndClearExpectations(&cloud_policy_client_);
   EXPECT_FALSE(base::PathExists(log_file_path_));
@@ -736,7 +712,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestExpeditedUploadAddUpload) {
   events_.clear();
   VerifyAndDeleteLogFile();
 
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   FastForwardUntilNoTasksRemain();
   EXPECT_FALSE(base::PathExists(log_file_path_));
 }
@@ -744,7 +719,6 @@ TEST_F(AppInstallEventLogManagerTest, RequestExpeditedUploadAddUpload) {
 // Add a log entry. Destroy the manager. Verify that an immediate store is
 // scheduled during destruction.
 TEST_F(AppInstallEventLogManagerTest, StoreOnShutdown) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   AddLogEntry(0 /* app_index */);
@@ -790,7 +764,6 @@ TEST_F(AppInstallEventLogManagerTest, Clear) {
 // cleared. Create a manager. Verify that the log file is deleted before the
 // manager attempts to load it.
 TEST_F(AppInstallEventLogManagerTest, RunClearRun) {
-  EXPECT_CALL(cloud_policy_client_, UploadAppInstallReport(_, _)).Times(0);
   CreateManager();
 
   AddLogEntry(0 /* app_index */);

components/policy/core/common/cloud/cloud_policy_client.cc

@@ -554,29 +554,6 @@ void CloudPolicyClient::UploadRealtimeReport(base::Value report,
   request_jobs_.push_back(service_->CreateJob(std::move(config)));
 }
 
-void CloudPolicyClient::UploadAppInstallReport(
-    const em::AppInstallReportRequest* app_install_report,
-    StatusCallback callback) {
-  CHECK(is_registered());
-  DCHECK(app_install_report);
-
-  std::unique_ptr<DMServerJobConfiguration> config = std::make_unique<
-      DMServerJobConfiguration>(
-      DeviceManagementService::JobConfiguration::TYPE_UPLOAD_APP_INSTALL_REPORT,
-      this,
-      /*critical=*/false, DMAuth::FromDMToken(dm_token_),
-      /*oauth_token=*/base::nullopt,
-      base::BindOnce(&CloudPolicyClient::OnReportUploadCompleted,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
-
-  *config->request()->mutable_app_install_report_request() =
-      *app_install_report;
-
-  CancelAppInstallReportUpload();
-  request_jobs_.push_back(service_->CreateJob(std::move(config)));
-  app_install_report_request_job_ = request_jobs_.back().get();
-}
-
 void CloudPolicyClient::CancelAppInstallReportUpload() {
   if (app_install_report_request_job_) {
     RemoveJob(app_install_report_request_job_);

components/policy/core/common/cloud/cloud_policy_client.h

@@ -264,13 +264,6 @@ class POLICY_EXPORT CloudPolicyClient {
   virtual void UploadRealtimeReport(base::Value report,
                                     StatusCallback callback);
 
-  // Uploads a report on the status of app push-installs. The client must be in
-  // a registered state. The |callback| will be called when the operation
-  // completes.
-  virtual void UploadAppInstallReport(
-      const enterprise_management::AppInstallReportRequest* app_install_report,
-      StatusCallback callback);
-
   // Cancels the pending app push-install status report upload, if an.
   virtual void CancelAppInstallReportUpload();
 

components/policy/core/common/cloud/cloud_policy_client_unittest.cc

@@ -76,7 +76,6 @@ const char kResultPayload[] = "output_payload";
 const char kAssetId[] = "fake-asset-id";
 const char kLocation[] = "fake-location";
 const char kGcmID[] = "fake-gcm-id";
-const char kPackageName[] = "com.example.app";
 const char kPolicyToken[] = "fake-policy-token";
 const char kPolicyName[] = "fake-policy-name";
 const char kValueValidationMessage[] = "fake-value-validation-message";
@@ -492,20 +491,6 @@ class CloudPolicyClientTest : public testing::Test {
                                    gcm_id_update_response_)));
   }
 
-  // Expects an TYPE_UPLOAD_APP_INSTALL_REPORT job to be started.  The job
-  // completes at the next call to base::RunLoop().RunUntilIdle() using the
-  // supplied |request|.
-  void ExpectUploadAppInstallReport(
-      const em::DeviceManagementRequest& request) {
-    EXPECT_CALL(service_, StartJob(_))
-        .WillOnce(DoAll(
-            service_.CaptureJobType(&job_type_),
-            service_.CaptureQueryParams(&query_params_),
-            service_.CaptureRequest(&job_request_),
-            service_.StartJobAsync(net::OK, DeviceManagementService::kSuccess,
-                                   upload_app_install_report_response_)));
-  }
-
   void CheckPolicyResponse() {
     ASSERT_TRUE(client_->GetPolicyFor(policy_type_, std::string()));
     EXPECT_THAT(*client_->GetPolicyFor(policy_type_, std::string()),
@@ -1827,90 +1812,6 @@ TEST_F(CloudPolicyClientTest, RequestGcmIdUpdate) {
             gcm_id_update_request_.SerializePartialAsString());
 }
 
-TEST_F(CloudPolicyClientTest, UploadAppInstallReport) {
-  RegisterClient();
-  em::DeviceManagementRequest request;
-  request.mutable_app_install_report_request();
-  ExpectUploadAppInstallReport(request);
-  EXPECT_CALL(callback_observer_, OnCallbackComplete(true)).Times(1);
-
-  CloudPolicyClient::StatusCallback callback =
-      base::BindOnce(&MockStatusCallbackObserver::OnCallbackComplete,
-                     base::Unretained(&callback_observer_));
-
-  em::AppInstallReportRequest app_install_report;
-  client_->UploadAppInstallReport(&app_install_report, std::move(callback));
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(
-      DeviceManagementService::JobConfiguration::TYPE_UPLOAD_APP_INSTALL_REPORT,
-      job_type_);
-  EXPECT_EQ(DM_STATUS_SUCCESS, client_->status());
-}
-
-TEST_F(CloudPolicyClientTest, CancelUploadAppInstallReport) {
-  RegisterClient();
-  em::DeviceManagementRequest request;
-  request.mutable_app_install_report_request();
-  ExpectUploadAppInstallReport(request);
-  EXPECT_CALL(callback_observer_, OnCallbackComplete(true)).Times(0);
-
-  CloudPolicyClient::StatusCallback callback =
-      base::BindOnce(&MockStatusCallbackObserver::OnCallbackComplete,
-                     base::Unretained(&callback_observer_));
-
-  em::AppInstallReportRequest app_install_report;
-  client_->UploadAppInstallReport(&app_install_report, std::move(callback));
-  EXPECT_EQ(1, client_->GetActiveRequestCountForTest());
-
-  // The job expected by the call to ExpectUploadAppInstallReport() completes
-  // when base::RunLoop().RunUntilIdle() is called.  To simulate a cancel
-  // before the response for the request is processed, make sure to cancel it
-  // before running a loop.
-  client_->CancelAppInstallReportUpload();
-  EXPECT_EQ(0, client_->GetActiveRequestCountForTest());
-  EXPECT_EQ(
-      DeviceManagementService::JobConfiguration::TYPE_UPLOAD_APP_INSTALL_REPORT,
-      job_type_);
-}
-
-TEST_F(CloudPolicyClientTest, UploadAppInstallReportSupersedesPending) {
-  RegisterClient();
-  em::DeviceManagementRequest request;
-  request.mutable_app_install_report_request();
-  ExpectUploadAppInstallReport(request);
-  EXPECT_CALL(callback_observer_, OnCallbackComplete(true)).Times(0);
-
-  CloudPolicyClient::StatusCallback callback =
-      base::BindOnce(&MockStatusCallbackObserver::OnCallbackComplete,
-                     base::Unretained(&callback_observer_));
-
-  em::AppInstallReportRequest app_install_report;
-  client_->UploadAppInstallReport(&app_install_report, std::move(callback));
-  EXPECT_EQ(1, client_->GetActiveRequestCountForTest());
-  Mock::VerifyAndClearExpectations(&service_);
-  Mock::VerifyAndClearExpectations(&callback_observer_);
-
-  // Starting another app push-install report upload should cancel the pending
-  // one.
-  request.mutable_app_install_report_request()
-      ->add_app_install_reports()
-      ->set_package(kPackageName);
-  ExpectUploadAppInstallReport(request);
-  EXPECT_CALL(callback_observer_, OnCallbackComplete(true)).Times(1);
-
-  callback = base::BindOnce(&MockStatusCallbackObserver::OnCallbackComplete,
-                            base::Unretained(&callback_observer_));
-  app_install_report.CopyFrom(request.app_install_report_request());
-  client_->UploadAppInstallReport(&app_install_report, std::move(callback));
-  EXPECT_EQ(1, client_->GetActiveRequestCountForTest());
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(
-      DeviceManagementService::JobConfiguration::TYPE_UPLOAD_APP_INSTALL_REPORT,
-      job_type_);
-  EXPECT_EQ(DM_STATUS_SUCCESS, client_->status());
-  EXPECT_EQ(0, client_->GetActiveRequestCountForTest());
-}
-
 TEST_F(CloudPolicyClientTest, PolicyReregistration) {
   RegisterClient();
 

components/policy/core/common/cloud/mock_cloud_policy_client.h

@@ -66,9 +66,6 @@ class MockCloudPolicyClient : public CloudPolicyClient {
                     const enterprise_management::SessionStatusReportRequest*,
                     const enterprise_management::ChildStatusReportRequest*,
                     StatusCallback&));
-  MOCK_METHOD2(UploadAppInstallReport,
-               void(const enterprise_management::AppInstallReportRequest*,
-                    StatusCallback callback));
   MOCK_METHOD0(CancelAppInstallReportUpload, void(void));
   void UpdateGcmId(const std::string& id, StatusCallback callback) override {
     UpdateGcmId_(id, callback);