New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 800540 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

extensions::SandboxedUnpacker should not use UtilityprocessMojoClient

Project Member Reported by jcivelli@chromium.org, Jan 9 2018

Issue description

As part of the effort to deprecate UtilityProcessHost, extensions::SandboxedUnpacker should be servicified instead of UtilityProcessMojoClient.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea8f3df127f940773ccb369c1ec9c071aac106ef

commit ea8f3df127f940773ccb369c1ec9c071aac106ef
Author: Jay Civelli <jcivelli@google.com>
Date: Wed Jan 24 05:17:32 2018

Changing SandboxedUnpacker image sanitization.

In preparation for moving extension unpacking to the browser process,
making the image sanitization (decoding/reencoding) be triggered by
SandboxedUnpacker in the browser process, using the new ImageSanitizer
class. ImageSanitizer decodes images safely using the data decoder
service and then reencodes them.

Bug:  800540 
Change-Id: I40edae3cbd4ab8f03a2494a8d615107bc7feff6d
Tbr: rockot@chromium.org, tsepez@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/870744
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531440}
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/BUILD.gn
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/chromeos/app_mode/kiosk_app_data.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/chromeos/app_mode/kiosk_external_update_validator.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/crx_installer.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/crx_installer.h
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/extension_service_test_base.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/extension_service_test_base.h
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/startup_helper.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/test_extension_system.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/chrome/browser/extensions/test_extension_system.h
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/BUILD.gn
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/DEPS
[add] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/image_sanitizer.cc
[add] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/image_sanitizer.h
[add] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/image_sanitizer_unittest.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/sandboxed_unpacker.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/sandboxed_unpacker.h
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/browser/sandboxed_unpacker_unittest.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/common/constants.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/common/constants.h
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/common/extension_unpacker.mojom
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/utility/unpacker.cc
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/utility/unpacker.h
[modify] https://crrev.com/ea8f3df127f940773ccb369c1ec9c071aac106ef/extensions/utility/unpacker_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0

commit 6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0
Author: Jay Civelli <jcivelli@google.com>
Date: Wed Jan 24 16:42:53 2018

Changing SandboxedUnpacker message catalog sanitization.

In preparation for moving extension unpacking to the browser process,
making the message catalog sanitization (decoding/reencoding of JSON
files) be triggered by SandboxedUnpacker in the browser process, using
the new JsonFileSanitizer class. JsonFileSanitizer decodes JSON files
safely using the data decoder service and then reencodes them.

Bug:  800540 
Change-Id: Ia0a1ced77e97abc801b9821f4ed3da044b294aa0
Reviewed-on: https://chromium-review.googlesource.com/871800
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531556}
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/BUILD.gn
[add] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/json_file_sanitizer.cc
[add] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/json_file_sanitizer.h
[add] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/json_file_sanitizer_unittest.cc
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/sandboxed_unpacker.cc
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/sandboxed_unpacker.h
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/browser/sandboxed_unpacker_unittest.cc
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/utility/unpacker.cc
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/utility/unpacker.h
[modify] https://crrev.com/6d0e68e73f0ae1f85b43cb1df4980603da3b6dd0/extensions/utility/unpacker_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72d7b437f9530ab480ad18a6d9576de1b0c5d4a2

commit 72d7b437f9530ab480ad18a6d9576de1b0c5d4a2
Author: Jay Civelli <jcivelli@google.com>
Date: Thu Jan 25 19:12:18 2018

Making ImageSanitizer less memory leak prone.

ImageSanitizer now clears its callbacks when finished to prevent holding
on to other objects and causing memory leaks.

Bug:  800540 
Change-Id: Ifcc4f403ee32df6819d906744c07a1f6bd0684ae
Reviewed-on: https://chromium-review.googlesource.com/887104
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531970}
[modify] https://crrev.com/72d7b437f9530ab480ad18a6d9576de1b0c5d4a2/extensions/browser/image_sanitizer.cc
[modify] https://crrev.com/72d7b437f9530ab480ad18a6d9576de1b0c5d4a2/extensions/browser/image_sanitizer_unittest.cc
[modify] https://crrev.com/72d7b437f9530ab480ad18a6d9576de1b0c5d4a2/extensions/browser/sandboxed_unpacker.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26a85640a7d70a4b35248dc804bf7891221e597b

