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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 19518006: Remove the two years old code to enable the nacl plugin once if it's disabled. This was added to de… (Closed)

Created:
7 years, 5 months ago by jam
Modified:
7 years, 5 months ago
Reviewers:
Bernhard Bauer
CC:
chromium-reviews, stuartmorgan+watch_chromium.org, jam
Visibility:
Public.

Description

Remove the two years old code to enable the nacl plugin once if it's disabled. This was added to deal with old profiles when we had the plugin disabled by default. There have been 15 releases in the meantime, which is enough chances to update this pref. This is in preparation for moving webkit/plugins to content/. BUG=237249 R=bauerb@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=212104

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -111 lines) Patch
M chrome/browser/plugins/plugin_prefs.h View 5 chunks +0 lines, -17 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs.cc View 10 chunks +3 lines, -71 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs_factory.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/plugins/plugin_prefs_unittest.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/common/pref_names.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/pref_names.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M content/browser/plugin_loader_posix.cc View 1 2 2 chunks +3 lines, -4 lines 4 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 1 chunk +0 lines, -4 lines 0 comments Download
M content/public/browser/plugin_service.h View 1 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
jam
7 years, 5 months ago (2013-07-17 15:31:16 UTC) #1
Bernhard Bauer
https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_loader_posix.cc File content/browser/plugin_loader_posix.cc (right): https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_loader_posix.cc#newcode78 content/browser/plugin_loader_posix.cc:78: webkit::npapi::PluginList::Singleton()->GetPluginPathsToLoad( Is this related to this change?
7 years, 5 months ago (2013-07-17 15:59:51 UTC) #2
jam
https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_loader_posix.cc File content/browser/plugin_loader_posix.cc (right): https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_loader_posix.cc#newcode78 content/browser/plugin_loader_posix.cc:78: webkit::npapi::PluginList::Singleton()->GetPluginPathsToLoad( On 2013/07/17 15:59:52, Bernhard Bauer wrote: > Is ...
7 years, 5 months ago (2013-07-17 16:02:16 UTC) #3
Bernhard Bauer
LGTM https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_loader_posix.cc File content/browser/plugin_loader_posix.cc (right): https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_loader_posix.cc#newcode78 content/browser/plugin_loader_posix.cc:78: webkit::npapi::PluginList::Singleton()->GetPluginPathsToLoad( On 2013/07/17 16:02:16, jam wrote: > On ...
7 years, 5 months ago (2013-07-17 16:06:33 UTC) #4
jam
7 years, 5 months ago (2013-07-17 16:10:42 UTC) #5 
https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_lo...
File content/browser/plugin_loader_posix.cc (right):

https://codereview.chromium.org/19518006/diff/27001/content/browser/plugin_lo...
content/browser/plugin_loader_posix.cc:78:
webkit::npapi::PluginList::Singleton()->GetPluginPathsToLoad(
On 2013/07/17 16:06:33, Bernhard Bauer wrote:
> On 2013/07/17 16:02:16, jam wrote:
> > On 2013/07/17 15:59:52, Bernhard Bauer wrote:
> > > Is this related to this change?
> > 
> > yep, I removed PluginService::GetPluginList since it was only there for
> > PluginPrefs
> 
> But... it's used by this class, no? :)
> 
> The least-intrusive change would be to only remove it from the PluginService
> interface, but leave it in PluginServiceImpl, but this way is fine with me
too.

My plan is to remove MockPluginList since most of its users are in chrome and I
want to hide PluginList altogether. I will introduce another way to do the same
thing through PluginService. At that point, there'll be no plugin_list_ member
in PluginServiceImpl. So I was doing this change in anticipation of what the
final code will look like.

Powered by Google App Engine
This is Rietveld 408576698