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

Issue 645203002: Block NPAPI plugins by default (Closed)

Created:
6 years, 2 months ago by Will Harris
Modified:
5 years, 11 months ago
Reviewers:
komoroske, jam, Ilya Sherman, sky
CC:
darin-cc_chromium.org, jam, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Block NPAPI plugins by default Add --enable-npapi flag in chrome://flags to support re-enable. BUG=295137 TEST=browser_tests, unit_tests, content_browsertests, blink layout tests Committed: https://crrev.com/0539f2f5214fc17fdd5d1964be0ceedf98fdb9ce Cr-Commit-Position: refs/heads/master@{#312374}

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : fix flags definition #

Patch Set 4 : fix grd #

Patch Set 5 : fix tests except UnifiedPepperFlashState #

Patch Set 6 : rebase #

Patch Set 7 : fix PluginPrefsTest.UnifiedPepperFlashState #

Patch Set 8 : two new tests to verify flag #

Patch Set 9 : content_shell needs npapi for webkit tests #

Patch Set 10 : missing test file #

Total comments: 4

Patch Set 11 : make npapi plugin load tests more robust. remove ifdefs #

Total comments: 1

Patch Set 12 : remove ifdef. remove tab. #

Patch Set 13 : reorder initializers for clang #

Total comments: 10

Patch Set 14 : remove more ifdefs #

Patch Set 15 : rebase and virtual/override fixes #

Patch Set 16 : rebase #

Patch Set 17 : add NPAPI UMA histogram #

Total comments: 2

Patch Set 18 : move UMA outside ifdef #

