BundledExchanges: Launch Data Decoder without Service Manager

This lets BundledExchangesReader use a Data Decoder service created
with content::LaunchDataDecoder(), instead of going through Service
Manager.

Bug: 977637
Change-Id: I0b96ef8ab29e7823400acf813ac91c650e254273
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875350Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710193}

content/browser/web_package/bundled_exchanges_handle.cc

@@ -19,6 +19,7 @@
 #include "content/browser/web_package/bundled_exchanges_utils.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/data_decoder_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -194,7 +195,8 @@ class InterceptorForFile final : public NavigationLoaderInterceptor {
         BundledExchangesSource::MaybeCreateFromFileUrl(request.url);
     if (!source)
       return false;
-    reader_ = base::MakeRefCounted<BundledExchangesReader>(std::move(source));
+    reader_ = base::MakeRefCounted<BundledExchangesReader>(std::move(source),
+                                                           LaunchDataDecoder());
     reader_->ReadMetadata(base::BindOnce(&InterceptorForFile::OnMetadataReady,
                                          weak_factory_.GetWeakPtr(), request));
     *client_request = forwarding_client_.BindNewPipeAndPassReceiver();
@@ -277,7 +279,9 @@ class InterceptorForTrustableFile final : public NavigationLoaderInterceptor {
                               DoneCallback done_callback,
                               int frame_tree_node_id)
       : source_(std::move(source)),
-        reader_(base::MakeRefCounted<BundledExchangesReader>(source_->Clone())),
+        reader_(
+            base::MakeRefCounted<BundledExchangesReader>(source_->Clone(),
+                                                         LaunchDataDecoder())),
         done_callback_(std::move(done_callback)),
         frame_tree_node_id_(frame_tree_node_id) {
     reader_->ReadMetadata(
@@ -549,7 +553,8 @@ class InterceptorForNavigationInfo final : public NavigationLoaderInterceptor {
       DoneCallback done_callback,
       int frame_tree_node_id)
       : reader_(base::MakeRefCounted<BundledExchangesReader>(
-            navigation_info->source().Clone())),
+            navigation_info->source().Clone(),
+            LaunchDataDecoder())),
         target_inner_url_(navigation_info->target_inner_url()),
         done_callback_(std::move(done_callback)),
         frame_tree_node_id_(frame_tree_node_id) {

content/browser/web_package/bundled_exchanges_reader.cc

@@ -13,13 +13,10 @@
 #include "content/browser/web_package/bundled_exchanges_source.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/common/service_manager_connection.h"
 #include "mojo/public/cpp/system/data_pipe_producer.h"
 #include "mojo/public/cpp/system/file_data_source.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 #include "net/base/url_util.h"
-#include "services/data_decoder/public/mojom/constants.mojom.h"
-#include "services/service_manager/public/cpp/connector.h"
 
 namespace content {
 
@@ -136,11 +133,10 @@ class BundledExchangesReader::SharedFileDataSource final
 };
 
 BundledExchangesReader::BundledExchangesReader(
-    std::unique_ptr<BundledExchangesSource> source)
+    std::unique_ptr<BundledExchangesSource> source,
+    mojo::Remote<data_decoder::mojom::DataDecoderService> service)
     : source_(std::move(source)),
-      parser_(ServiceManagerConnection::GetForProcess()
-                  ? ServiceManagerConnection::GetForProcess()->GetConnector()
-                  : nullptr),
+      parser_(std::move(service)),
       file_(base::MakeRefCounted<SharedFile>(source_->Clone())) {}
 
 BundledExchangesReader::~BundledExchangesReader() {

content/browser/web_package/bundled_exchanges_reader.h

@@ -36,8 +36,9 @@ class BundledExchangesSource;
 class CONTENT_EXPORT BundledExchangesReader final
     : public base::RefCounted<BundledExchangesReader> {
  public:
-  explicit BundledExchangesReader(
-      std::unique_ptr<BundledExchangesSource> source);
+  BundledExchangesReader(
+      std::unique_ptr<BundledExchangesSource> source,
+      mojo::Remote<data_decoder::mojom::DataDecoderService> service);
 
   // Starts parsing, and runs |callback| when meta data gets to be available.
   // |error| is set only on failures.

content/browser/web_package/mock_bundled_exchanges_reader_factory.cc

@@ -161,7 +161,8 @@ class MockBundledExchangesReaderFactoryImpl final
 
     auto reader = base::MakeRefCounted<BundledExchangesReader>(
         BundledExchangesSource::MaybeCreateFromTrustedFileUrl(
-            net::FilePathToFileURL(temp_file_path_)));
+            net::FilePathToFileURL(temp_file_path_)),
+        mojo::Remote<data_decoder::mojom::DataDecoderService>());
 
     std::unique_ptr<MockParserFactory> factory =
         std::make_unique<MockParserFactory>();

services/data_decoder/public/cpp/safe_bundled_exchanges_parser.cc

