Skip to content

Commit

Permalink
Servifying MediaParser.mojom
Browse files Browse the repository at this point in the history
As part of the effort to deprecate UtilityProcessMojoClient,
servicifying MediaParser.mojom and making SafeAudioVideoChecker and
SafeMediaMetaDataParser client libraries.

Bug: 782928
Change-Id: I6357eadd9a1bc258b57d88b05d4aad21538cd6a2
Reviewed-on: https://chromium-review.googlesource.com/758750
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515614}
  • Loading branch information
jcivelli-google authored and Commit Bot committed Nov 10, 2017
1 parent e55c9ef commit 069422f
Show file tree
Hide file tree
Showing 46 changed files with 644 additions and 327 deletions.
5 changes: 5 additions & 0 deletions chrome/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,11 @@ if (enable_basic_printing || enable_print_preview) {
[ "//components/printing/service:pdf_compositor_manifest" ]
}

if (enable_extensions) {
chrome_packaged_services +=
[ "//chrome/services/media_gallery_util:manifest" ]
}

if (is_chromeos) {
chrome_packaged_services +=
[ "//chrome/browser/chromeos:ash_pref_connector_manifest" ]
Expand Down
2 changes: 1 addition & 1 deletion chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -3279,7 +3279,7 @@ From <ph name="DOWNLOAD_DOMAIN">$3<ex>example.com</ex></ph>
<message name="IDS_UTILITY_PROCESS_IMAGE_WRITER_NAME" desc="The name of the utility process used for writing Chrome OS system images.">
Chrome OS System Image Writer
</message>
<message name="IDS_UTILITY_PROCESS_MEDIA_FILE_CHECKER_NAME" desc="The name of the utility process used for checking media files.">
<message name="IDS_UTILITY_PROCESS_MEDIA_GALLERY_UTILITY_NAME" desc="The name of the utility process used for checking media files.">
Media File Checker
</message>
<message name="IDS_UTILITY_PROCESS_MEDIA_LIBRARY_FILE_CHECKER_NAME" desc="The name of the utility process used for checking media library files.">
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2421,10 +2421,6 @@ split_static_library("browser") {
"media_galleries/fileapi/native_media_file_util.h",
"media_galleries/fileapi/readahead_file_stream_reader.cc",
"media_galleries/fileapi/readahead_file_stream_reader.h",
"media_galleries/fileapi/safe_audio_video_checker.cc",
"media_galleries/fileapi/safe_audio_video_checker.h",
"media_galleries/fileapi/safe_media_metadata_parser.cc",
"media_galleries/fileapi/safe_media_metadata_parser.h",
"media_galleries/fileapi/supported_audio_video_checker.cc",
"media_galleries/fileapi/supported_audio_video_checker.h",
"media_galleries/fileapi/supported_image_type_validator.cc",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ include_rules = [
"+chrome/install_static",
"+chrome/installer/util",
"+chrome/services/file_util/public",
"+chrome/services/media_gallery_util/public",
"+chrome/services/util_win/public/interfaces",
"+chrome_elf/blacklist",
"+chrome_elf/chrome_elf_constants.h",
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@
#include "chrome/browser/extensions/bookmark_app_navigation_throttle.h"
#include "chrome/browser/extensions/chrome_content_browser_client_extensions_part.h"
#include "chrome/browser/speech/extension_api/tts_engine_extension_api.h"
#include "chrome/services/media_gallery_util/public/interfaces/constants.mojom.h"
#include "components/guest_view/browser/guest_view_base.h"
#include "components/guest_view/browser/guest_view_manager.h"
#include "extensions/browser/extension_navigation_throttle.h"
Expand Down Expand Up @@ -3127,6 +3128,11 @@ void ChromeContentBrowserClient::RegisterOutOfProcessServices(
(*services)[profiling::mojom::kServiceName] =
base::ASCIIToUTF16("Profiling Service");

#if BUILDFLAG(ENABLE_EXTENSIONS)
(*services)[chrome::mojom::kMediaGalleryUtilServiceName] =
l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_MEDIA_GALLERY_UTILITY_NAME);
#endif

#if !defined(OS_ANDROID)
(*services)[chrome::mojom::kProfileImportServiceName] =
l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_PROFILE_IMPORTER_NAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"identity": [ "identity_manager" ],
// Only used in classic ash case.
"local_state": [ "pref_client" ],
"media_gallery_util": [ "parse_media" ],
"nacl_broker": [ "browser" ],
"nacl_loader": [ "browser" ],
"patch": [ "patch_file" ],
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ static_library("extensions") {
"//chrome/common/extensions/api:api_registration",
"//chrome/common/extensions/api:extensions_features",
"//chrome/common/safe_browsing:proto",
"//chrome/services/media_gallery_util/public/cpp",
"//components/app_modal",
"//components/autofill/content/browser",
"//components/bookmarks/browser",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/chrome_extension_function_details.h"
#include "chrome/browser/extensions/extension_tab_util.h"
#include "chrome/browser/media_galleries/fileapi/safe_media_metadata_parser.h"
#include "chrome/browser/media_galleries/gallery_watch_manager.h"
#include "chrome/browser/media_galleries/media_file_system_registry.h"
#include "chrome/browser/media_galleries/media_galleries_histograms.h"
Expand All @@ -38,6 +37,7 @@
#include "chrome/common/extensions/api/media_galleries.h"
#include "chrome/common/pref_names.h"
#include "chrome/grit/generated_resources.h"
#include "chrome/services/media_gallery_util/public/cpp/safe_media_metadata_parser.h"
#include "components/storage_monitor/storage_info.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h"
#include "content/public/browser/blob_handle.h"
Expand All @@ -48,6 +48,7 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/api/file_system/file_system_api.h"
#include "extensions/browser/app_window/app_window.h"
#include "extensions/browser/app_window/app_window_registry.h"
Expand Down Expand Up @@ -673,12 +674,14 @@ void MediaGalleriesGetMetadataFunction::GetMetadata(
metadata_type == MediaGalleries::GET_METADATA_TYPE_ALL ||
metadata_type == MediaGalleries::GET_METADATA_TYPE_NONE;

scoped_refptr<metadata::SafeMediaMetadataParser> parser(
new metadata::SafeMediaMetadataParser(GetProfile(), blob_uuid,
total_blob_length, mime_type,
get_attached_images));
parser->Start(base::Bind(
&MediaGalleriesGetMetadataFunction::OnSafeMediaMetadataParserDone, this));
auto parser = base::MakeRefCounted<chrome::SafeMediaMetadataParser>(
GetProfile(), blob_uuid, total_blob_length, mime_type,
get_attached_images);
parser->Start(
content::ServiceManagerConnection::GetForProcess()->GetConnector(),
base::Bind(
&MediaGalleriesGetMetadataFunction::OnSafeMediaMetadataParserDone,
this));
}

void MediaGalleriesGetMetadataFunction::OnSafeMediaMetadataParserDone(
Expand Down
55 changes: 0 additions & 55 deletions chrome/browser/media_galleries/fileapi/safe_audio_video_checker.cc

This file was deleted.

53 changes: 0 additions & 53 deletions chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
#include "base/task_scheduler/post_task.h"
#include "base/task_scheduler/task_traits.h"
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h"
#include "chrome/services/media_gallery_util/public/cpp/safe_audio_video_checker.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/common/service_manager_connection.h"
#include "net/base/mime_util.h"
#include "services/service_manager/public/cpp/connector.h"
#include "third_party/WebKit/common/mime_util/mime_util.h"

namespace {
Expand Down Expand Up @@ -75,10 +77,9 @@ void SupportedAudioVideoChecker::StartPreWriteValidation(
DCHECK(callback_.is_null());
callback_ = result_callback;

base::PostTaskWithTraitsAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
base::BindOnce(&OpenBlocking, path_),
base::BindOnce(&SupportedAudioVideoChecker::OnFileOpen,
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(&SupportedAudioVideoChecker::RetrieveConnectorOnUIThread,
weak_factory_.GetWeakPtr()));
}

Expand All @@ -88,13 +89,48 @@ SupportedAudioVideoChecker::SupportedAudioVideoChecker(
weak_factory_(this) {
}

void SupportedAudioVideoChecker::OnFileOpen(base::File file) {
// static
void SupportedAudioVideoChecker::RetrieveConnectorOnUIThread(
base::WeakPtr<SupportedAudioVideoChecker> this_ptr) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
std::unique_ptr<service_manager::Connector> connector =
content::ServiceManagerConnection::GetForProcess()
->GetConnector()
->Clone();
// We need a fresh connector so that we can use it on the IO thread. It has
// to be retrieved from the UI thread. We must use static method and pass a
// WeakPtr around as WeakPtrs are not thread-safe.
content::BrowserThread::PostTask(
content::BrowserThread::IO, FROM_HERE,
base::BindOnce(&SupportedAudioVideoChecker::OnConnectorRetrieved,
this_ptr, std::move(connector)));
}

// static
void SupportedAudioVideoChecker::OnConnectorRetrieved(
base::WeakPtr<SupportedAudioVideoChecker> this_ptr,
std::unique_ptr<service_manager::Connector> connector) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (!this_ptr)
return;

base::PostTaskWithTraitsAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
base::BindOnce(&OpenBlocking, this_ptr->path_),
base::BindOnce(&SupportedAudioVideoChecker::OnFileOpen, this_ptr,
std::move(connector)));
}

void SupportedAudioVideoChecker::OnFileOpen(
std::unique_ptr<service_manager::Connector> connector,
base::File file) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (!file.IsValid()) {
callback_.Run(base::File::FILE_ERROR_SECURITY);
return;
}

safe_checker_ = new SafeAudioVideoChecker(std::move(file), callback_);
safe_checker_ = new SafeAudioVideoChecker(std::move(file), callback_,
std::move(connector));
safe_checker_->Start();
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_MEDIA_GALLERIES_FILEAPI_SUPPORTED_AUDIO_VIDEO_CHECKER_H_
#define CHROME_BROWSER_MEDIA_GALLERIES_FILEAPI_SUPPORTED_AUDIO_VIDEO_CHECKER_H_

#include <memory>

#include "base/files/file.h"
#include "base/files/file_path.h"
#include "base/macros.h"
Expand All @@ -15,6 +17,10 @@
class MediaFileValidatorFactory;
class SafeAudioVideoChecker;

namespace service_manager {
class Connector;
}

// Uses SafeAudioVideoChecker to validate supported audio and video files in
// the utility process and then uses AVScanningFileValidator to ask the OS to
// virus scan them. The entire file is not decoded so a positive result from
Expand All @@ -32,7 +38,15 @@ class SupportedAudioVideoChecker : public AVScanningFileValidator {

explicit SupportedAudioVideoChecker(const base::FilePath& file);

void OnFileOpen(base::File file);
static void RetrieveConnectorOnUIThread(
base::WeakPtr<SupportedAudioVideoChecker> this_ptr);

static void OnConnectorRetrieved(
base::WeakPtr<SupportedAudioVideoChecker> this_ptr,
std::unique_ptr<service_manager::Connector> connector);

void OnFileOpen(std::unique_ptr<service_manager::Connector> connector,
base::File file);

base::FilePath path_;
storage::CopyOrMoveFileValidator::ResultCallback callback_;
Expand Down
1 change: 0 additions & 1 deletion chrome/common/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ group("test_features") {

mojom("mojo_bindings") {
sources = [
"media_parser.mojom",
"mojom/inline_install.mojom",
"removable_storage_writer.mojom",
]
Expand Down
15 changes: 0 additions & 15 deletions chrome/common/extensions/media_parser.typemap

This file was deleted.

5 changes: 1 addition & 4 deletions chrome/common/extensions/typemaps.gni
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,4 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

typemaps = [
"//chrome/common/extensions/media_parser.typemap",
"//chrome/common/extensions/mojom/inline_install.typemap",
]
typemaps = [ "//chrome/common/extensions/mojom/inline_install.typemap" ]
Loading

0 comments on commit 069422f

Please sign in to comment.