Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(200)

Issue 1158423003: Disable NPAPI support (Closed)

Created:
5 years, 6 months ago by Will Harris
Modified:
5 years, 6 months ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, jam, darin-cc_chromium.org, markusheintz_, mkwst+moarreviews-shell_chromium.org, jochen+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Disable NPAPI support by removing the --enable-npapi switch, chrome://flags and enterprise policy override. --enable-npapi-for-testing will remain temporarily until the rest of the NPAPI code and tests are removed. This is a selective revert of 0539f2f5214fc17fdd5d1964be0ceedf98fdb9ce and 26d09db19381c2236a2890c045761465c78bc958. BUG=493212, 489066 Committed: https://crrev.com/cfdd80f3d2a845a2267dd9ffe81011609f74eb0c Cr-Commit-Position: refs/heads/master@{#332006}

Patch Set 1 : remove switch and flags based on revert of 0539f2f5 #

Patch Set 2 : remove enterprise policy override based on revert of 26d09db #

Total comments: 5

Patch Set 3 : remove unneeded function declaration #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -170 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/content_settings/content_settings_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
D chrome/browser/plugins/enable_npapi_plugins_policy_handler.h View 1 1 chunk +0 lines, -37 lines 0 comments Download
D chrome/browser/plugins/enable_npapi_plugins_policy_handler.cc View 1 1 chunk +0 lines, -46 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs.h View 1 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs.cc View 1 3 chunks +0 lines, -11 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs_factory.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/policy/configuration_policy_handler_list_factory.cc View 1 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/common/pref_names.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/pref_names.cc View 1 1 chunk +0 lines, -3 lines 0 comments Download
M content/browser/plugin_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/plugin_service_impl.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/plugin_service_impl.cc View 1 2 chunks +2 lines, -16 lines 0 comments Download
M content/public/browser/plugin_service.h View 1 1 chunk +0 lines, -3 lines 0 comments Download
M content/public/common/content_switches.h View 1 chunk +1 line, -1 line 0 comments Download
M content/public/common/content_switches.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/shell/app/shell_main_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/test/fake_plugin_service.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M content/test/fake_plugin_service.cc View 1 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 15 (4 generated)
Will Harris
PTAL Changelist looks scary, but it's really just two reverts, leaving a few files in ...
5 years, 6 months ago (2015-05-28 18:46:43 UTC) #3
jam
https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/content_settings/content_settings_browsertest.cc File chrome/browser/content_settings/content_settings_browsertest.cc (right): https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/content_settings/content_settings_browsertest.cc#newcode373 chrome/browser/content_settings/content_settings_browsertest.cc:373: command_line->AppendSwitch(switches::kEnableNpapiForTesting); just delete these two tests https://codereview.chromium.org/1158423003/diff/20001/content/browser/plugin_browsertest.cc File content/browser/plugin_browsertest.cc ...
5 years, 6 months ago (2015-05-28 19:22:38 UTC) #4
Will Harris
On 2015/05/28 19:22:38, jam wrote: > https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/content_settings/content_settings_browsertest.cc > File chrome/browser/content_settings/content_settings_browsertest.cc (right): > > https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/content_settings/content_settings_browsertest.cc#newcode373 > ...
5 years, 6 months ago (2015-05-28 19:34:24 UTC) #5
jam
lgtm
5 years, 6 months ago (2015-05-28 19:59:35 UTC) #6
Will Harris
On 2015/05/28 19:59:35, jam wrote: > lgtm Context as we spoke offline: the --enable-npapi-for-testing flag ...
5 years, 6 months ago (2015-05-28 20:00:52 UTC) #7
Mattias Nissler (ping if slow)
policy changes LGTM after removing the stray declaration https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/policy/chrome_browser_policy_connector.h File chrome/browser/policy/chrome_browser_policy_connector.h (right): https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/policy/chrome_browser_policy_connector.h#newcode43 chrome/browser/policy/chrome_browser_policy_connector.h:43: void ...
5 years, 6 months ago (2015-05-29 07:10:02 UTC) #8
Bernhard Bauer
I don't think you need it from me, but LGTM.
5 years, 6 months ago (2015-05-29 09:24:03 UTC) #9
Will Harris
thanks for the reviews. https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/policy/chrome_browser_policy_connector.h File chrome/browser/policy/chrome_browser_policy_connector.h (right): https://codereview.chromium.org/1158423003/diff/20001/chrome/browser/policy/chrome_browser_policy_connector.h#newcode43 chrome/browser/policy/chrome_browser_policy_connector.h:43: void AppendExtraFlagsPerPolicy(); On 2015/05/29 07:10:02, ...
5 years, 6 months ago (2015-05-29 16:54:57 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1158423003/40001
5 years, 6 months ago (2015-05-29 16:55:43 UTC) #13
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 6 months ago (2015-05-29 17:56:37 UTC) #14
commit-bot: I haz the power
5 years, 6 months ago (2015-05-29 17:57:36 UTC) #15
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/cfdd80f3d2a845a2267dd9ffe81011609f74eb0c
Cr-Commit-Position: refs/heads/master@{#332006}

Powered by Google App Engine
This is Rietveld 408576698