New issue
Advanced search Search tips

Issue 822399 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Feature

Blocking:
issue 717696



Sign in to add a comment

Wire-up the ThirdPartyBlocking group policy

Project Member Reported by pmonette@chromium.org, Mar 15 2018

Issue description

chrisha@chromium.org added a pref to control the blocking of third-party
injection via group policy. This was before the feature was implemented so it didn't control
anything yet.

It is time to make use of that pref.

 
Project Member

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

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

commit a54726ff975e09efe95d241091a32954bdd58468
Author: Patrick Monette <pmonette@chromium.org>
Date: Tue Mar 20 18:07:37 2018

Wire-up the ThirdPartyBlocking group policy

To make this happen, the ProblematicProgramsUpdater class needed a
refactoring. This is because we do not support dynamic refresh of the
group policy, so its value needed to be cached somehow (as it can still
change during runtime).

Now, the presence of the ThirdPartyConflictsManager indicates if the
feature is enabled or not.

Bug:  822399 
Change-Id: Ia9b0b5e52df2c777b343ccc3e524ee17a1ab0e57
Reviewed-on: https://chromium-review.googlesource.com/964785
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544439}
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/component_updater/third_party_module_list_component_installer_win.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/module_database_win.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/module_database_win.h
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/module_event_sink_impl_win_unittest.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/problematic_programs_updater_win.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/problematic_programs_updater_win.h
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/problematic_programs_updater_win_unittest.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/conflicts/third_party_conflicts_manager_win.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/common/chrome_features.cc
[modify] https://crrev.com/a54726ff975e09efe95d241091a32954bdd58468/chrome/common/chrome_features.h

Labels: Target-66 Merge-Request-66 OS-Windows
Requesting merge to m66. This CL makes it possible to disable the Incompatible Applications Warning feature via group policy for enterprise users.

Thanks!
Project Member

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

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

commit 05bc73c5307f415d77a8a5a6ba4d257f5b70ef99
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Mar 20 18:56:56 2018

Revert "Wire-up the ThirdPartyBlocking group policy"

This reverts commit a54726ff975e09efe95d241091a32954bdd58468.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 544439 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2E1NDcyNmZmOTc1ZTA5ZWZlOTVkMjQxMDkxYTMyOTU0YmRkNTg0NjgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Win/28248

Sample Failed Step: compile

Original change's description:
> Wire-up the ThirdPartyBlocking group policy
> 
> To make this happen, the ProblematicProgramsUpdater class needed a
> refactoring. This is because we do not support dynamic refresh of the
> group policy, so its value needed to be cached somehow (as it can still
> change during runtime).
> 
> Now, the presence of the ThirdPartyConflictsManager indicates if the
> feature is enabled or not.
> 
> Bug:  822399 
> Change-Id: Ia9b0b5e52df2c777b343ccc3e524ee17a1ab0e57
> Reviewed-on: https://chromium-review.googlesource.com/964785
> Commit-Queue: Patrick Monette <pmonette@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544439}

Change-Id: I0502506465a1a356d941f5baa838350a2cf3b349
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  822399 
Reviewed-on: https://chromium-review.googlesource.com/971563
Cr-Commit-Position: refs/heads/master@{#544460}
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/component_updater/third_party_module_list_component_installer_win.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/module_database_win.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/module_database_win.h
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/module_event_sink_impl_win_unittest.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/problematic_programs_updater_win.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/problematic_programs_updater_win.h
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/problematic_programs_updater_win_unittest.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/conflicts/third_party_conflicts_manager_win.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/common/chrome_features.cc
[modify] https://crrev.com/05bc73c5307f415d77a8a5a6ba4d257f5b70ef99/chrome/common/chrome_features.h

Project Member

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

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

commit aff8f1f817acad3b3d80418cae2391f13dcf9217
Author: Patrick Monette <pmonette@chromium.org>
Date: Tue Mar 20 23:03:09 2018

Reland "Wire-up the ThirdPartyBlocking group policy"

This is a reland of a54726ff975e09efe95d241091a32954bdd58468

Original change's description:
> Wire-up the ThirdPartyBlocking group policy
>
> To make this happen, the ProblematicProgramsUpdater class needed a
> refactoring. This is because we do not support dynamic refresh of the
> group policy, so its value needed to be cached somehow (as it can still
> change during runtime).
>
> Now, the presence of the ThirdPartyConflictsManager indicates if the
> feature is enabled or not.
>
> Bug:  822399 
> Change-Id: Ia9b0b5e52df2c777b343ccc3e524ee17a1ab0e57
> Reviewed-on: https://chromium-review.googlesource.com/964785
> Commit-Queue: Patrick Monette <pmonette@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544439}

