New issue
Advanced search Search tips

Issue 921746 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Move InspectModule() usage out of process

Project Member Reported by pmonette@chromium.org, Jan 14

Issue description

Module inspection requires a few crypto DLLs that remain in the process space long after they are no longer needed. Moving them out of process and killing the process when it's not needed will reduce the browser process memory usage.

In addition, it will reduce jank caused by loading DLLs on background threads. See https://crbug.com/914067
 
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit 6317ae71ab8112523c84080d871485276dd2f536
Author: Patrick Monette <pmonette@chromium.org>
Date: Wed Jan 16 21:29:10 2019

Don't call CollapseMatchingPrefixInPath() in InspectModule()

As InspectModule() will be moved out-of-process in a future CL, the
goal of this change is to ensure InspectModule() doesn't need the
StringMapping of environment variables. This will avoid copying
the mapping across processes every time InspectModule() is called.

Since CollapseMatchingPrefixInPath() still needs to be done on the
inspected module, this responsability is now held by the
ModuleInspector class.

Bug: 921746
Change-Id: Ic06336d6b18ba278be9fe37a2c13414705644b31
Reviewed-on: https://chromium-review.googlesource.com/c/1409819
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623375}
[modify] https://crrev.com/6317ae71ab8112523c84080d871485276dd2f536/chrome/browser/conflicts/module_blacklist_cache_updater_win_unittest.cc
[modify] https://crrev.com/6317ae71ab8112523c84080d871485276dd2f536/chrome/browser/conflicts/module_info_win.cc
[modify] https://crrev.com/6317ae71ab8112523c84080d871485276dd2f536/chrome/browser/conflicts/module_info_win.h
[modify] https://crrev.com/6317ae71ab8112523c84080d871485276dd2f536/chrome/browser/conflicts/module_info_win_unittest.cc
[modify] https://crrev.com/6317ae71ab8112523c84080d871485276dd2f536/chrome/browser/conflicts/module_inspector_win.cc
[modify] https://crrev.com/6317ae71ab8112523c84080d871485276dd2f536/chrome/browser/conflicts/module_inspector_win.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit 9fbb3180826c5ec68a200508da670446ff66081f
Author: Patrick Monette <pmonette@chromium.org>
Date: Wed Jan 16 21:56:59 2019

Don't wrap ModuleInspectionResult in a unique_ptr in InspectModule()

This CL also contains 2 other changes that makes sense now that we're
not using unique_ptr anymore:

Make ModuleInspectionResult move-only to ensure it is never copied, and
make ModuleInfoData hold its ModuleInspectionResult using a
base::Optional instead of a unique_ptr.

Bug: 921746
Change-Id: I22d7c7332497d6f7bb87ee179a284f8f3e985c74
Reviewed-on: https://chromium-review.googlesource.com/c/1413402
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623387}
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/incompatible_applications_updater_win_unittest.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_blacklist_cache_updater_win_unittest.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_database_win.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_database_win.h
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_info_win.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_info_win.h
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_info_win_unittest.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_inspector_win.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_inspector_win.h
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_inspector_win_unittest.cc
[modify] https://crrev.com/9fbb3180826c5ec68a200508da670446ff66081f/chrome/browser/conflicts/module_list_filter_win_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16 (6 days ago)

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

commit 1358050918d791ba221001c7a453223d2709e81f
Author: Patrick Monette <pmonette@chromium.org>
Date: Wed Jan 16 22:25:52 2019

Rename CertificateType to CertificateInfo::Type

This will avoid name clashing when declaring a CertificateType enum in
mojom.

Bug: 921746
Change-Id: If72e1cb6be1dc42c1f14e72d380c13265f8c505e
Reviewed-on: https://chromium-review.googlesource.com/c/1413654
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623402}
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/incompatible_applications_updater_win.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/incompatible_applications_updater_win_unittest.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/module_blacklist_cache_updater_win.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/module_blacklist_cache_updater_win_unittest.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/module_info_util_win.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/module_info_util_win.h
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/module_info_util_win_unittest.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/module_info_win_unittest.cc
[modify] https://crrev.com/1358050918d791ba221001c7a453223d2709e81f/chrome/browser/conflicts/third_party_metrics_recorder_win.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 17 (5 days ago)

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

commit d8b0755c159cf0035eac1a15268c9b14f6c185a3
Author: Patrick Monette <pmonette@chromium.org>
Date: Thu Jan 17 18:39:17 2019

Refactor the usage of AfterStartupTaskUtils in ModuleInspector

Now the AfterStartupTaskUtils class is used to get notified when startup
is finished and the ModuleInspector has the responsability of posting
the InspectModule tasks.

