Media gallery: Decouple media gallery from extension blob reader.

Currently media gallery api reads media data from a blob in browser
process with extension's blob reader.

This CL abstracts a factory class to create MediaDataSource, so that
Android can implement a different source that read data from local file.

Eventually, media gallery api will not depend on extension, which is
not supported on Android, and can have different data source.

Bug: 826021,826975
Change-Id: I6d9d6c0ffc8cbae42cd036af34fb4944cf25e685
Reviewed-on: https://chromium-review.googlesource.com/997012
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550653}

chrome/browser/extensions/BUILD.gn

@@ -247,6 +247,8 @@ static_library("extensions") {
     "api/language_settings_private/language_settings_private_delegate_factory.h",
     "api/management/chrome_management_api_delegate.cc",
     "api/management/chrome_management_api_delegate.h",
+    "api/media_galleries/blob_data_source_factory.cc",
+    "api/media_galleries/blob_data_source_factory.h",
     "api/media_galleries/media_galleries_api.cc",
     "api/media_galleries/media_galleries_api.h",
     "api/messaging/chrome_messaging_delegate.cc",

chrome/browser/extensions/api/media_galleries/blob_data_source_factory.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/api/media_galleries/blob_data_source_factory.h"
+#include "content/public/browser/browser_context.h"
+#include "extensions/browser/blob_reader.h"
+#include "mojo/public/cpp/bindings/binding.h"
+
+namespace extensions {
+namespace {
+
+// Media data source that reads data from a blob in browser process.
+class BlobMediaDataSource : public chrome::mojom::MediaDataSource {
+ public:
+  BlobMediaDataSource(chrome::mojom::MediaDataSourcePtr* interface,
+                      content::BrowserContext* browser_context,
+                      const std::string& blob_uuid,
+                      BlobDataSourceFactory::MediaDataCallback callback)
+      : binding_(this, mojo::MakeRequest(interface)),
+        browser_context_(browser_context),
+        blob_uuid_(blob_uuid),
+        callback_(callback),
+        weak_factory_(this) {}
+
+  ~BlobMediaDataSource() override = default;
+
+ private:
+  // chrome::mojom::MediaDataSource implementation.
+  void Read(int64_t position,
+            int64_t length,
+            chrome::mojom::MediaDataSource::ReadCallback callback) override {
+    StartBlobRequest(std::move(callback), position, length);
+  }
+
+  void StartBlobRequest(chrome::mojom::MediaDataSource::ReadCallback callback,
+                        int64_t position,
+                        int64_t length) {
+    BlobReader* reader = new BlobReader(  // BlobReader is self-deleting.
+        browser_context_, blob_uuid_,
+        base::BindRepeating(&BlobMediaDataSource::OnBlobReaderDone,
+                            weak_factory_.GetWeakPtr(),
+                            base::Passed(&callback)));
+    reader->SetByteRange(position, length);
+    reader->Start();
+  }
+
+  void OnBlobReaderDone(chrome::mojom::MediaDataSource::ReadCallback callback,
+                        std::unique_ptr<std::string> data,
+                        int64_t size) {
+    callback_.Run(std::move(callback), std::move(data));
+  }
+
+  mojo::Binding<chrome::mojom::MediaDataSource> binding_;
+
+  content::BrowserContext* const browser_context_;
+  std::string blob_uuid_;
+
+  BlobDataSourceFactory::MediaDataCallback callback_;
+
+  base::WeakPtrFactory<BlobMediaDataSource> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlobMediaDataSource);
+};
+
+}  // namespace
+
+BlobDataSourceFactory::BlobDataSourceFactory(
+    content::BrowserContext* browser_context,
+    const std::string& blob_uuid)
+    : browser_context_(browser_context), blob_uuid_(blob_uuid) {}
+
+BlobDataSourceFactory::~BlobDataSourceFactory() = default;
+
+std::unique_ptr<chrome::mojom::MediaDataSource>
+BlobDataSourceFactory::CreateMediaDataSource(
+    chrome::mojom::MediaDataSourcePtr* request,
+    MediaDataCallback media_data_callback) {
+  return std::make_unique<BlobMediaDataSource>(request, browser_context_,
+                                               blob_uuid_, media_data_callback);
+}
+
+}  // namespace extensions

