Make EncryptionModule EncryptRecord asynchronous.

This is in preparation to doing actual encryption.

Bug: b:153649905
Change-Id: I1b224debc46d60bd20a79e254bf2580bdb565835
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375731Reviewed-by: Zach Trudo <zatrudo@google.com>
Commit-Queue: Leonid Baraz <lbaraz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801521}

chrome/browser/policy/messaging_layer/encryption/encryption_module.cc

@@ -2,19 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <string>
-
 #include "chrome/browser/policy/messaging_layer/encryption/encryption_module.h"
+
+#include "base/callback.h"
+#include "base/strings/string_piece.h"
 #include "chrome/browser/policy/messaging_layer/util/status.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
+#include "components/policy/proto/record.pb.h"
 
 namespace reporting {
 
-// EncryptRecord will attempt to encrypt the provided |record|. On success the
-// return value will contain the encrypted string.
-StatusOr<std::string> EncryptionModule::EncryptRecord(
-    base::StringPiece record) const {
-  return Status(error::UNIMPLEMENTED, "EncryptRecord isn't implemented");
+void EncryptionModule::EncryptRecord(
+    base::StringPiece record,
+    base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) const {
+  std::move(cb).Run(
+      Status(error::UNIMPLEMENTED, "EncryptRecord isn't implemented"));
 }
 
 }  // namespace reporting

chrome/browser/policy/messaging_layer/encryption/encryption_module.h

@@ -5,16 +5,15 @@
 #ifndef CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_ENCRYPTION_MODULE_H_
 #define CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_ENCRYPTION_MODULE_H_
 
-#include <string>
-
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string_piece.h"
 #include "chrome/browser/policy/messaging_layer/util/status.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
+#include "components/policy/proto/record.pb.h"
 
 namespace reporting {
 
-// TODO(b/153659559) Temporary EncryptionModule until the real one is ready.
 class EncryptionModule : public base::RefCountedThreadSafe<EncryptionModule> {
  public:
   EncryptionModule() = default;
@@ -22,9 +21,13 @@ class EncryptionModule : public base::RefCountedThreadSafe<EncryptionModule> {
   EncryptionModule(const EncryptionModule& other) = delete;
   EncryptionModule& operator=(const EncryptionModule& other) = delete;
 
-  // EncryptRecord will attempt to encrypt the provided |record|. On success the
-  // return value will contain the encrypted string.
-  virtual StatusOr<std::string> EncryptRecord(base::StringPiece record) const;
+  // EncryptRecord will attempt to encrypt the provided |record| and respond
+  // with the callback. On success the returned EncryptedRecord will contain
+  // the encrypted string and encryption information. EncryptedRecord then can
+  // be further updated by the caller.
+  virtual void EncryptRecord(
+      base::StringPiece record,
+      base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) const;
 
  protected:
   virtual ~EncryptionModule() = default;

chrome/browser/policy/messaging_layer/encryption/test_encryption_module.cc

@@ -2,11 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <string>
-
 #include "chrome/browser/policy/messaging_layer/encryption/test_encryption_module.h"
 
+#include "base/callback.h"
+#include "base/strings/string_piece.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
+#include "components/policy/proto/record.pb.h"
 
 using ::testing::Invoke;
 
@@ -16,7 +17,13 @@ namespace test {
 TestEncryptionModule::TestEncryptionModule() {
   ON_CALL(*this, EncryptRecord)
       .WillByDefault(
-          Invoke([](base::StringPiece record) { return std::string(record); }));
+          Invoke([](base::StringPiece record,
+                    base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) {
+            EncryptedRecord encrypted_record;
+            encrypted_record.set_encrypted_wrapped_record(std::string(record));
+            // encryption_info is not set.
+            std::move(cb).Run(encrypted_record);
+          }));
 }
 
 TestEncryptionModule::~TestEncryptionModule() = default;

chrome/browser/policy/messaging_layer/encryption/test_encryption_module.h

@@ -5,10 +5,11 @@
 #ifndef CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_TEST_ENCRYPTION_MODULE_H_
 #define CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_TEST_ENCRYPTION_MODULE_H_
 
-#include <string>
-
+#include "base/callback.h"
+#include "base/strings/string_piece.h"
 #include "chrome/browser/policy/messaging_layer/public/report_queue.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
+#include "components/policy/proto/record.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -20,9 +21,10 @@ class TestEncryptionModule : public EncryptionModule {
  public:
   TestEncryptionModule();
 
-  MOCK_METHOD(StatusOr<std::string>,
+  MOCK_METHOD(void,
               EncryptRecord,
-              (base::StringPiece record),
+              (base::StringPiece record,
+               base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb),
               (const override));
 
  protected:

chrome/browser/policy/messaging_layer/public/report_queue.cc

@@ -8,6 +8,8 @@
 #include <string>
 #include <utility>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/json/json_writer.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
@@ -94,11 +96,27 @@ void ReportQueue::SendRecordToStorage(std::string record,
   ASSIGN_OR_ONCE_CALLBACK_AND_RETURN(WrappedRecord wrapped_record, callback,
                                      WrapRecord(record));
 
-  ASSIGN_OR_ONCE_CALLBACK_AND_RETURN(EncryptedRecord encrypted_record, callback,
-                                     EncryptRecord(wrapped_record));
+  std::string serialized_wrapped_record;
+  wrapped_record.SerializeToString(&serialized_wrapped_record);
 
-  storage_->AddRecord(encrypted_record, config_->priority(),
-                      std::move(callback));
+  encryption_->EncryptRecord(
+      serialized_wrapped_record,
+      base::BindOnce(
+          [](const Priority& priority, scoped_refptr<StorageModule> storage,
+             EnqueueCallback callback,
+             StatusOr<EncryptedRecord> encrypted_record_result) {
+            if (!encrypted_record_result.ok()) {
+              std::move(callback).Run(encrypted_record_result.status());
+              return;
+            }
+            // Complete EncryptedRecord.
+            auto& encrypted_record = encrypted_record_result.ValueOrDie();
+            auto* sequencing_information =
+                encrypted_record.mutable_sequencing_information();
+            sequencing_information->set_priority(priority);
+            storage->AddRecord(encrypted_record, priority, std::move(callback));
+          },
+          config_->priority(), storage_, std::move(callback)));
 }
 
 StatusOr<WrappedRecord> ReportQueue::WrapRecord(base::StringPiece record_data) {
@@ -124,22 +142,4 @@ StatusOr<std::string> ReportQueue::GetLastRecordDigest() {
   return "LastRecordDigest";
 }
 
-StatusOr<EncryptedRecord> ReportQueue::EncryptRecord(
-    WrappedRecord wrapped_record) {
-  std::string serialized_wrapped_record;
-  wrapped_record.SerializeToString(&serialized_wrapped_record);
-
-  ASSIGN_OR_RETURN(std::string encrypted_string_record,
-                   encryption_->EncryptRecord(serialized_wrapped_record));
-
-  EncryptedRecord encrypted_record;
-  encrypted_record.set_encrypted_wrapped_record(encrypted_string_record);
-
-  auto* sequencing_information =
-      encrypted_record.mutable_sequencing_information();
-  sequencing_information->set_priority(config_->priority());
-
-  return encrypted_record;
-}
-
 }  // namespace reporting

chrome/browser/policy/messaging_layer/public/report_queue.h

@@ -9,6 +9,7 @@
 #include <string>
 #include <utility>
 
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/sequence_checker.h"
@@ -82,8 +83,6 @@ class ReportQueue {
 
   StatusOr<reporting::WrappedRecord> WrapRecord(base::StringPiece record_data);
   StatusOr<std::string> GetLastRecordDigest();
-  StatusOr<reporting::EncryptedRecord> EncryptRecord(
-      reporting::WrappedRecord wrapped_record);
 
   std::unique_ptr<ReportQueueConfiguration> config_;
   scoped_refptr<StorageModule> storage_;

chrome/browser/policy/messaging_layer/public/report_queue_unittest.cc

@@ -237,8 +237,11 @@ TEST_F(ReportQueueTest, CallSuccessCallbackFailure) {
 // has been scheduled. The callback should fail, indicating that encryption was
 // unsuccessful.
 TEST_F(ReportQueueTest, EnqueueSuccessEncryptFailure) {
-  EXPECT_CALL(*test_encryption_module(), EncryptRecord(_))
-      .WillOnce(Return(Status(error::UNKNOWN, "Failing for tests")));
+  EXPECT_CALL(*test_encryption_module(), EncryptRecord(_, _))
+      .WillOnce(WithArg<1>(
+          Invoke([](base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) {
+            std::move(cb).Run(Status(error::UNKNOWN, "Failing for tests"));
+          })));
   reporting::test::TestMessage test_message;
   test_message.set_test("TEST_MESSAGE");
   TestEvent<Status> a;