Bug: 921746
Change-Id: I87a4e1fc82ee3f962da1b601ebb640ad6952d0b4
Reviewed-on: https://chromium-review.googlesource.com/c/1413361
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623765}
[modify] https://crrev.com/d8b0755c159cf0035eac1a15268c9b14f6c185a3/chrome/browser/conflicts/module_database_win_unittest.cc
[modify] https://crrev.com/d8b0755c159cf0035eac1a15268c9b14f6c185a3/chrome/browser/conflicts/module_inspector_win.cc
[modify] https://crrev.com/d8b0755c159cf0035eac1a15268c9b14f6c185a3/chrome/browser/conflicts/module_inspector_win.h

Project Member

Comment 6 by bugdroid1@chromium.org, Yesterday (36 hours ago)

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

commit afe350c66cab481750b9623fb1b8540a7d23148d
Author: Patrick Monette <pmonette@chromium.org>
Date: Mon Jan 21 17:59:35 2019

Rename the ShellUtilWin service to UtilWin

As I plan to add other utilities to this interface that doesn't relate
to the shell, the previous name is no longer fitting.

Bug: 921746
Change-Id: Ie61f3378315eefdc3751aac895c3c362fc95d4ce
Reviewed-on: https://chromium-review.googlesource.com/c/1416270
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Auto-Submit: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624609}
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/app/chrome_content_browser_overlay_manifest.cc
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/browser/shell_integration_win.cc
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/browser/win/chrome_select_file_dialog_factory.cc
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/BUILD.gn
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/manifest.json
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/public/mojom/BUILD.gn
[rename] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/public/mojom/util_win.mojom
[rename] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/public/mojom/util_win.typemap
[rename] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/public/mojom/util_win_mojom_traits.cc
[rename] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/public/mojom/util_win_mojom_traits.h
[rename] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/util_win_impl.cc
[rename] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/util_win_impl.h
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/util_win_service.cc
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/services/util_win/util_win_service.h
[modify] https://crrev.com/afe350c66cab481750b9623fb1b8540a7d23148d/chrome/typemaps.gni

Project Member

Comment 7 by bugdroid1@chromium.org, Yesterday (36 hours ago)

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

commit 0ec491f17f72fd16ea90db2103f8fc52eade12a1
Author: Patrick Monette <pmonette@chromium.org>
Date: Mon Jan 21 18:17:39 2019

Move chrome/browser/conflicts struct declarations to their own target

This will allow both "//chrome/browser/BUILD.gn" and
"//chrome/services/util_win/BUILD.gn" to depend on module_info_win.h.

Bug: 921746
Change-Id: I12e7b1c083f9b6e0cf8f83c2667762d74f7f3414
Reviewed-on: https://chromium-review.googlesource.com/c/1416330
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624629}
[modify] https://crrev.com/0ec491f17f72fd16ea90db2103f8fc52eade12a1/chrome/browser/BUILD.gn
[add] https://crrev.com/0ec491f17f72fd16ea90db2103f8fc52eade12a1/chrome/browser/conflicts/BUILD.gn
[modify] https://crrev.com/0ec491f17f72fd16ea90db2103f8fc52eade12a1/chrome/browser/ui/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Yesterday (32 hours ago)

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

commit d2f2147ad9e33e06729eb0905df67445113afbf3
Author: Patrick Monette <pmonette@chromium.org>
Date: Mon Jan 21 22:16:08 2019

Add a InspectModule() function to the UtilWin interface

This function does the same as InspectModule() in
chrome/browser/conflicts/module_info_win.cc but runs out of process.

Bug: 921746
Change-Id: I6aaf919741ac253f5666f57bf199aabb5e4331b2
Reviewed-on: https://chromium-review.googlesource.com/c/1415763
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624684}
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/BUILD.gn
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/DEPS
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/public/mojom/util_win.mojom
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/public/mojom/util_win.typemap
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/public/mojom/util_win_mojom_traits.cc
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/public/mojom/util_win_mojom_traits.h
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/util_win_impl.cc
[modify] https://crrev.com/d2f2147ad9e33e06729eb0905df67445113afbf3/chrome/services/util_win/util_win_impl.h

Project Member

Comment 9 by bugdroid1@chromium.org, Today (9 hours ago)

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

commit 5a0921ab569ee10d89eca68406bb262f816de7cb
Author: Patrick Monette <pmonette@chromium.org>
Date: Tue Jan 22 20:35:29 2019

Get the CertificateInfo of the current exe from ModuleDatabase

The ThirdPartyConflictsManager needs the CertificateInfo of the current
executable to identify third-party DLLs. It used to create it itself,
but that information is already available from the ModuleDatabase.

Bug: 921746
Change-Id: I3d57d289f17bac563fed498e59f48c3bbc9f4f64
Reviewed-on: https://chromium-review.googlesource.com/c/1426061
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624901}
[modify] https://crrev.com/5a0921ab569ee10d89eca68406bb262f816de7cb/chrome/browser/conflicts/third_party_conflicts_manager_win.cc
[modify] https://crrev.com/5a0921ab569ee10d89eca68406bb262f816de7cb/chrome/browser/conflicts/third_party_conflicts_manager_win.h

Sign in to add a comment