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

Issue 6242013: Add LegacyContentSettingsProvider Interface. (Closed)

Created:
9 years, 11 months ago by markusheintz_
Modified:
9 years, 6 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Add ContentSettingsProvider Interface. BUG=TBA TEST=TAB

Patch Set 1 #

Patch Set 2 : " #

Total comments: 10

Patch Set 3 : " #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -0 lines) Patch
M chrome/browser/content_settings/content_settings_provider.h View 1 2 2 chunks +99 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
markusheintz_
This would be a real legacy interface that should allow copying code from the HostContentSettingsMap ...
9 years, 11 months ago (2011-01-26 14:21:04 UTC) #1
Bernhard Bauer
http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_settings/content_settings_provider.h File chrome/browser/content_settings/content_settings_provider.h (right): http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_settings/content_settings_provider.h#newcode15 chrome/browser/content_settings/content_settings_provider.h:15: #include "googleurl/src/gurl.h" Forward-declare GURL? http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_settings/content_settings_provider.h#newcode48 chrome/browser/content_settings/content_settings_provider.h:48: Remove blank line. ...
9 years, 11 months ago (2011-01-26 14:50:47 UTC) #2
markusheintz_
9 years, 11 months ago (2011-01-26 16:27:47 UTC) #3
Fixed nits and lints. Please wait with any further review on this. Unit any more
new code appears here.

http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/content_settings_provider.h (right):

http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/content_settings_provider.h:15: #include
"googleurl/src/gurl.h"
On 2011/01/26 14:50:47, Bernhard Bauer wrote:
> Forward-declare GURL?

Done.

http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/content_settings_provider.h:48: 
On 2011/01/26 14:50:47, Bernhard Bauer wrote:
> Remove blank line.
> 
> Also, you should add a virtual destructor.

Done.

http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/content_settings_provider.h:89: virtual
ContentSettings GetNonDefaultContentSettings(const GURL& url) const = 0;
On 2011/01/26 14:50:47, Bernhard Bauer wrote:
> 80 cols

Done.

http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/content_settings_provider.h:100: const
std::string& resource_identifier,
On 2011/01/26 14:50:47, Bernhard Bauer wrote:
> Methinks your editor is indent-technically challenged. Align parameters plz.

Done.

http://codereview.chromium.org/6242013/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/content_settings_provider.h:135: 
On 2011/01/26 14:50:47, Bernhard Bauer wrote:
> Remove blank line

Done.

Powered by Google App Engine
This is Rietveld 408576698