chrome/browser/extensions/api/media_galleries/blob_data_source_factory.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_API_MEDIA_GALLERIES_BLOB_DATA_SOURCE_FACTORY_H_
+#define CHROME_BROWSER_EXTENSIONS_API_MEDIA_GALLERIES_BLOB_DATA_SOURCE_FACTORY_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "chrome/services/media_gallery_util/public/cpp/safe_media_metadata_parser.h"
+#include "chrome/services/media_gallery_util/public/mojom/media_parser.mojom.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace extensions {
+
+// Factory to provide media data source for extension media gallery API.
+// Internally it will read media data from a blob in browser process.
+class BlobDataSourceFactory
+    : public SafeMediaMetadataParser::MediaDataSourceFactory {
+ public:
+  BlobDataSourceFactory(content::BrowserContext* browser_context,
+                        const std::string& blob_uuid);
+  ~BlobDataSourceFactory() override;
+
+ private:
+  // SafeMediaMetadataParser::MediaDataSourceFactory implementation.
+  std::unique_ptr<chrome::mojom::MediaDataSource> CreateMediaDataSource(
+      chrome::mojom::MediaDataSourcePtr* request,
+      MediaDataCallback media_data_callback) override;
+
+  content::BrowserContext* browser_context_;
+  std::string blob_uuid_;
+  MediaDataCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlobDataSourceFactory);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_API_MEDIA_GALLERIES_BLOB_DATA_SOURCE_FACTORY_H_

chrome/browser/extensions/api/media_galleries/media_galleries_api.cc

@@ -24,6 +24,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/api/media_galleries/blob_data_source_factory.h"
 #include "chrome/browser/extensions/chrome_extension_function_details.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/media_galleries/gallery_watch_manager.h"