@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "services/data_decoder/public/mojom/constants.mojom.h"
-#include "services/service_manager/public/cpp/connector.h"
 
 namespace data_decoder {
 
@@ -16,10 +15,11 @@ constexpr char kConnectionError[] =
 }  // namespace
 
 SafeBundledExchangesParser::SafeBundledExchangesParser(
-    service_manager::Connector* connector) {
-  if (connector) {
-    connector->Connect(mojom::kServiceName,
-                       factory_.BindNewPipeAndPassReceiver());
+    mojo::Remote<data_decoder::mojom::DataDecoderService> service)
+    : service_(std::move(service)) {
+  if (service_.is_bound()) {
+    service_->BindBundledExchangesParserFactory(
+        factory_.BindNewPipeAndPassReceiver());
   }
 }
 

services/data_decoder/public/cpp/safe_bundled_exchanges_parser.h

@@ -14,10 +14,7 @@
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/data_decoder/public/mojom/bundled_exchanges_parser.mojom.h"
-
-namespace service_manager {
-class Connector;
-}  // namespace service_manager
+#include "services/data_decoder/public/mojom/data_decoder_service.mojom.h"
 
 namespace data_decoder {
 
@@ -25,9 +22,10 @@ namespace data_decoder {
 // mojom::BundledExchangesParser service.
 class SafeBundledExchangesParser {
  public:
-  // |connector| can be null if SetBundledExchangesParserForTesting() will be
+  // |service| can be unbound if SetBundledExchangesParserForTesting() will be
   // called before calling Open*().
-  explicit SafeBundledExchangesParser(service_manager::Connector* connector);
+  explicit SafeBundledExchangesParser(
+      mojo::Remote<data_decoder::mojom::DataDecoderService> service);
   // Remaining callbacks on flight will be dropped.
   ~SafeBundledExchangesParser();
 
@@ -64,6 +62,7 @@ class SafeBundledExchangesParser {
                         mojom::BundleResponsePtr response,
                         mojom::BundleResponseParseErrorPtr error);
 
+  mojo::Remote<data_decoder::mojom::DataDecoderService> service_;
   mojo::Remote<mojom::BundledExchangesParserFactory> factory_;
   mojo::Remote<mojom::BundledExchangesParser> parser_;
   mojom::BundledExchangesParser::ParseMetadataCallback metadata_callback_;

services/data_decoder/public/cpp/safe_bundled_exchanges_parser_unittest.cc

@@ -15,7 +15,7 @@
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
-#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
+#include "services/data_decoder/data_decoder_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace data_decoder {
@@ -109,7 +109,13 @@ class MockDataSource final : public mojom::BundleDataSource {
 
 class SafeBundledExchangesParserTest : public testing::Test {
  public:
-  service_manager::Connector* GetConnector() { return service_.connector(); }
+  mojo::Remote<mojom::DataDecoderService> GetService() {
+    DCHECK(!service_);
+    mojo::Remote<mojom::DataDecoderService> remote;
+    service_ = std::make_unique<DataDecoderService>(
+        remote.BindNewPipeAndPassReceiver());
+    return remote;
+  }
 
   MockFactory* InitializeMockFactory(SafeBundledExchangesParser* parser) {
     std::unique_ptr<MockFactory> factory = std::make_unique<MockFactory>();
@@ -123,11 +129,11 @@ class SafeBundledExchangesParserTest : public testing::Test {
 
  private:
   base::test::TaskEnvironment task_environment_;
-  TestDataDecoderService service_;
+  std::unique_ptr<DataDecoderService> service_;
 };
 
 TEST_F(SafeBundledExchangesParserTest, ParseGoldenFile) {
-  SafeBundledExchangesParser parser(GetConnector());
+  SafeBundledExchangesParser parser(GetService());
   base::File test_file =
       OpenTestFile(base::FilePath(FILE_PATH_LITERAL("hello.wbn")));
   ASSERT_EQ(base::File::FILE_OK, parser.OpenFile(std::move(test_file)));
@@ -185,12 +191,12 @@ TEST_F(SafeBundledExchangesParserTest, ParseGoldenFile) {
 }
 
 TEST_F(SafeBundledExchangesParserTest, OpenInvalidFile) {
-  SafeBundledExchangesParser parser(GetConnector());
+  SafeBundledExchangesParser parser(GetService());
   EXPECT_EQ(base::File::FILE_ERROR_FAILED, parser.OpenFile(base::File()));
 }
 
 TEST_F(SafeBundledExchangesParserTest, CallWithoutOpen) {
-  SafeBundledExchangesParser parser(GetConnector());
+  SafeBundledExchangesParser parser(GetService());
   bool metadata_parsed = false;
   parser.ParseMetadata(base::BindOnce(
       [](bool* metadata_parsed, mojom::BundleMetadataPtr metadata,
@@ -221,7 +227,7 @@ TEST_F(SafeBundledExchangesParserTest, CallWithoutOpen) {
 }
 
 TEST_F(SafeBundledExchangesParserTest, UseMockFactory) {
-  SafeBundledExchangesParser parser(GetConnector());
+  SafeBundledExchangesParser parser(GetService());
   MockFactory* raw_factory = InitializeMockFactory(&parser);
 
   EXPECT_FALSE(raw_factory->GetCreatedParser());
@@ -244,7 +250,7 @@ TEST_F(SafeBundledExchangesParserTest, UseMockFactory) {
 }
 
 TEST_F(SafeBundledExchangesParserTest, ConnectionError) {
-  SafeBundledExchangesParser parser(GetConnector());
+  SafeBundledExchangesParser parser(GetService());
   MockFactory* raw_factory = InitializeMockFactory(&parser);
 
   mojo::PendingRemote<mojom::BundleDataSource> remote_data_source;