Unified diffs Side-by-side diffs Delta from patch set Stats (+230 lines, -8 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/content_settings_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +66 lines, -2 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/prerender/prerender_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
A chrome/test/data/load_npapi_plugin.html View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +31 lines, -0 lines 0 comments Download
M content/browser/plugin_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/plugin_service_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +3 lines, -0 lines 0 comments Download
M content/browser/plugin_service_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +25 lines, -6 lines 0 comments Download
M content/public/browser/plugin_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +3 lines, -0 lines 0 comments Download
M content/shell/app/shell_main_delegate.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M content/test/fake_plugin_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M content/test/fake_plugin_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M content/test/plugin/plugin_client.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +53 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 3 chunks +17 lines, -0 lines 0 comments Download

Messages

Total messages: 35 (13 generated)
Will Harris
PTAL komoroske for sanity check on strings.
6 years, 2 months ago (2014-10-10 22:09:58 UTC) #3
Will Harris
looks like there's quite a few tests I need to fix up still.
6 years, 2 months ago (2014-10-13 03:47:52 UTC) #6
Will Harris
jam@ PTAL - This appears to be passing on all the tests I can find, ...
6 years, 2 months ago (2014-10-16 15:58:15 UTC) #7
Will Harris
sky@ can you take a look at content/browser, where the main changes are made, most ...
6 years, 2 months ago (2014-10-16 18:45:45 UTC) #9
sky
John is an owner for all this code, he should review.
6 years, 2 months ago (2014-10-16 19:53:09 UTC) #10
jam
https://codereview.chromium.org/645203002/diff/780001/chrome/test/data/load_npapi_plugin.html File chrome/test/data/load_npapi_plugin.html (right): https://codereview.chromium.org/645203002/diff/780001/chrome/test/data/load_npapi_plugin.html#newcode21 chrome/test/data/load_npapi_plugin.html:21: setTimeout(function(){notloaded()}, 2000); this seems flaky? what if it takes ...
6 years, 2 months ago (2014-10-16 22:10:57 UTC) #11
Will Harris
PTAL https://codereview.chromium.org/645203002/diff/780001/chrome/test/data/load_npapi_plugin.html File chrome/test/data/load_npapi_plugin.html (right): https://codereview.chromium.org/645203002/diff/780001/chrome/test/data/load_npapi_plugin.html#newcode21 chrome/test/data/load_npapi_plugin.html:21: setTimeout(function(){notloaded()}, 2000); On 2014/10/16 22:10:57, jam wrote: > ...
6 years, 2 months ago (2014-10-17 18:57:09 UTC) #12
jam
lgtm https://codereview.chromium.org/645203002/diff/840001/chrome/browser/content_settings/content_settings_browsertest.cc File chrome/browser/content_settings/content_settings_browsertest.cc (right): https://codereview.chromium.org/645203002/diff/840001/chrome/browser/content_settings/content_settings_browsertest.cc#newcode328 chrome/browser/content_settings/content_settings_browsertest.cc:328: #if defined(OS_MACOSX) || defined(OS_WIN) ditto https://codereview.chromium.org/645203002/diff/840001/chrome/browser/plugins/plugin_prefs_unittest.cc File chrome/browser/plugins/plugin_prefs_unittest.cc ...
6 years, 2 months ago (2014-10-20 15:50:32 UTC) #13
Will Harris
+isherman@ for OWNERS on histograms.xml https://codereview.chromium.org/645203002/diff/840001/chrome/browser/content_settings/content_settings_browsertest.cc File chrome/browser/content_settings/content_settings_browsertest.cc (right): https://codereview.chromium.org/645203002/diff/840001/chrome/browser/content_settings/content_settings_browsertest.cc#newcode328 chrome/browser/content_settings/content_settings_browsertest.cc:328: #if defined(OS_MACOSX) || defined(OS_WIN) ...
6 years, 2 months ago (2014-10-20 17:59:56 UTC) #15
Ilya Sherman
histograms.xml lgtm
6 years, 2 months ago (2014-10-21 01:34:57 UTC) #16
jschuh
Just a reminder that we want to have UMA data for what users run Chrome ...
6 years ago (2014-12-09 17:10:50 UTC) #17
Will Harris
Rebased, and added the NPAPI histogram. isherman@ can you take a quick look over please? ...
5 years, 11 months ago (2015-01-20 02:45:12 UTC) #20
Ilya Sherman
Histograms LGTM % a nit: https://codereview.chromium.org/645203002/diff/960001/content/browser/plugin_service_impl.cc File content/browser/plugin_service_impl.cc (right): https://codereview.chromium.org/645203002/diff/960001/content/browser/plugin_service_impl.cc#newcode201 content/browser/plugin_service_impl.cc:201: NPAPI_STATUS_ENUM_COUNT); nit: To reduce ...
5 years, 11 months ago (2015-01-20 20:45:18 UTC) #21
Will Harris
thanks for all the reviews. https://codereview.chromium.org/645203002/diff/960001/content/browser/plugin_service_impl.cc File content/browser/plugin_service_impl.cc (right): https://codereview.chromium.org/645203002/diff/960001/content/browser/plugin_service_impl.cc#newcode201 content/browser/plugin_service_impl.cc:201: NPAPI_STATUS_ENUM_COUNT); On 2015/01/20 20:45:18, ...
5 years, 11 months ago (2015-01-20 22:35:12 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/645203002/980001
5 years, 11 months ago (2015-01-20 22:36:11 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/25125)
5 years, 11 months ago (2015-01-21 00:08:19 UTC) #26
Will Harris
On 2015/01/21 00:08:19, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
5 years, 11 months ago (2015-01-21 01:07:56 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/645203002/980001
5 years, 11 months ago (2015-01-21 01:14:06 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/25125)
5 years, 11 months ago (2015-01-21 01:47:55 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/645203002/980001
5 years, 11 months ago (2015-01-21 03:19:01 UTC) #33
commit-bot: I haz the power
Committed patchset #18 (id:980001)
5 years, 11 months ago (2015-01-21 14:37:16 UTC) #34
commit-bot: I haz the power
5 years, 11 months ago (2015-01-21 14:38:53 UTC) #35
Message was sent while issue was closed.
Patchset 18 (id:??) landed as
https://crrev.com/0539f2f5214fc17fdd5d1964be0ceedf98fdb9ce
Cr-Commit-Position: refs/heads/master@{#312374}

Powered by Google App Engine
This is Rietveld 408576698