ash: Launch Data Decoder without Service Manager

Ports ash usage of the Data Decoder service to use the service interface
directly rather than going through a Service Manager Connector.

Bug: 977637
Change-Id: I3cf24b34ab0eeff6665055b5add7fb1d90c591bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872918Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#708678}

ash/BUILD.gn

@@ -1352,6 +1352,7 @@ component("ash") {
     "//components/discardable_memory/public/mojom",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/system",
+    "//services/data_decoder/public/mojom",
     "//services/device/public/cpp/bluetooth",
     "//services/device/public/mojom",
     "//services/media_session/public/cpp",

ash/DEPS

@@ -27,6 +27,7 @@ include_rules = [
   "+media",
   "+mojo/public",
   "+services/content/public",
+  "+services/data_decoder/public",
   "+services/media_session/public",
   "+services/preferences/public",
   "+services/service_manager/public",

ash/login/ui/image_parser.cc

@@ -7,8 +7,10 @@
 #include <utility>
 
 #include "ash/shell.h"
+#include "ash/shell_delegate.h"
 #include "base/bind.h"
 #include "ipc/ipc_channel.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "services/data_decoder/public/cpp/decode_image.h"
 
 namespace ash {
@@ -40,9 +42,9 @@ void DecodeAnimation(const std::vector<uint8_t>& image_data,
   // also goes through libpng, but APNG support is handled specifically by
   // blink's PNGImageReader.cpp.
   data_decoder::DecodeAnimation(
-      Shell::Get()->connector(), image_data, true /*shrink_to_fit*/,
-      kMaxImageSizeInBytes,
-      base::Bind(&ConvertToAnimationFrame, Passed(&on_decoded)));
+      Shell::Get()->shell_delegate()->LaunchDataDecoder(), image_data,
+      true /*shrink_to_fit*/, kMaxImageSizeInBytes,
+      base::BindOnce(&ConvertToAnimationFrame, std::move(on_decoded)));
 }
 
 }  // namespace ash

ash/public/cpp/manifest.cc

@@ -13,7 +13,6 @@
 #include "base/no_destructor.h"
 #include "chromeos/services/multidevice_setup/public/mojom/constants.mojom.h"
 #include "services/content/public/mojom/constants.mojom.h"
-#include "services/data_decoder/public/mojom/constants.mojom.h"
 #include "services/device/public/mojom/constants.mojom.h"
 #include "services/preferences/public/mojom/preferences.mojom.h"
 #include "services/service_manager/public/cpp/manifest_builder.h"
@@ -43,7 +42,6 @@ const service_manager::Manifest& GetManifest() {
           .RequireCapability("*", "accessibility")
           .RequireCapability("*", "app")
           .RequireCapability(content::mojom::kServiceName, "navigation")
-          .RequireCapability(data_decoder::mojom::kServiceName, "image_decoder")
           .RequireCapability(device::mojom::kServiceName,
                              "device:bluetooth_system")
           .RequireCapability(device::mojom::kServiceName, "device:fingerprint")

ash/shell/shell_delegate_impl.cc

@@ -33,5 +33,12 @@ bool ShellDelegateImpl::CanGoBack(gfx::NativeWindow window) const {
   return true;
 }
 
+mojo::Remote<data_decoder::mojom::DataDecoderService>
+ShellDelegateImpl::LaunchDataDecoder() {
+  mojo::Remote<data_decoder::mojom::DataDecoderService> remote;
+  ignore_result(remote.BindNewPipeAndPassReceiver());
+  return remote;
+}
+
 }  // namespace shell
 }  // namespace ash

ash/shell/shell_delegate_impl.h

@@ -24,6 +24,8 @@ class ShellDelegateImpl : public ShellDelegate {
   std::unique_ptr<ScreenshotDelegate> CreateScreenshotDelegate() override;
   AccessibilityDelegate* CreateAccessibilityDelegate() override;
   bool CanGoBack(gfx::NativeWindow window) const override;
+  mojo::Remote<data_decoder::mojom::DataDecoderService> LaunchDataDecoder()
+      override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ShellDelegateImpl);

ash/shell_delegate.h