@@ -674,9 +675,11 @@ void MediaGalleriesGetMetadataFunction::GetMetadata(
       metadata_type == MediaGalleries::GET_METADATA_TYPE_ALL ||
       metadata_type == MediaGalleries::GET_METADATA_TYPE_NONE;
 
+  auto media_data_source_factory =
+      std::make_unique<BlobDataSourceFactory>(GetProfile(), blob_uuid);
   auto parser = std::make_unique<SafeMediaMetadataParser>(
-      GetProfile(), blob_uuid, total_blob_length, mime_type,
-      get_attached_images);
+      total_blob_length, mime_type, get_attached_images,
+      std::move(media_data_source_factory));
   SafeMediaMetadataParser* parser_ptr = parser.get();
   parser_ptr->Start(
       content::ServiceManagerConnection::GetForProcess()->GetConnector(),

chrome/services/media_gallery_util/ipc_data_source.cc

@@ -36,7 +36,7 @@ void IPCDataSource::Read(int64_t position,
 
   utility_task_runner_->PostTask(
       FROM_HERE,
-      base::BindOnce(&IPCDataSource::ReadBlob, base::Unretained(this),
+      base::BindOnce(&IPCDataSource::ReadMediaData, base::Unretained(this),
                      destination, callback, position, size));
 }
 
@@ -55,10 +55,10 @@ void IPCDataSource::SetBitrate(int bitrate) {
   DCHECK_CALLED_ON_VALID_THREAD(data_source_thread_checker_);
 }
 
-void IPCDataSource::ReadBlob(uint8_t* destination,
-                             const DataSource::ReadCB& callback,
-                             int64_t position,
-                             int size) {
+void IPCDataSource::ReadMediaData(uint8_t* destination,
+                                  const DataSource::ReadCB& callback,
+                                  int64_t position,
+                                  int size) {
   DCHECK_CALLED_ON_VALID_THREAD(utility_thread_checker_);
   CHECK_GE(total_size_, 0);
   CHECK_GE(position, 0);
@@ -69,7 +69,7 @@ void IPCDataSource::ReadBlob(uint8_t* destination,
   int64_t clamped_size =
       std::min(static_cast<int64_t>(size), total_size_ - position);
 
-  media_data_source_->ReadBlob(
+  media_data_source_->Read(
       position, clamped_size,
       base::BindOnce(&IPCDataSource::ReadDone, base::Unretained(this),
                      destination, callback));

chrome/services/media_gallery_util/ipc_data_source.h

@@ -42,11 +42,11 @@ class IPCDataSource : public media::DataSource {
   void SetBitrate(int bitrate) override;
 
  private:
-  // Blob data read helpers: must be run on the utility thread.
-  void ReadBlob(uint8_t* destination,
-                const ReadCB& callback,
-                int64_t position,
-                int size);
+  // Media data read helpers: must be run on the utility thread.
+  void ReadMediaData(uint8_t* destination,
+                     const ReadCB& callback,
+                     int64_t position,
+                     int size);
   void ReadDone(uint8_t* destination,
                 const ReadCB& callback,
                 const std::vector<uint8_t>& data);

chrome/services/media_gallery_util/public/cpp/safe_media_metadata_parser.cc

@@ -8,47 +8,17 @@
 
 #include "base/callback.h"
 #include "chrome/services/media_gallery_util/public/mojom/constants.mojom.h"
-#include "content/public/browser/browser_context.h"
-#include "extensions/browser/blob_reader.h"
-#include "mojo/public/cpp/bindings/binding.h"
 #include "services/service_manager/public/cpp/connector.h"
 
-class SafeMediaMetadataParser::MediaDataSourceImpl
-    : public chrome::mojom::MediaDataSource {
- public:
-  MediaDataSourceImpl(SafeMediaMetadataParser* owner,
-                      chrome::mojom::MediaDataSourcePtr* interface)
-      : binding_(this, mojo::MakeRequest(interface)),
-        safe_media_metadata_parser_(owner) {}
-
-  ~MediaDataSourceImpl() override = default;
-
- private:
-  void ReadBlob(int64_t position,
-                int64_t length,
-                ReadBlobCallback callback) override {
-    safe_media_metadata_parser_->StartBlobRequest(std::move(callback), position,
-                                                  length);
-  }
-
-  mojo::Binding<chrome::mojom::MediaDataSource> binding_;
-  // |safe_media_metadata_parser_| owns |this|.
-  SafeMediaMetadataParser* const safe_media_metadata_parser_;
-
-  DISALLOW_COPY_AND_ASSIGN(MediaDataSourceImpl);
-};
-
 SafeMediaMetadataParser::SafeMediaMetadataParser(
-    content::BrowserContext* browser_context,
-    const std::string& blob_uuid,
-    int64_t blob_size,
+    int64_t size,
     const std::string& mime_type,
-    bool get_attached_images)
-    : browser_context_(browser_context),
-      blob_uuid_(blob_uuid),
-      blob_size_(blob_size),
+    bool get_attached_images,
+    std::unique_ptr<MediaDataSourceFactory> media_source_factory)
+    : size_(size),
       mime_type_(mime_type),
       get_attached_images_(get_attached_images),
+      media_source_factory_(std::move(media_source_factory)),
       weak_factory_(this) {}
 
 SafeMediaMetadataParser::~SafeMediaMetadataParser() = default;
@@ -65,9 +35,11 @@ void SafeMediaMetadataParser::Start(service_manager::Connector* connector,
 
 void SafeMediaMetadataParser::OnMediaParserCreated() {
   chrome::mojom::MediaDataSourcePtr source;
-  media_data_source_ = std::make_unique<MediaDataSourceImpl>(this, &source);
+  media_data_source_ = media_source_factory_->CreateMediaDataSource(
+      &source, base::BindRepeating(&SafeMediaMetadataParser::OnMediaDataReady,
+                                   weak_factory_.GetWeakPtr()));
   media_parser()->ParseMediaMetadata(
-      mime_type_, blob_size_, get_attached_images_, std::move(source),
+      mime_type_, size_, get_attached_images_, std::move(source),
       base::BindOnce(&SafeMediaMetadataParser::ParseMediaMetadataDone,
                      base::Unretained(this)));
 }
@@ -96,22 +68,9 @@ void SafeMediaMetadataParser::ParseMediaMetadataDone(
                            std::move(attached_images_copy));
 }
 
-void SafeMediaMetadataParser::StartBlobRequest(
-    chrome::mojom::MediaDataSource::ReadBlobCallback callback,
-    int64_t position,
-    int64_t length) {
-  BlobReader* reader = new BlobReader(  // BlobReader is self-deleting.
-      browser_context_, blob_uuid_,
-      base::Bind(&SafeMediaMetadataParser::BlobReaderDone,
-                 weak_factory_.GetWeakPtr(), base::Passed(&callback)));
-  reader->SetByteRange(position, length);
-  reader->Start();
-}
-
-void SafeMediaMetadataParser::BlobReaderDone(
-    chrome::mojom::MediaDataSource::ReadBlobCallback callback,
-    std::unique_ptr<std::string> data,
-    int64_t /* blob_total_size */) {
+void SafeMediaMetadataParser::OnMediaDataReady(
+    chrome::mojom::MediaDataSource::ReadCallback callback,
+    std::unique_ptr<std::string> data) {
   if (media_parser())
     std::move(callback).Run(std::vector<uint8_t>(data->begin(), data->end()));
 }

chrome/services/media_gallery_util/public/cpp/safe_media_metadata_parser.h

@@ -18,18 +18,14 @@
 #include "chrome/services/media_gallery_util/public/cpp/media_parser_provider.h"
 #include "chrome/services/media_gallery_util/public/mojom/media_parser.mojom.h"
 
-namespace content {
-class BrowserContext;
-}
-
 namespace service_manager {
 class Connector;
 }
 
-// Parses the media metadata of a Blob safely in a utility process. This class
-// expects the MIME type of the Blob to be already known. It creates a utility
-// process to do further MIME-type-specific metadata extraction from the Blob
-// data.
+// Parses the media metadata safely in a utility process. This class expects the
+// MIME type and the size of media data to be already known. It creates a
+// utility process to do further MIME-type-specific metadata extraction from the
+// media data.
 class SafeMediaMetadataParser : public MediaParserProvider {
  public:
   typedef base::OnceCallback<void(
@@ -38,11 +34,26 @@ class SafeMediaMetadataParser : public MediaParserProvider {
       std::unique_ptr<std::vector<metadata::AttachedImage>> attached_images)>
       DoneCallback;
 
-  SafeMediaMetadataParser(content::BrowserContext* browser_context,
-                          const std::string& blob_uuid,
-                          int64_t blob_size,
-                          const std::string& mime_type,
-                          bool get_attached_images);
+  // Factory to create media data source instance. The underlying implementation
+  // may read from different kinds of storage.
+  class MediaDataSourceFactory {
+   public:
+    typedef base::RepeatingCallback<void(
+        chrome::mojom::MediaDataSource::ReadCallback callback,
+        std::unique_ptr<std::string> data)>
+        MediaDataCallback;
+
+    virtual std::unique_ptr<chrome::mojom::MediaDataSource>
+    CreateMediaDataSource(chrome::mojom::MediaDataSourcePtr* request,
+                          MediaDataCallback media_data_callback) = 0;
+    virtual ~MediaDataSourceFactory() {}
+  };
+
+  SafeMediaMetadataParser(
+      int64_t size,
+      const std::string& mime_type,
+      bool get_attached_images,
+      std::unique_ptr<MediaDataSourceFactory> media_source_factory);
   ~SafeMediaMetadataParser() override;
 
   // Should be called on the thread |connector| is associated with. |callback|
@@ -50,8 +61,6 @@ class SafeMediaMetadataParser : public MediaParserProvider {
   void Start(service_manager::Connector* connector, DoneCallback callback);
 
  private:
-  class MediaDataSourceImpl;
-
   // MediaParserProvider implementation:
   void OnMediaParserCreated() override;
   void OnConnectionError() override;
@@ -62,27 +71,19 @@ class SafeMediaMetadataParser : public MediaParserProvider {
       std::unique_ptr<base::DictionaryValue> metadata_dictionary,
       const std::vector<metadata::AttachedImage>& attached_images);
 
-  // Starts to read the blob data and sends the data back to the utility
-  // process.
-  void StartBlobRequest(
-      chrome::mojom::MediaDataSource::ReadBlobCallback callback,
-      int64_t position,
-      int64_t length);
-
-  // Invoked when the full blob content has been read.
-  void BlobReaderDone(chrome::mojom::MediaDataSource::ReadBlobCallback callback,
-                      std::unique_ptr<std::string> data,
-                      int64_t /* blob_total_size */);
-
-  content::BrowserContext* const browser_context_;
-  const std::string blob_uuid_;
-  const int64_t blob_size_;
+  // Invoked when the media data has been read, which will be sent back to
+  // utility process soon. |data| might be partial content of the media data.
+  void OnMediaDataReady(chrome::mojom::MediaDataSource::ReadCallback callback,
+                        std::unique_ptr<std::string> data);
+
+  const int64_t size_;
   const std::string mime_type_;
   bool get_attached_images_;
 
   DoneCallback callback_;
 
-  std::unique_ptr<MediaDataSourceImpl> media_data_source_;
+  std::unique_ptr<chrome::mojom::MediaDataSource> media_data_source_;
+  std::unique_ptr<MediaDataSourceFactory> media_source_factory_;
 
   base::WeakPtrFactory<SafeMediaMetadataParser> weak_factory_;
 

chrome/services/media_gallery_util/public/mojom/media_parser.mojom

@@ -9,7 +9,7 @@ import "mojo/public/mojom/base/file.mojom";
 import "mojo/public/mojom/base/time.mojom";
 
 interface MediaParser {
-  // Extracts metadata from a |mime_type| blob of data of |total_size| and
+  // Extracts metadata from media data with |mime_type|, |total_size| and
   // available from |media_data_source|. If there are images referred to in the
   // metadata, and |get_attached_images| is true, return the images in
   // |attached_images|.
@@ -47,9 +47,9 @@ interface MediaParserFactory {
 };
 
 interface MediaDataSource {
-  // ParseMediaMetadata interface used to read blob data for parsing from the
+  // ParseMediaMetadata interface used to read data for parsing from the
   // calling process.
-  ReadBlob(int64 position, int64 length) => (array<uint8> data);
+  Read(int64 position, int64 length) => (array<uint8> data);
 };
 
 struct AttachedImage {