TBR: jochen@chromium.org, grt@chromium.org
Bug:  822399 
Change-Id: Ic9236dc69b3c89d882894c24bd7d172f110dc27a
Reviewed-on: https://chromium-review.googlesource.com/971921
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544579}
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/component_updater/third_party_module_list_component_installer_win.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/module_database_win.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/module_database_win.h
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/module_event_sink_impl_win_unittest.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/problematic_programs_updater_win.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/problematic_programs_updater_win.h
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/problematic_programs_updater_win_unittest.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/conflicts/third_party_conflicts_manager_win.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/common/chrome_features.cc
[modify] https://crrev.com/aff8f1f817acad3b3d80418cae2391f13dcf9217/chrome/common/chrome_features.h

Project Member

Comment 5 by sheriffbot@chromium.org, Mar 21 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Why is this urgent for M66? Why can it now wait until 67?
Hi owners,

This CL is needed in m66 because we're committed externally to launching the Incompatible Applications feature in m66, and this is an important bug fix for that feature.

https://blog.chromium.org/2017/11/reducing-chrome-crashes-caused-by-third.html
This is a fairly large change. Can you please confirm if this is well tested and safe merge overall?
Hi Abdul,

This change is smaller than it looks like since it is moving code around a little.

The code has been in canary for a week without crashes, I think it is very safe to merge.
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66. Branch:3359
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 26 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0474b3aa11b6f14d8c529142613934fb32ad58d5

commit 0474b3aa11b6f14d8c529142613934fb32ad58d5
Author: Patrick Monette <pmonette@chromium.org>
Date: Mon Mar 26 18:29:22 2018

Reland "Wire-up the ThirdPartyBlocking group policy"

This is a reland of a54726ff975e09efe95d241091a32954bdd58468

Original change's description:
> Wire-up the ThirdPartyBlocking group policy
>
> To make this happen, the ProblematicProgramsUpdater class needed a
> refactoring. This is because we do not support dynamic refresh of the
> group policy, so its value needed to be cached somehow (as it can still
> change during runtime).
>
> Now, the presence of the ThirdPartyConflictsManager indicates if the
> feature is enabled or not.
>
> Bug:  822399 
> Change-Id: Ia9b0b5e52df2c777b343ccc3e524ee17a1ab0e57
> Reviewed-on: https://chromium-review.googlesource.com/964785
> Commit-Queue: Patrick Monette <pmonette@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544439}

TBR: jochen@chromium.org, grt@chromium.org
Bug:  822399 
Change-Id: Ic9236dc69b3c89d882894c24bd7d172f110dc27a
Reviewed-on: https://chromium-review.googlesource.com/971921
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#544579}(cherry picked from commit aff8f1f817acad3b3d80418cae2391f13dcf9217)
Reviewed-on: https://chromium-review.googlesource.com/981152
Cr-Commit-Position: refs/branch-heads/3359@{#441}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/component_updater/third_party_module_list_component_installer_win.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/module_database_win.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/module_database_win.h
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/module_event_sink_impl_win_unittest.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/problematic_programs_updater_win.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/problematic_programs_updater_win.h
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/problematic_programs_updater_win_unittest.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/conflicts/third_party_conflicts_manager_win.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/common/chrome_features.cc
[modify] https://crrev.com/0474b3aa11b6f14d8c529142613934fb32ad58d5/chrome/common/chrome_features.h

Status: Fixed (was: Assigned)

Sign in to add a comment