@@ -11,6 +11,8 @@
 #include "ash/ash_export.h"
 #include "base/callback.h"
 #include "base/strings/string16.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "services/data_decoder/public/mojom/data_decoder_service.mojom.h"
 #include "ui/gfx/native_widget_types.h"
 
 namespace aura {
@@ -42,6 +44,12 @@ class ASH_EXPORT ShellDelegate {
   virtual bool CanGoBack(gfx::NativeWindow window) const = 0;
 
   virtual void OpenKeyboardShortcutHelpPage() const {}
+
+  // Launches an instance of the Data Decoder service and returns a remote
+  // endpoint to communicate with it. In testing environments, this may return
+  // a disconnected Remote.
+  virtual mojo::Remote<data_decoder::mojom::DataDecoderService>
+  LaunchDataDecoder() = 0;
 };
 
 }  // namespace ash

ash/test_shell_delegate.cc

@@ -33,4 +33,11 @@ bool TestShellDelegate::CanGoBack(gfx::NativeWindow window) const {
   return true;
 }
 
+mojo::Remote<data_decoder::mojom::DataDecoderService>
+TestShellDelegate::LaunchDataDecoder() {
+  mojo::Remote<data_decoder::mojom::DataDecoderService> remote;
+  ignore_result(remote.BindNewPipeAndPassReceiver());
+  return remote;
+}
+
 }  // namespace ash

ash/test_shell_delegate.h

@@ -22,6 +22,8 @@ class TestShellDelegate : public ShellDelegate {
   std::unique_ptr<ScreenshotDelegate> CreateScreenshotDelegate() override;
   AccessibilityDelegate* CreateAccessibilityDelegate() override;
   bool CanGoBack(gfx::NativeWindow window) const override;
+  mojo::Remote<data_decoder::mojom::DataDecoderService> LaunchDataDecoder()
+      override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestShellDelegate);

ash/wallpaper/wallpaper_utils/wallpaper_decoder.cc

@@ -5,6 +5,7 @@
 #include "ash/wallpaper/wallpaper_utils/wallpaper_decoder.h"
 
 #include "ash/shell.h"
+#include "ash/shell_delegate.h"
 #include "base/bind.h"
 #include "ipc/ipc_channel.h"
 
