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

Issue 597124 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 604282
issue 680928
issue 691410
issue 738648

Blocking:
issue 577685



Sign in to add a comment

Mojoify the utility process.

Project Member Reported by amistry@chromium.org, Mar 23 2016

Issue description

See summary. 'nuff said.

 
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 5 2016

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

commit 7980086d9047dbda9ea991b7de25ac77de1bae6a
Author: amistry <amistry@chromium.org>
Date: Tue Apr 05 03:00:52 2016

Avoid adding Mojo service to the utility process when running with elevated privileges.

BUG= 597124 

Review URL: https://codereview.chromium.org/1852153002

Cr-Commit-Position: refs/heads/master@{#385094}

[modify] https://crrev.com/7980086d9047dbda9ea991b7de25ac77de1bae6a/chrome/utility/chrome_content_utility_client.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 5 2016

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

commit 6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d
Author: amistry <amistry@chromium.org>
Date: Tue Apr 05 20:42:07 2016

Convert the utility process image decoder into a Mojo service.

BUG= 597124 

Review URL: https://codereview.chromium.org/1844103004

Cr-Commit-Position: refs/heads/master@{#385278}

[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/BUILD.gn
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/DEPS
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/image_decoder.cc
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/image_decoder.h
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/chrome_common.gypi
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/chrome_utility.gypi
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/common/BUILD.gn
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/common/chrome_utility_messages.h
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/common/image_decoder.mojom
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/BUILD.gn
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/DEPS
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/chrome_content_utility_client.h
[delete] https://crrev.com/f368f9504083392dc3fbcf2a57418318115d72db/chrome/utility/chrome_content_utility_client_unittest.cc
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/image_decoder_impl.cc
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/image_decoder_impl.h
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/image_decoder_impl_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 5 2016

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

commit 6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d
Author: amistry <amistry@chromium.org>
Date: Tue Apr 05 20:42:07 2016

Convert the utility process image decoder into a Mojo service.

BUG= 597124 

Review URL: https://codereview.chromium.org/1844103004

Cr-Commit-Position: refs/heads/master@{#385278}

[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/BUILD.gn
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/DEPS
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/image_decoder.cc
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/browser/image_decoder.h
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/chrome_common.gypi
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/chrome_utility.gypi
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/common/BUILD.gn
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/common/chrome_utility_messages.h
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/common/image_decoder.mojom
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/BUILD.gn
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/DEPS
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/chrome_content_utility_client.h
[delete] https://crrev.com/f368f9504083392dc3fbcf2a57418318115d72db/chrome/utility/chrome_content_utility_client_unittest.cc
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/image_decoder_impl.cc
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/image_decoder_impl.h
[add] https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d/chrome/utility/image_decoder_impl_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 8 2016

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

commit c7a7a767d643aef68eb2af82f961c11872abe59d
Author: amistry <amistry@chromium.org>
Date: Fri Apr 08 04:21:54 2016

Make IPC::ParamTraits<>GetSize work with base::Value as a result of JSON parsing.

The JSON parser uses a JSONStringValue for string values, which looks almost
like a base::StringValue, but doesn't support GetAsString(base::StringValue**).

BUG= 597124 

Review URL: https://codereview.chromium.org/1866633002

Cr-Commit-Position: refs/heads/master@{#385989}

[modify] https://crrev.com/c7a7a767d643aef68eb2af82f961c11872abe59d/ipc/ipc_message_utils.cc
[modify] https://crrev.com/c7a7a767d643aef68eb2af82f961c11872abe59d/ipc/ipc_message_utils_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 12 2016

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

commit a9947c2ff694a3cbe6f646113e3372f03d2cc607
Author: amistry <amistry@chromium.org>
Date: Tue Apr 12 04:28:25 2016

Use token-based initialisation for the utility process MojoApplication.

BUG= 597124 

Review URL: https://codereview.chromium.org/1877743002

Cr-Commit-Position: refs/heads/master@{#386584}

[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/browser/mojo/mojo_application_host.cc
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/browser/mojo/mojo_application_host.h
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/browser/utility_process_host_impl.cc
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/browser/utility_process_host_impl.h
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/child/child_thread_impl.cc
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/child/child_thread_impl.h
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/child/mojo/mojo_application.cc
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/child/mojo/mojo_application.h
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/common/in_process_child_thread_params.cc
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/common/in_process_child_thread_params.h
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/public/common/mojo_channel_switches.cc
[modify] https://crrev.com/a9947c2ff694a3cbe6f646113e3372f03d2cc607/content/public/common/mojo_channel_switches.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 18 2016

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

commit 1bd8d3bba0e883748dcbf7c89c335c9864b39f2f
Author: amistry <amistry@chromium.org>
Date: Mon Apr 18 00:11:58 2016

Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG= 597124 

Review URL: https://codereview.chromium.org/1885813002

Cr-Commit-Position: refs/heads/master@{#387835}

[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/media_galleries/fileapi/safe_audio_video_checker.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/media_galleries/fileapi/safe_iapps_library_parser.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/media_galleries/fileapi/safe_iapps_library_parser.h
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/media_galleries/fileapi/safe_picasa_album_table_reader.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/media_galleries/fileapi/safe_picasa_album_table_reader.h
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/printing/pwg_raster_converter.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/safe_browsing/sandboxed_zip_analyzer.cc
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/browser/safe_browsing/sandboxed_zip_analyzer.h
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/common/chrome_utility_messages.h
[modify] https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f/chrome/utility/chrome_content_utility_client.cc

Blockedon: 604282
Project Member

Comment 9 by bugdroid1@chromium.org, May 4 2016

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

commit f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf
Author: amistry <amistry@chromium.org>
Date: Wed May 04 08:55:25 2016

Convert the utility process JSON parser into a Mojo service.

BUG= 597124 

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

[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/chrome/chrome_utility.gypi
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/chrome/utility/BUILD.gn
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/chrome/utility/chrome_content_utility_client.cc
[delete] https://crrev.com/cb3bd1e8780cdd8d87ceb617a5b804607ed9098a/chrome/utility/safe_json_parser_handler.cc
[delete] https://crrev.com/cb3bd1e8780cdd8d87ceb617a5b804607ed9098a/chrome/utility/safe_json_parser_handler.h
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json.gypi
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/BUILD.gn
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/DEPS
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/public/interfaces/BUILD.gn
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/public/interfaces/safe_json.mojom
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/public/interfaces/safe_json.typemap
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/safe_json_parser_impl.cc
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/safe_json_parser_impl.h
[delete] https://crrev.com/cb3bd1e8780cdd8d87ceb617a5b804607ed9098a/components/safe_json/safe_json_parser_message_filter.cc
[delete] https://crrev.com/cb3bd1e8780cdd8d87ceb617a5b804607ed9098a/components/safe_json/safe_json_parser_message_filter.h
[delete] https://crrev.com/cb3bd1e8780cdd8d87ceb617a5b804607ed9098a/components/safe_json/safe_json_parser_messages.cc
[delete] https://crrev.com/cb3bd1e8780cdd8d87ceb617a5b804607ed9098a/components/safe_json/safe_json_parser_messages.h
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/utility/BUILD.gn
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/utility/safe_json_parser_mojo_impl.cc
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/safe_json/utility/safe_json_parser_mojo_impl.h
[add] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/components/typemaps.gni
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/ipc/ipc_message_start.h
[modify] https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf/mojo/public/tools/bindings/chromium_bindings_configuration.gni

Project Member

Comment 10 by bugdroid1@chromium.org, May 4 2016

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

commit 3a007c9f5503470c0aa174140b91c0694625a0bc
Author: mathp <mathp@chromium.org>
Date: Wed May 04 12:13:10 2016

Revert of Convert the utility process JSON parser into a Mojo service. (patchset #13 id:240001 of https://codereview.chromium.org/1861573002/ )

Reason for revert:
broke browsertest:

https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/14012

Original issue's description:
> Convert the utility process JSON parser into a Mojo service.
>
> BUG= 597124 
>
> Committed: https://crrev.com/f8ce981e73207dfb46f6ccbfe2cd29378b3c4baf
> Cr-Commit-Position: refs/heads/master@{#391463}

TBR=sdefresne@chromium.org,bauerb@chromium.org,thakis@chromium.org,tsepez@chromium.org,amistry@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 597124 

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

[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/chrome/chrome_utility.gypi
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/chrome/utility/BUILD.gn
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/chrome/utility/chrome_content_utility_client.cc
[add] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/chrome/utility/safe_json_parser_handler.cc
[add] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/chrome/utility/safe_json_parser_handler.h
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json.gypi
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/BUILD.gn
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/DEPS
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/safe_json/public/interfaces/BUILD.gn
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/safe_json/public/interfaces/safe_json.mojom
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/safe_json/public/interfaces/safe_json.typemap
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/safe_json_parser_impl.cc
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/safe_json_parser_impl.h
[add] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/safe_json_parser_message_filter.cc
[add] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/safe_json_parser_message_filter.h
[add] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/safe_json_parser_messages.cc
[add] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/components/safe_json/safe_json_parser_messages.h
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/safe_json/utility/BUILD.gn
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/safe_json/utility/safe_json_parser_mojo_impl.cc
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/safe_json/utility/safe_json_parser_mojo_impl.h
[delete] https://crrev.com/b7ceb29f22004c344c982e48de1e436834b69274/components/typemaps.gni
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/ipc/ipc_message_start.h
[modify] https://crrev.com/3a007c9f5503470c0aa174140b91c0694625a0bc/mojo/public/tools/bindings/chromium_bindings_configuration.gni

Blocking: 577685
Project Member

Comment 12 by bugdroid1@chromium.org, May 10 2016

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

commit b84adb70aa05a1f9af36f6eca29cf3cf760c02bf
Author: amistry <amistry@chromium.org>
Date: Tue May 10 00:46:10 2016

Avoid incrementing an invalid iterator in the utility process client.

BUG= 597124 

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

[modify] https://crrev.com/b84adb70aa05a1f9af36f6eca29cf3cf760c02bf/chrome/utility/chrome_content_utility_client.cc

Project Member

Comment 13 by bugdroid1@chromium.org, May 11 2016

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

commit ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b
Author: amistry <amistry@chromium.org>
Date: Wed May 11 06:44:18 2016

Re-land: Convert the utility process JSON parser into a Mojo service.

Original change: https://codereview.chromium.org/1861573002

This change is idential to the original.

BUG= 597124 
TBR=bauerb@chromium.org,sdefresne@chromium.org,tsepez@chromium.org,thakis@chromium.org

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

[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/chrome/chrome_utility.gypi
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/chrome/utility/BUILD.gn
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/chrome/utility/chrome_content_utility_client.cc
[delete] https://crrev.com/6106983010a1c937e602d9b11521e4dc9464c3e7/chrome/utility/safe_json_parser_handler.cc
[delete] https://crrev.com/6106983010a1c937e602d9b11521e4dc9464c3e7/chrome/utility/safe_json_parser_handler.h
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json.gypi
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/BUILD.gn
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/DEPS
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/public/interfaces/BUILD.gn
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/public/interfaces/safe_json.mojom
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/public/interfaces/safe_json.typemap
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/safe_json_parser_impl.cc
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/safe_json_parser_impl.h
[delete] https://crrev.com/6106983010a1c937e602d9b11521e4dc9464c3e7/components/safe_json/safe_json_parser_message_filter.cc
[delete] https://crrev.com/6106983010a1c937e602d9b11521e4dc9464c3e7/components/safe_json/safe_json_parser_message_filter.h
[delete] https://crrev.com/6106983010a1c937e602d9b11521e4dc9464c3e7/components/safe_json/safe_json_parser_messages.cc
[delete] https://crrev.com/6106983010a1c937e602d9b11521e4dc9464c3e7/components/safe_json/safe_json_parser_messages.h
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/utility/BUILD.gn
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/utility/safe_json_parser_mojo_impl.cc
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/safe_json/utility/safe_json_parser_mojo_impl.h
[add] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/components/typemaps.gni
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/ipc/ipc_message_start.h
[modify] https://crrev.com/ab6f6ae93d12cf5913efbb7f1f1f6c34e2cab39b/mojo/public/tools/bindings/chromium_bindings_configuration.gni

Cc: sa...@chromium.org tibell@chromium.org
Owner: ----
Status: Available (was: Assigned)

Comment 17 by sa...@chromium.org, Dec 13 2016

Owner: noel@chromium.org
Project Member

Comment 18 by bugdroid1@chromium.org, Dec 23 2016

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

commit 32afc44b332255130c247e3bfc4a27261d11ddfb
Author: noel <noel@chromium.org>
Date: Fri Dec 23 08:28:46 2016

Convert utility process out-of-process file patching to mojo

Define interface chrome::mojom::FilePatcher for out-of-process file
patching. Add that interface to the utility process, and expose the
interface to the browser process by mojo policy [1].

ChromeOutOfProcessPatcher: remove PatchHost() helper class, replace
it with a content::UtilityProcessMojoClient() and use that to start
and stop the utility process needed for out-of-process file patches
and to perform out-of-process file patches using mojo calls, rather
than IPC::Send().

Add a in-process browser test for the ChromeOutOfProcessPatcher (it
had no tests http://bit.ly/2h8mafX it seems). Copy the golden files
to random locations (input_file, patch_file) and patch them: checks
the patched output_file matches the golden output file.

Delete all the IPC related to out-of-process file patching since we
no longer use them, we use chrome::mojom::FilePatcher instead.

[1] chrome_content_utility_manifest_overlay.json

TBR=waffles@chromium.org
BUG= 597124 ,  660325 

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

[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/browser/component_updater/component_patcher_operation_out_of_process.cc
[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/browser/component_updater/component_patcher_operation_out_of_process.h
[add] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/browser/component_updater/component_patcher_operation_out_of_process_browsertest.cc
[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/common/BUILD.gn
[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/common/chrome_utility_messages.h
[add] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/common/file_patcher.mojom
[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/test/BUILD.gn
[modify] https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb/chrome/utility/chrome_content_utility_client.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 25 2016

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

commit 53919401199f8cc8c0e5042dd9c0fc9716825a1c
Author: wfh <wfh@chromium.org>
Date: Sun Dec 25 21:30:26 2016

Revert of Convert utility process out-of-process file patching to mojo (patchset #15 id:420001 of https://codereview.chromium.org/2566053002/ )

Reason for revert:
broke component updater - see  crbug.com/676960 

Original issue's description:
> Convert utility process out-of-process file patching to mojo
>
> Define interface chrome::mojom::FilePatcher for out-of-process file
> patching. Add that interface to the utility process, and expose the
> interface to the browser process by mojo policy [1].
>
> ChromeOutOfProcessPatcher: remove PatchHost() helper class, replace
> it with a content::UtilityProcessMojoClient() and use that to start
> and stop the utility process needed for out-of-process file patches
> and to perform out-of-process file patches using mojo calls, rather
> than IPC::Send().
>
> Add a in-process browser test for the ChromeOutOfProcessPatcher (it
> had no tests http://bit.ly/2h8mafX it seems). Copy the golden files
> to random locations (input_file, patch_file) and patch them: checks
> the patched output_file matches the golden output file.
>
> Delete all the IPC related to out-of-process file patching since we
> no longer use them, we use chrome::mojom::FilePatcher instead.
>
> [1] chrome_content_utility_manifest_overlay.json
>
> TBR=waffles@chromium.org
> BUG= 597124 ,  660325 
>
> Committed: https://crrev.com/32afc44b332255130c247e3bfc4a27261d11ddfb
> Cr-Commit-Position: refs/heads/master@{#440601}

TBR=jochen@chromium.org,dcheng@chromium.org,sammc@chromium.org,tibell@chromium.org,waffles@chromium.org,sorin@chromium.org,noel@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 597124 ,  660325 , 676960 

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

[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/browser/component_updater/component_patcher_operation_out_of_process.cc
[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/browser/component_updater/component_patcher_operation_out_of_process.h
[delete] https://crrev.com/79c889974fa1506fa36408f2bf4538f4a7ca70cf/chrome/browser/component_updater/component_patcher_operation_out_of_process_browsertest.cc
[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/common/BUILD.gn
[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/common/chrome_utility_messages.h
[delete] https://crrev.com/79c889974fa1506fa36408f2bf4538f4a7ca70cf/chrome/common/file_patcher.mojom
[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/test/BUILD.gn
[modify] https://crrev.com/53919401199f8cc8c0e5042dd9c0fc9716825a1c/chrome/utility/chrome_content_utility_client.cc

Comment 20 by noel@chromium.org, Jan 13 2017

Blockedon: 680928
Project Member

Comment 21 by bugdroid1@chromium.org, Jan 23 2017

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

commit bfee08c642470b8a7d39988d718af6584f567adc
Author: noel <noel@chromium.org>
Date: Mon Jan 23 05:02:25 2017

Convert utility process out-of-process file patching IPC to mojo

Define interface chrome::mojom::FilePatcher for out-of-process file
patching. Add the interface to the utility process and expose it to
to the browser process by mojo policy [1].

ChromeOutOfProcessPatcher: remove PatchHost() helper class, replace
it with a content::UtilityProcessMojoClient() and use that to start
and stop the utility process needed for out-of-process file patches
and to perform out-of-process file patches using mojo calls, rather
than IPC::Send(). Move the mojo calls to the IO thread to avoid bug
676960 (mojo does not work from the blocking pool).

Add an in-process browsertest for the ChromeOutOfProcessPatcher (it
had no tests http://bit.ly/2h8mafX it seems). Copy the golden files
to random locations (input_file, patch_file) and patch them: checks
the patched output_file content matches the golden output file. Add
tests for invalid file inputs to the file patcher. Note all work in
the tests originates from the blocking pool:  bug 676960 .

[1] chrome_content_utility_manifest_overlay.json

BUG= 597124 

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

[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/browser/component_updater/component_patcher_operation_out_of_process.cc
[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/browser/component_updater/component_patcher_operation_out_of_process.h
[add] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/browser/component_updater/component_patcher_operation_out_of_process_browsertest.cc
[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/common/BUILD.gn
[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/common/chrome_utility_messages.h
[add] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/common/file_patcher.mojom
[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/test/BUILD.gn
[modify] https://crrev.com/bfee08c642470b8a7d39988d718af6584f567adc/chrome/utility/chrome_content_utility_client.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Jan 28 2017

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

commit a92d829bc8a9e2c3343010d3691b9da93b7275eb
Author: noel <noel@chromium.org>
Date: Sat Jan 28 02:31:54 2017

Content Utility Client: OnUnpackWebResource is not used

OnUnpackWebResource() is an orphan declaration. Let's remove
it since there is no corresponding definition.

BUG= 597124 

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

[modify] https://crrev.com/a92d829bc8a9e2c3343010d3691b9da93b7275eb/chrome/utility/chrome_content_utility_client.h

Project Member

Comment 23 by bugdroid1@chromium.org, Feb 2 2017

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

commit fe451778fade1e59bd5c422833795f0836ef08a2
Author: noel <noel@chromium.org>
Date: Thu Feb 02 03:38:57 2017

Remove unused out-of-process file patching IPC handlers

OnPatchFileBsdiff and OnPatchFileCourgette are no longer used and have
no corresponding definition. Remove them.

TBR=jochen@chromium.org
BUG= 597124 

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

[modify] https://crrev.com/fe451778fade1e59bd5c422833795f0836ef08a2/chrome/utility/chrome_content_utility_client.h

Comment 24 Deleted

Comment 25 by noel@chromium.org, Feb 13 2017

Blockedon: 691410
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 1 2017

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

commit b8721b0aa68a020f092aede9b18b33389ed24e8f
Author: noel <noel@chromium.org>
Date: Wed Mar 01 03:28:44 2017

Convert utility process zip creator IPC to mojo

Convert the OS_CHROMEOS-only zip creator IPC to use mojo rather
than IPC. Expose the mojo to the browser process using the mojo
utility process policy file. Update the client to call the mojo
service using a mojo utility client, remove all dependencies on
UtilityProcessHostClient. Add a new mojo utility client api:

  utility_process_mojo_client_->set_exposed_directory(dir)

to allow mojo utility clients to expose the target directory in
the utility process sandbox just like UtilityProcessHostClient.
Minor name changes, no abbrvs.

Now in chrome/utility/chrome_content_utility_client.cc continue
the saga of deleting the Send and ReleaseProcessIfNeeded helper
routines: make remaining users call these services directly and
remove the helpers. Add the mojo ZipFileCreatorImpl class.

Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per
https://codereview.chromium.org/2703463005

BUG= 597124 

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

[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/browser/chromeos/file_manager/zip_file_creator.cc
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/browser/chromeos/file_manager/zip_file_creator.h
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/common/BUILD.gn
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/common/chrome_utility_messages.h
[add] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/common/zip_file_creator.mojom
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/chrome/utility/chrome_content_utility_client.h
[modify] https://crrev.com/b8721b0aa68a020f092aede9b18b33389ed24e8f/content/public/browser/utility_process_mojo_client.h

Project Member

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

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

commit e9cc49145670e9a034505aeda4f9ee7bb0a7b3c6
Author: noel <noel@chromium.org>
Date: Thu Mar 02 04:42:14 2017

Convert utility process zip creator IPC to mojo: part 2

Follow on from [1], the comment added there about |this| during
the mojo callback is wrong. Checking this->hasOneRef() after we
reset() the mojo client shows there's always one reference left
in all cases (mojo error callback, mojo success callback).

Ditch the comment. The reset and client callback can be done in
any order. Invoke the client callback with a one-liner.

Covered by existing tests: ZipFileCreatorTest.SomeFilesZip, per
https://codereview.chromium.org/2703463005

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

TBR=jochen@chromium.org
BUG= 597124 

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

[modify] https://crrev.com/e9cc49145670e9a034505aeda4f9ee7bb0a7b3c6/chrome/browser/chromeos/file_manager/zip_file_creator.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Mar 11 2017

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

commit 31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914
Author: noel <noel@chromium.org>
Date: Sat Mar 11 02:03:29 2017

Convert utility process Safe Browsing ZIP/DMG Analyzer IPC to mojo

Add SafeArchiveAnalyzer mojom service used to inspect ZIP / DMG
file archives for safe browsing download protection. Expose the
mojo to the browser via the utility process policy file. Update
clients to call the service using a mojo utility client, remove
all dependencies on UtilityProcessHostClient. Add native traits
for the file analyze results.

Align the ZIP/DMG clients: make their API's match, remove class
base::File fields (curry the file(s) to the analyzer step), and
remove ZIP client temp file clean-up code (not needed since the
utility process now owns the files).

Covered by existing tests:

SandboxedDMGAnalyzerTest.AnalyzeDMG, SandboxedZipAnalyzerTest.*

both of which use a content::InProcessUtilityThreadHelper viz.,
SafeArchiveAnalyzer mojo is called by these tests.

BUG= 597124 

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

[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac_unittest.cc
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/browser/safe_browsing/sandboxed_zip_analyzer.cc
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/browser/safe_browsing/sandboxed_zip_analyzer.h
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/common/BUILD.gn
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/common/chrome_utility_messages.h
[add] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/common/safe_archive_analyzer.mojom
[add] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/common/safe_archive_analyzer.typemap
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/typemaps.gni
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/utility/chrome_content_utility_client.cc
[modify] https://crrev.com/31e7b9090b8a1301e5c51a47f5bbd5bbe37e8914/chrome/utility/chrome_content_utility_client.h

Project Member

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

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

commit 2ff97c10b4f5ba659bce7de71fa8cd6f818d3839
Author: noel <noel@chromium.org>
Date: Tue Mar 14 23:40:10 2017

Out-of-process file patcher: create mojo client with MakeUnique<>

Create the utility process mojo client with base::MakeUnique<>. No
change in behavior, no new tests.

BUG= 597124 

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

[modify] https://crrev.com/2ff97c10b4f5ba659bce7de71fa8cd6f818d3839/chrome/browser/component_updater/component_patcher_operation_out_of_process.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Mar 15 2017

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

commit 66e59dfbe693f78e9aa030af3ae8cfe835246cbe
Author: noel <noel@chromium.org>
Date: Wed Mar 15 23:25:38 2017

Out-of-process file patcher: DCHECK the files in the utility process

In chrome_content_utility_client.cc, the files received for patching are
never invalid. DCHECK that, as we have done in subsequent patches mojo'n
the utility process when the mojo API includes a base::File.

No change in behavior, no new tests.

BUG= 597124 

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

[modify] https://crrev.com/66e59dfbe693f78e9aa030af3ae8cfe835246cbe/chrome/utility/chrome_content_utility_client.cc

Status: Assigned (was: Available)
Just updating bug statuses so that "Available" means available. If you don't want to own this anymore, please revert it to Available and remove the owner value. Thanks!

Comment 32 by noel@chromium.org, May 29 2017

Blockedon: -688897

Comment 33 by noel@chromium.org, Jun 16 2017

Depends on issue 73098 which had a patch from the time of gyp for the last IPC in the list.  I pinged on the bug a while back (issue 73098#c42), no response.

I will dust off the patch from that bug, update to use mojo and gn build, and see if I can create a new patch ...
Project Member

Comment 34 by bugdroid1@chromium.org, Jun 27 2017

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

commit 8584e4dc5242f4519c333ffd5ebbe671d68373f4
Author: Noel Gordon <noel@chromium.org>
Date: Tue Jun 27 22:55:39 2017

Convert utility process Shell Handler Win IPC to mojo

Shell operations can cause 3rd-party shell extensions to be loaded into
the calling process (browser). The utility process protects the browser
process from potential instability 3rd-party extensions can cause.

The feature is enabled by the IsolateShellOperations feature flag which
is off by default, in which case, Chrome calls the WIN shell operations
(::GetOpenFileName and ::GetSaveFileName) directly.

Change the client interface for these Shell Handler Win IPC to use mojo
and move the receiver in the utility process into the ShellHandler mojo
interface. Make the mojo calls SYNC: the old code used WaitableEvent to
emulate SYNC IPC. Remove that code, use real SYNC calls instead.

Remove last chrome_utility_messages.h IPC, add clean-up TODO (this file
should be renamed param traits now there are no more IPC in it).

Patch is based on an earlier patch by pmonette@ [1], which was reverted
due to gyp DBG build problems (see issue 73098).

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

Bug:  597124 , 73098
Cq-Include-Trybots: master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: Ia4ef401bbc343428cbdb5fc1db58e64c06c82c93
Reviewed-on: https://chromium-review.googlesource.com/538519
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482779}
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/browser/chrome_browser_main_win.cc
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/browser/win/chrome_select_file_dialog_factory.cc
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/browser/win/chrome_select_file_dialog_factory.h
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/common/BUILD.gn
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/common/chrome_utility_messages.h
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/common/shell_handler_win.mojom
[add] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/common/shell_handler_win.typemap
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/typemaps.gni
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/utility/BUILD.gn
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/utility/chrome_content_utility_client.cc
[delete] https://crrev.com/ade8eb2c8224a0ba5ae97a2350812d65b9c496eb/chrome/utility/ipc_shell_handler_win.cc
[delete] https://crrev.com/ade8eb2c8224a0ba5ae97a2350812d65b9c496eb/chrome/utility/ipc_shell_handler_win.h
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/utility/shell_handler_impl_win.cc
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/chrome/utility/shell_handler_impl_win.h
[modify] https://crrev.com/8584e4dc5242f4519c333ffd5ebbe671d68373f4/mojo/public/cpp/bindings/sync_call_restrictions.h

Project Member

Comment 35 by bugdroid1@chromium.org, Jun 28 2017

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

commit d4c0b80def96186ea0662090ea9c54f22ee5c4cd
Author: Noel Gordon <noel@chromium.org>
Date: Wed Jun 28 22:55:34 2017

Rename chrome/common/chrome_utility_messages.h

This file has no IPC messages anymore: it only defines param traits
specific to the safe browsing feature.

Move the file to the safe_browsing directory, along with its mojo and
typemap files, rename the file, update BUILD files.

Bug:  597124 
Change-Id: I96b88de4a41615ff9a7b77c7e40b3b317cfab57b
Reviewed-on: https://chromium-review.googlesource.com/548615
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483181}
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/browser/media_galleries/fileapi/safe_iapps_library_parser.cc
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/browser/media_galleries/fileapi/safe_picasa_album_table_reader.cc
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/browser/printing/pwg_raster_converter.cc
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/browser/safe_browsing/sandboxed_zip_analyzer.h
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/BUILD.gn
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/common_message_generator.h
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/safe_browsing/BUILD.gn
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/safe_browsing/OWNERS
[rename] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/safe_browsing/safe_archive_analyzer.mojom
[rename] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/safe_browsing/safe_archive_analyzer.typemap
[rename] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/common/safe_browsing/safe_archive_analyzer_param_traits.h
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/typemaps.gni
[modify] https://crrev.com/d4c0b80def96186ea0662090ea9c54f22ee5c4cd/chrome/utility/chrome_content_utility_client.cc

Comment 36 by noel@chromium.org, Jun 28 2017

Status: Fixed (was: Assigned)
Project Member

Comment 37 by bugdroid1@chromium.org, Jun 28 2017

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

commit 143e13bc7c099d7b244aaae23adb4aade2e83282
Author: Mike Wasserman <msw@google.com>
Date: Wed Jun 28 23:41:01 2017

Revert "Rename chrome/common/chrome_utility_messages.h"

This reverts commit d4c0b80def96186ea0662090ea9c54f22ee5c4cd.

Reason for revert: Looks like this caused a Mac compile failure on the main waterfall:
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium%2FMac%2F29509%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Original change's description:
> Rename chrome/common/chrome_utility_messages.h
> 
> This file has no IPC messages anymore: it only defines param traits
> specific to the safe browsing feature.
> 
> Move the file to the safe_browsing directory, along with its mojo and
> typemap files, rename the file, update BUILD files.
> 
> Bug:  597124 
> Change-Id: I96b88de4a41615ff9a7b77c7e40b3b317cfab57b
> Reviewed-on: https://chromium-review.googlesource.com/548615
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Commit-Queue: Noel Gordon <noel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#483181}

TBR=thestig@chromium.org,noel@chromium.org,sammc@chromium.org,rsesek@chromium.org

Change-Id: Ife28bf03e1f9ff78cdbd97bae420ce1ba297167e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  597124 
Reviewed-on: https://chromium-review.googlesource.com/553427
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483198}
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/browser/media_galleries/fileapi/safe_iapps_library_parser.cc
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/browser/media_galleries/fileapi/safe_picasa_album_table_reader.cc
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/browser/printing/pwg_raster_converter.cc
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/browser/safe_browsing/sandboxed_zip_analyzer.h
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/BUILD.gn
[rename] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/chrome_utility_messages.h
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/common_message_generator.h
[rename] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/safe_archive_analyzer.mojom
[rename] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/safe_archive_analyzer.typemap
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/safe_browsing/BUILD.gn
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/common/safe_browsing/OWNERS
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/typemaps.gni
[modify] https://crrev.com/143e13bc7c099d7b244aaae23adb4aade2e83282/chrome/utility/chrome_content_utility_client.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Jun 28 2017

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

commit 575c78d71e1d4a3d878807bf4fbde337cff18f97
Author: Michael Wasserman <msw@chromium.org>
Date: Wed Jun 28 23:44:53 2017

Revert "Revert "Rename chrome/common/chrome_utility_messages.h""

This reverts commit 143e13bc7c099d7b244aaae23adb4aade2e83282.

Reason for revert: Re-landing the original change - revert not needed?

Original change's description:
> Revert "Rename chrome/common/chrome_utility_messages.h"
> 
> This reverts commit d4c0b80def96186ea0662090ea9c54f22ee5c4cd.
> 
> Reason for revert: Looks like this caused a Mac compile failure on the main waterfall:
> https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium%2FMac%2F29509%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout
> 
> Original change's description:
> > Rename chrome/common/chrome_utility_messages.h
> > 
> > This file has no IPC messages anymore: it only defines param traits
> > specific to the safe browsing feature.
> > 
> > Move the file to the safe_browsing directory, along with its mojo and
> > typemap files, rename the file, update BUILD files.
> > 
> > Bug:  597124 
> > Change-Id: I96b88de4a41615ff9a7b77c7e40b3b317cfab57b
> > Reviewed-on: https://chromium-review.googlesource.com/548615
> > Reviewed-by: Sam McNally <sammc@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Commit-Queue: Noel Gordon <noel@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#483181}
> 
> TBR=thestig@chromium.org,noel@chromium.org,sammc@chromium.org,rsesek@chromium.org
> 
> Change-Id: Ife28bf03e1f9ff78cdbd97bae420ce1ba297167e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  597124 
> Reviewed-on: https://chromium-review.googlesource.com/553427
> Commit-Queue: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#483198}

TBR=msw@chromium.org,thestig@chromium.org,noel@chromium.org,sammc@chromium.org,rsesek@chromium.org,msw@google.com

Change-Id: I933de5b8b3aacb5beefe0a061ba0e00c4f299f30
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  597124 
Reviewed-on: https://chromium-review.googlesource.com/553428
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483201}
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/browser/media_galleries/fileapi/safe_iapps_library_parser.cc
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/browser/media_galleries/fileapi/safe_picasa_album_table_reader.cc
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/browser/printing/pdf_to_emf_converter.cc
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/browser/printing/pwg_raster_converter.cc
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/browser/safe_browsing/sandboxed_zip_analyzer.h
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/BUILD.gn
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/common_message_generator.h
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/safe_browsing/BUILD.gn
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/safe_browsing/OWNERS
[rename] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/safe_browsing/safe_archive_analyzer.mojom
[rename] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/safe_browsing/safe_archive_analyzer.typemap
[rename] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/common/safe_browsing/safe_archive_analyzer_param_traits.h
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/typemaps.gni
[modify] https://crrev.com/575c78d71e1d4a3d878807bf4fbde337cff18f97/chrome/utility/chrome_content_utility_client.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Jun 29 2017

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

commit dc6282f894f97ee424f62dc1ca3235856ed1e43b
Author: Noel Gordon <noel@chromium.org>
Date: Thu Jun 29 07:24:01 2017

Revert "Revert "Rename chrome/common/chrome_utility_messages.h""

This reverts commit 143e13bc7c099d7b244aaae23adb4aade2e83282.

Reason for revert:  No need to revert the original change.  Actual problem is a known issue  http://crbug.com/696529  where the compile
goes green on subsequent runs.

Original change's description:
> Revert "Rename chrome/common/chrome_utility_messages.h"
> 
> This reverts commit d4c0b80def96186ea0662090ea9c54f22ee5c4cd.
> 
> Reason for revert: Looks like this caused a Mac compile failure on the main waterfall:
> https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium%2FMac%2F29509%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout
> 
> Original change's description:
> > Rename chrome/common/chrome_utility_messages.h
> > 
> > This file has no IPC messages anymore: it only defines param traits
> > specific to the safe browsing feature.
> > 
> > Move the file to the safe_browsing directory, along with its mojo and
> > typemap files, rename the file, update BUILD files.
> > 
> > Bug:  597124 
> > Change-Id: I96b88de4a41615ff9a7b77c7e40b3b317cfab57b
> > Reviewed-on: https://chromium-review.googlesource.com/548615
> > Reviewed-by: Sam McNally <sammc@chromium.org>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Commit-Queue: Noel Gordon <noel@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#483181}
> 
> TBR=thestig@chromium.org,noel@chromium.org,sammc@chromium.org,rsesek@chromium.org
> 
> Change-Id: Ife28bf03e1f9ff78cdbd97bae420ce1ba297167e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  597124 
> Reviewed-on: https://chromium-review.googlesource.com/553427
> Commit-Queue: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#483198}

TBR=msw@chromium.org,thestig@chromium.org,noel@chromium.org,sammc@chromium.org,rsesek@chromium.org,msw@google.com

Change-Id: If25b9b3c6e45a709cbbfff916bae3efcff3d1c92
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  597124 
Reviewed-on: https://chromium-review.googlesource.com/554632
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483314}

Comment 40 by noel@chromium.org, Jul 1 2017

Blockedon: 738648

Comment 41 by noel@chromium.org, Jul 1 2017

Status: Started (was: Fixed)
Noticed content/common/utility_messages.h has IPC related to utility process batch mode, we should probably get rid of that too,  issue 738648 .

Comment 42 by noel@chromium.org, Jul 7 2017

Status: Fixed (was: Started)

Sign in to add a comment