commit 26a85640a7d70a4b35248dc804bf7891221e597b
Author: Jay Civelli <jcivelli@google.com>
Date: Fri Jan 26 21:29:39 2018

Moving the last part of extension unpacking to the browser process.

Moving the manifest parsing and extension validation from the
Unpacker class to SandboxedUnpacker. As a result, all the extension
unpacking is done from the browser process and the Unpacker class is
removed.

The 2 remaining methods in unpacker.cc still used for unzipping the
extension have been moved to utility_handler.cc and the associated
tests renamed from unpacker_unittest.cc to utility_handler_unittest.cc.
The unzipping will also eventually be moved to the browser process at
which point utility_handler files will be removed as well.

Tbr: rockot@chromium.org
Bug:  800540 
Change-Id: Ib286fa88edc96e4c186aba6e2ee1d9c2317efe84
Reviewed-on: https://chromium-review.googlesource.com/883967
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532064}
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/browser/BUILD.gn
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/browser/sandboxed_unpacker.cc
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/browser/sandboxed_unpacker.h
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/browser/sandboxed_unpacker_unittest.cc
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/common/extension_unpacker.mojom
[delete] https://crrev.com/5fec41e6a7702005b7b4f388deabb54ec91fcd8f/extensions/common/extension_unpacker.typemap
[delete] https://crrev.com/5fec41e6a7702005b7b4f388deabb54ec91fcd8f/extensions/common/typemaps.gni
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/utility/BUILD.gn
[delete] https://crrev.com/5fec41e6a7702005b7b4f388deabb54ec91fcd8f/extensions/utility/unpacker.cc
[delete] https://crrev.com/5fec41e6a7702005b7b4f388deabb54ec91fcd8f/extensions/utility/unpacker.h
[delete] https://crrev.com/5fec41e6a7702005b7b4f388deabb54ec91fcd8f/extensions/utility/unpacker_unittest.cc
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/utility/utility_handler.cc
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/utility/utility_handler.h
[add] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/extensions/utility/utility_handler_unittest.cc
[modify] https://crrev.com/26a85640a7d70a4b35248dc804bf7891221e597b/mojo/public/tools/bindings/chromium_bindings_configuration.gni

Status: Fixed (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94b4b8b47e56ab732ab13ddb00fe7a1b960b2f9f

commit 94b4b8b47e56ab732ab13ddb00fe7a1b960b2f9f
Author: Jay Civelli <jcivelli@google.com>
Date: Tue Mar 20 18:58:21 2018

Extension clean-up now that it does not use UtilityProcessHost

Removes references to extension specific mojo interface that have been
removed and simplifies ExtensionTest.

Bug:  800540 
Change-Id: Ib58d7a2ea103ce4c23e334c15be8a2b5b2151349
Reviewed-on: https://chromium-review.googlesource.com/969784
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544462}
[modify] https://crrev.com/94b4b8b47e56ab732ab13ddb00fe7a1b960b2f9f/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/94b4b8b47e56ab732ab13ddb00fe7a1b960b2f9f/extensions/BUILD.gn
[modify] https://crrev.com/94b4b8b47e56ab732ab13ddb00fe7a1b960b2f9f/extensions/browser/extensions_test.cc
[modify] https://crrev.com/94b4b8b47e56ab732ab13ddb00fe7a1b960b2f9f/extensions/browser/extensions_test.h
[delete] https://crrev.com/178958e618d585102f685944de2afd280c809e9a/extensions/test/data/OWNERS
[delete] https://crrev.com/178958e618d585102f685944de2afd280c809e9a/extensions/test/data/extensions_test_content_utility_manifest_overlay.json
[delete] https://crrev.com/178958e618d585102f685944de2afd280c809e9a/extensions/test/test_content_browser_client.cc
[delete] https://crrev.com/178958e618d585102f685944de2afd280c809e9a/extensions/test/test_content_browser_client.h

Sign in to add a comment