@@ -32,16 +33,11 @@ void ConvertToImageSkia(OnWallpaperDecoded callback, const SkBitmap& image) {
 void DecodeWallpaper(const std::string& image_data,
                      const data_decoder::mojom::ImageCodec& image_codec,
                      OnWallpaperDecoded callback) {
-  // The connector for the mojo service manager is null in unit tests.
-  if (!Shell::Get()->connector()) {
-    std::move(callback).Run(gfx::ImageSkia());
-    return;
-  }
   std::vector<uint8_t> image_bytes(image_data.begin(), image_data.end());
   data_decoder::DecodeImage(
-      Shell::Get()->connector(), std::move(image_bytes), image_codec,
-      /*shrink_to_fit=*/true, kMaxImageSizeInBytes,
-      /*desired_image_frame_size=*/gfx::Size(),
+      Shell::Get()->shell_delegate()->LaunchDataDecoder(),
+      std::move(image_bytes), image_codec, /*shrink_to_fit=*/true,
+      kMaxImageSizeInBytes, /*desired_image_frame_size=*/gfx::Size(),
       base::BindOnce(&ConvertToImageSkia, std::move(callback)));
 }
 

chrome/browser/ui/ash/chrome_shell_delegate.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "content/public/browser/data_decoder_service.h"
 #include "ui/aura/window.h"
 #include "url/gurl.h"
 
@@ -74,3 +75,8 @@ std::unique_ptr<ash::ScreenshotDelegate>
 ChromeShellDelegate::CreateScreenshotDelegate() {
   return std::make_unique<ChromeScreenshotGrabber>();
 }
+
+mojo::Remote<data_decoder::mojom::DataDecoderService>
+ChromeShellDelegate::LaunchDataDecoder() {
+  return content::LaunchDataDecoder();
+}

chrome/browser/ui/ash/chrome_shell_delegate.h

@@ -19,6 +19,8 @@ class ChromeShellDelegate : public ash::ShellDelegate {
   ash::AccessibilityDelegate* CreateAccessibilityDelegate() override;
   void OpenKeyboardShortcutHelpPage() const override;
   bool CanGoBack(gfx::NativeWindow window) const override;
+  mojo::Remote<data_decoder::mojom::DataDecoderService> LaunchDataDecoder()
+      override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ChromeShellDelegate);

services/data_decoder/public/cpp/decode_image.cc

@@ -20,13 +20,15 @@ namespace {
 // Helper callback which owns a mojo::Remote<ImageDecoder> until invoked. This
 // keeps the ImageDecoder pipe open just long enough to dispatch a reply, at
 // which point the reply is forwarded to the wrapped |callback|.
-void OnDecodeImage(mojo::Remote<mojom::ImageDecoder> decoder,
+void OnDecodeImage(mojo::Remote<mojom::DataDecoderService> service,
+                   mojo::Remote<mojom::ImageDecoder> decoder,
                    mojom::ImageDecoder::DecodeImageCallback callback,
                    const SkBitmap& bitmap) {
   std::move(callback).Run(bitmap);
 }
 
-void OnDecodeImages(mojo::Remote<mojom::ImageDecoder> decoder,
+void OnDecodeImages(mojo::Remote<mojom::DataDecoderService> service,
+                    mojo::Remote<mojom::ImageDecoder> decoder,
                     mojom::ImageDecoder::DecodeAnimationCallback callback,
                     std::vector<mojom::AnimationFramePtr> bitmaps) {
   std::move(callback).Run(std::move(bitmaps));
@@ -41,37 +43,95 @@ void DecodeImage(service_manager::Connector* connector,
                  uint64_t max_size_in_bytes,
                  const gfx::Size& desired_image_frame_size,
                  mojom::ImageDecoder::DecodeImageCallback callback) {
+  mojo::PendingRemote<mojom::ImageDecoder> pending_decoder;
+  connector->Connect(mojom::kServiceName,
+                     pending_decoder.InitWithNewPipeAndPassReceiver());
+  return DecodeImage(std::move(pending_decoder), encoded_bytes, codec,
+                     shrink_to_fit, max_size_in_bytes, desired_image_frame_size,
+                     std::move(callback));
+}
+
+void DecodeImage(mojo::PendingRemote<mojom::ImageDecoder> pending_decoder,
+                 const std::vector<uint8_t>& encoded_bytes,
+                 mojom::ImageCodec codec,
+                 bool shrink_to_fit,
+                 uint64_t max_size_in_bytes,
+                 const gfx::Size& desired_image_frame_size,
+                 mojom::ImageDecoder::DecodeImageCallback callback) {
+  mojo::Remote<mojom::DataDecoderService> null_service;
+  mojo::Remote<mojom::ImageDecoder> decoder(std::move(pending_decoder));
+
+  // |call_once| runs |callback| on its first invocation.
+  auto call_once = base::AdaptCallbackForRepeating(std::move(callback));
+  decoder.set_disconnect_handler(base::BindOnce(call_once, SkBitmap()));
+
+  mojom::ImageDecoder* raw_decoder = decoder.get();
+  raw_decoder->DecodeImage(
+      encoded_bytes, codec, shrink_to_fit, max_size_in_bytes,
+      desired_image_frame_size,
+      base::BindOnce(&OnDecodeImage, std::move(null_service),
+                     std::move(decoder), std::move(call_once)));
+}
+
+void DecodeImage(mojo::Remote<mojom::DataDecoderService> service,
+                 const std::vector<uint8_t>& encoded_bytes,
+                 mojom::ImageCodec codec,
+                 bool shrink_to_fit,
+                 uint64_t max_size_in_bytes,
+                 const gfx::Size& desired_image_frame_size,
+                 mojom::ImageDecoder::DecodeImageCallback callback) {
   mojo::Remote<mojom::ImageDecoder> decoder;
-  connector->Connect(mojom::kServiceName, decoder.BindNewPipeAndPassReceiver());
+  service->BindImageDecoder(decoder.BindNewPipeAndPassReceiver());
 
   // |call_once| runs |callback| on its first invocation.
   auto call_once = base::AdaptCallbackForRepeating(std::move(callback));
-  decoder.set_disconnect_handler(base::Bind(call_once, SkBitmap()));
+  decoder.set_disconnect_handler(base::BindOnce(call_once, SkBitmap()));
 
   mojom::ImageDecoder* raw_decoder = decoder.get();
   raw_decoder->DecodeImage(
       encoded_bytes, codec, shrink_to_fit, max_size_in_bytes,
       desired_image_frame_size,
-      base::BindOnce(&OnDecodeImage, std::move(decoder), std::move(call_once)));
+      base::BindOnce(&OnDecodeImage, std::move(service), std::move(decoder),
+                     std::move(call_once)));
+}
+
+void DecodeAnimation(mojo::PendingRemote<mojom::ImageDecoder> pending_decoder,
+                     const std::vector<uint8_t>& encoded_bytes,
+                     bool shrink_to_fit,
+                     uint64_t max_size_in_bytes,
+                     mojom::ImageDecoder::DecodeAnimationCallback callback) {
+  mojo::Remote<mojom::DataDecoderService> null_service;
+  mojo::Remote<mojom::ImageDecoder> decoder(std::move(pending_decoder));
+
+  // |call_once| runs |callback| on its first invocation.
+  auto call_once = base::AdaptCallbackForRepeating(std::move(callback));
+  decoder.set_disconnect_handler(base::BindOnce(
+      call_once, base::Passed(std::vector<mojom::AnimationFramePtr>())));
+
+  mojom::ImageDecoder* raw_decoder = decoder.get();
+  raw_decoder->DecodeAnimation(
+      encoded_bytes, shrink_to_fit, max_size_in_bytes,
+      base::BindOnce(&OnDecodeImages, std::move(null_service),
+                     std::move(decoder), std::move(call_once)));
 }
 
-void DecodeAnimation(service_manager::Connector* connector,
+void DecodeAnimation(mojo::Remote<mojom::DataDecoderService> service,
                      const std::vector<uint8_t>& encoded_bytes,
                      bool shrink_to_fit,
                      uint64_t max_size_in_bytes,
                      mojom::ImageDecoder::DecodeAnimationCallback callback) {
   mojo::Remote<mojom::ImageDecoder> decoder;
-  connector->Connect(mojom::kServiceName, decoder.BindNewPipeAndPassReceiver());
+  service->BindImageDecoder(decoder.BindNewPipeAndPassReceiver());
 
   // |call_once| runs |callback| on its first invocation.
   auto call_once = base::AdaptCallbackForRepeating(std::move(callback));
-  decoder.set_disconnect_handler(base::Bind(
+  decoder.set_disconnect_handler(base::BindOnce(
       call_once, base::Passed(std::vector<mojom::AnimationFramePtr>())));
 
   mojom::ImageDecoder* raw_decoder = decoder.get();
   raw_decoder->DecodeAnimation(
       encoded_bytes, shrink_to_fit, max_size_in_bytes,
-      base::BindOnce(&OnDecodeImages, std::move(decoder),
+      base::BindOnce(&OnDecodeImages, std::move(service), std::move(decoder),
                      std::move(call_once)));
 }
 

services/data_decoder/public/cpp/decode_image.h

@@ -9,6 +9,9 @@
 
 #include <vector>
 
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
+#include "services/data_decoder/public/mojom/data_decoder_service.mojom.h"
 #include "services/data_decoder/public/mojom/image_decoder.mojom.h"
 
 namespace gfx {
@@ -40,10 +43,38 @@ void DecodeImage(service_manager::Connector* connector,
                  const gfx::Size& desired_image_frame_size,
                  mojom::ImageDecoder::DecodeImageCallback callback);
 
+// Same as above but uses an ImageDecoder interface directly instead of going
+// through the Service Manager.
+void DecodeImage(mojo::PendingRemote<mojom::ImageDecoder> pending_decoder,
+                 const std::vector<uint8_t>& encoded_bytes,
+                 mojom::ImageCodec codec,
+                 bool shrink_to_fit,
+                 uint64_t max_size_in_bytes,
+                 const gfx::Size& desired_image_frame_size,
+                 mojom::ImageDecoder::DecodeImageCallback callback);
+
+// Same as above but uses a dedicated DataDecoderService instance for the
+// operation.
+void DecodeImage(mojo::Remote<mojom::DataDecoderService> service,
+                 const std::vector<uint8_t>& encoded_bytes,
+                 mojom::ImageCodec codec,
+                 bool shrink_to_fit,
+                 uint64_t max_size_in_bytes,
+                 const gfx::Size& desired_image_frame_size,
+                 mojom::ImageDecoder::DecodeImageCallback callback);
+
 // Helper function to decode an animation via the data_decoder service. Any
 // image with multiple frames is considered an animation, so long as the frames
 // are all the same size.
-void DecodeAnimation(service_manager::Connector* connector,
+void DecodeAnimation(mojo::PendingRemote<mojom::ImageDecoder> pending_decoder,
+                     const std::vector<uint8_t>& encoded_bytes,
+                     bool shrink_to_fit,
+                     uint64_t max_size_in_bytes,
+                     mojom::ImageDecoder::DecodeAnimationCallback callback);
+
+// Same as above but uses a dedicated DataDecoderService instance for the
+// operation.
+void DecodeAnimation(mojo::Remote<mojom::DataDecoderService> service,
                      const std::vector<uint8_t>& encoded_bytes,
                      bool shrink_to_fit,
                      uint64_t max_size_in_bytes,