New issue
Advanced search Search tips

Issue 691410 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 692120
issue 597124



Sign in to add a comment

Mojo extensions/common/extension_utility_messages.h

Project Member Reported by noel@chromium.org, Feb 13 2017

Issue description

Mojo the IPC in this file.


 
Blocking: 692120
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 16 2017

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

commit c8702c4cdecf0859c34a5ae564c89463ac727967
Author: noel <noel@chromium.org>
Date: Thu Mar 16 08:51:19 2017

Convert utility process extension Unpacker IPC to mojo

Add ExtensionUnpacker mojo, used to (unzip and) unpack a chrome
extension, and expose it to the browser via the utility process
policy file. Make callers use a utility process mojo client.

Add [Native] IPC enum param traits via a mojo typemap to handle
extensions::Manifest::Location. Set the wire limit for the enum
value to one less than Manifest::NUM_LOCATIONS (for compat with
the pre-existing custom traits removed in this patch since this
patch auto-generates them: manifest_location_param_traits.cc).

In the extensions/utility/utility_handler.cc, retain the CHECKs
on the Manifest::Location enum limits while bug 692069 is still
being investigated.

Move DecodeImages declarations from the extension messages-file
into its own file (extension_utility_types.h) to decouple those
declarations from this messages-file. A follow-up patch deletes
the messages-file so DecodeImages needs a new home.

Update all BUILD.gn for using clients to directly depend on the
//extensions/common:common target (and hence, its :mojo) to fix
post-compile step failures (warning about under-specified BUILD
files via "targets is_dirty" errors, see review comment #169).

Add TestContentBrowserClient to the extension test harness, and
also an overlay file in test/data (needed to expose the mojo in
the extension test harness to the unit tests listed below).

Covered by existing unit tests:
  {sandboxed_unpacker, zipfile_installer}_unittest.cc

BUG= 691410 

Review-Url: https://codereview.chromium.org/2697463002
Cr-Commit-Position: refs/heads/master@{#457374}

[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/browser/extensions/zipfile_installer.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/browser/extensions/zipfile_installer.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/test/BUILD.gn
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/chrome/utility/extensions/extensions_handler.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/BUILD.gn
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/DEPS
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/browser/BUILD.gn
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/browser/extensions_test.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/browser/extensions_test.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/browser/sandboxed_unpacker.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/browser/sandboxed_unpacker.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/browser/sandboxed_unpacker_unittest.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/BUILD.gn
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/OWNERS
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/extension_messages.cc
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/extension_unpacker.mojom
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/extension_utility_messages.h
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/extension_utility_types.h
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/manifest_location.typemap
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/manifest_location_param_traits.cc
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/manifest_location_param_traits.h
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/common/typemaps.gni
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/test/data/extensions_test_content_utility_manifest_overlay.json
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/test/test_content_browser_client.cc
[add] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/test/test_content_browser_client.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/test/test_content_utility_client.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/test/test_content_utility_client.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/utility/unpacker.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/utility/unpacker.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/utility/utility_handler.cc
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/extensions/utility/utility_handler.h
[modify] https://crrev.com/c8702c4cdecf0859c34a5ae564c89463ac727967/mojo/public/tools/bindings/chromium_bindings_configuration.gni

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 21 2017

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

commit bcca171b7b0b1bf4b086d66b0906d2ff8cc2a4f4
Author: noel <noel@chromium.org>
Date: Tue Mar 21 11:19:50 2017

Utility process: use interfaces rather than overloaded term services

The term "service" gets overloaded a lot. Since services are a real
thing and not the same thing as mojom interfaces, it's important to
distinguish them in code and comments [1].

Here fix the comments in the utility process where we should'a used
the word "interfaces". No change in behaviour, no new tests.

[1] https://codereview.chromium.org/2697463002/#msg203

BUG= 691410 

Review-Url: https://codereview.chromium.org/2764443002
Cr-Commit-Position: refs/heads/master@{#458374}

[modify] https://crrev.com/bcca171b7b0b1bf4b086d66b0906d2ff8cc2a4f4/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/bcca171b7b0b1bf4b086d66b0906d2ff8cc2a4f4/chrome/utility/extensions/extensions_handler.cc
[modify] https://crrev.com/bcca171b7b0b1bf4b086d66b0906d2ff8cc2a4f4/extensions/utility/utility_handler.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 29 2017

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

commit 825202ab2ec8a1d8eba2a08f481773e07641dda3
Author: noel <noel@chromium.org>
Date: Wed Mar 29 04:37:19 2017

Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file: no longer needed or used.

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG= 691410 

Review-Url: https://codereview.chromium.org/2699663003
Cr-Commit-Position: refs/heads/master@{#460280}

[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/chrome/utility/extensions/extensions_handler.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/chrome/utility/extensions/extensions_handler.h
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/browser/updater/extension_downloader.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/browser/updater/safe_manifest_parser.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/browser/updater/safe_manifest_parser.h
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/BUILD.gn
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/extension_message_generator.h
[delete] https://crrev.com/ce3ccb9d8093a1eb542098f2e68fd51c90fa989d/extensions/common/extension_utility_messages.h
[add] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/manifest_parser.mojom
[add] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/manifest_parser.typemap
[add] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/manifest_parser_struct_traits.cc
[add] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/manifest_parser_struct_traits.h
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/typemaps.gni
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/update_manifest.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/common/update_manifest.h
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/shell/utility/shell_content_utility_client.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/shell/utility/shell_content_utility_client.h
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/test/data/extensions_test_content_utility_manifest_overlay.json
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/test/test_content_utility_client.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/test/test_content_utility_client.h
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/utility/utility_handler.cc
[modify] https://crrev.com/825202ab2ec8a1d8eba2a08f481773e07641dda3/extensions/utility/utility_handler.h

Comment 5 by noel@chromium.org, Mar 29 2017

Owner: noel@chromium.org
Status: Fixed (was: Untriaged)
^^^^ [delete] https://crrev.com/ce3ccb9d8093a1eb542098f2e68fd51c90fa989d/extensions/common/extension_utility_messages.h

Closing bug: fixed.

Comment 6 by noel@chromium.org, Mar 29 2017

Status: Started (was: Fixed)
Oh, minor thing needed to conclude. With the messages filed deleted, the IPC START value defined for it messages is also not needed.  I've uploaded a patch to remove that: https://codereview.chromium.org/2785463003


Project Member

Comment 7 by bugdroid1@chromium.org, Mar 29 2017

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

commit 0a5a46198c69904bc7d92d27a2b1ab654a52ef1b
Author: noel <noel@chromium.org>
Date: Wed Mar 29 22:59:25 2017

Remove IPC Message start entry ExtensionUtilityMsgStart

The extensions/common/extension_utility_messages.h message
file was deleted in http://crrev.com/460280

Its corresponding IPC START entry ExtensionUtilityMsgStart
is not needed either, so we can remove it.

BUG= 691410 

Review-Url: https://codereview.chromium.org/2785463003
Cr-Commit-Position: refs/heads/master@{#460564}

[modify] https://crrev.com/0a5a46198c69904bc7d92d27a2b1ab654a52ef1b/ipc/ipc_message_start.h

Comment 8 by noel@chromium.org, Mar 29 2017

Status: Fixed (was: Started)

Sign in to add a comment