Allow SourceStreamToDataPipe to self-own via callback

Currently, the callback is passed to SourceStreamToDataPipe in the
constructor.  This means that it has to be owned externally while the
callback is outstanding.  By moving the callback to Start, the
SourceStreamToDataPipe can be owned by the callback itself.

https://chromium-review.googlesource.com/c/chromium/src/+/1830360 uses
this logic.  Breaking this small patch out from there.

Change-Id: I93ad30f243e29f61e903dbd6b4ddfa37652e05c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841184
Auto-Submit: enne <enne@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703582}

content/browser/loader/source_stream_to_data_pipe.cc

@@ -11,11 +11,9 @@ namespace content {
 
 SourceStreamToDataPipe::SourceStreamToDataPipe(
     std::unique_ptr<net::SourceStream> source,
-    mojo::ScopedDataPipeProducerHandle dest,
-    base::OnceCallback<void(int)> completion_callback)
+    mojo::ScopedDataPipeProducerHandle dest)
     : source_(std::move(source)),
       dest_(std::move(dest)),
-      completion_callback_(std::move(completion_callback)),
       writable_handle_watcher_(FROM_HERE,
                                mojo::SimpleWatcher::ArmingPolicy::MANUAL,
                                base::SequencedTaskRunnerHandle::Get()) {
@@ -27,7 +25,9 @@ SourceStreamToDataPipe::SourceStreamToDataPipe(
 
 SourceStreamToDataPipe::~SourceStreamToDataPipe() = default;
 
-void SourceStreamToDataPipe::Start() {
+void SourceStreamToDataPipe::Start(
+    base::OnceCallback<void(int)> completion_callback) {
+  completion_callback_ = std::move(completion_callback);
   ReadMore();
 }
 

content/browser/loader/source_stream_to_data_pipe.h

@@ -24,12 +24,11 @@ class CONTENT_EXPORT SourceStreamToDataPipe {
  public:
   // Reads out the data from |source| and write into |dest|.
   SourceStreamToDataPipe(std::unique_ptr<net::SourceStream> source,
-                         mojo::ScopedDataPipeProducerHandle dest,
-                         base::OnceCallback<void(int)> completion_callback);
+                         mojo::ScopedDataPipeProducerHandle dest);
   ~SourceStreamToDataPipe();
 
   // Start reading the source.
-  void Start();
+  void Start(base::OnceCallback<void(int)> completion_callback);
   int64_t TransferredBytes() const { return transferred_bytes_; }
 
  private:

content/browser/loader/source_stream_to_data_pipe_unittest.cc

@@ -63,9 +63,12 @@ class SourceStreamToDataPipeTest
                                   &consumer_end_));
 
     adapter_ = std::make_unique<SourceStreamToDataPipe>(
-        std::move(source), std::move(producer_end),
-        base::BindOnce(&SourceStreamToDataPipeTest::FinishedReading,
-                       base::Unretained(this)));
+        std::move(source), std::move(producer_end));
+  }
+
+  base::OnceCallback<void(int)> callback() {
+    return base::BindOnce(&SourceStreamToDataPipeTest::FinishedReading,
+                          base::Unretained(this));
   }
 
   void CompleteReadsIfAsync() {
@@ -142,7 +145,7 @@ INSTANTIATE_TEST_SUITE_P(
 TEST_P(SourceStreamToDataPipeTest, EmptyStream) {
   Init();
   source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode);
-  adapter()->Start();
+  adapter()->Start(callback());
 
   std::string output;
   EXPECT_EQ(ReadPipe(&output), net::OK);
@@ -156,7 +159,7 @@ TEST_P(SourceStreamToDataPipeTest, Simple) {
   source()->AddReadResult(message, sizeof(message) - 1, net::OK,
                           GetParam().mode);
   source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode);
-  adapter()->Start();
+  adapter()->Start(callback());
 
   std::string output;
   EXPECT_EQ(ReadPipe(&output), net::OK);
@@ -170,7 +173,7 @@ TEST_P(SourceStreamToDataPipeTest, Error) {
   source()->AddReadResult(message, sizeof(message) - 1, net::OK,
                           GetParam().mode);
   source()->AddReadResult(nullptr, 0, net::ERR_FAILED, GetParam().mode);
-  adapter()->Start();
+  adapter()->Start(callback());
 
   std::string output;
   EXPECT_EQ(ReadPipe(&output), net::ERR_FAILED);
@@ -183,7 +186,7 @@ TEST_P(SourceStreamToDataPipeTest, ConsumerClosed) {
   Init();
   source()->AddReadResult(message, sizeof(message) - 1, net::OK,
                           GetParam().mode);
-  adapter()->Start();
+  adapter()->Start(callback());
 
   CloseConsumerHandle();
   CompleteReadsIfAsync();

content/browser/web_package/signed_exchange_loader.cc

@@ -300,9 +300,7 @@ void SignedExchangeLoader::OnHTTPExchangeFound(
   }
   pending_body_consumer_ = std::move(consumer_handle);
   body_data_pipe_adapter_ = std::make_unique<SourceStreamToDataPipe>(
-      std::move(payload_stream), std::move(producer_handle),
-      base::BindOnce(&SignedExchangeLoader::FinishReadingBody,
-                     base::Unretained(this)));
+      std::move(payload_stream), std::move(producer_handle));
 
   StartReadingBody();
 }
@@ -337,7 +335,8 @@ void SignedExchangeLoader::StartReadingBody() {
 
   // Start reading.
   client_->OnStartLoadingResponseBody(std::move(pending_body_consumer_));
-  body_data_pipe_adapter_->Start();
+  body_data_pipe_adapter_->Start(base::BindOnce(
+      &SignedExchangeLoader::FinishReadingBody, base::Unretained(this)));
 }
 
 void SignedExchangeLoader::FinishReadingBody(int result) {