Check for default content setting pattern when requiring user authorization for plug-ins.

Check for default content setting pattern when requiring user authorization for plug-ins.

We now check in the renderer if a plug-in is allowed by default, which is the case if the matching patterns are wildcards.

BUG=100581
TEST=see bug


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106270

[Bernhard Bauer, 2011-10-18 16:43:40]

Please review.

Marja: This also implements serializing ContentSettingPatterns. I'm happy to
discuss the right way to do that ;-)

[marja, 2011-10-19 07:44:31]

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:296: // For simple
content types, the DefaultProvider should always return
Why was this DCHECK removed, isn't it valid still?

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:206:
scoped_ptr<base::Value> setting;
Would it make sense to call this "value" instead of "setting"?

http://codereview.chromium.org/8334020/diff/3001/chrome/renderer/content_sett...
File chrome/renderer/content_settings_observer.cc (right):

http://codereview.chromium.org/8334020/diff/3001/chrome/renderer/content_sett...
chrome/renderer/content_settings_observer.cc:81:
DCHECK_NE(CONTENT_SETTINGS_TYPE_PLUGINS, type);
The tests still seem to call this for plug-ins.

[Bernhard Bauer, 2011-10-19 09:53:50]

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:296: // For simple
content types, the DefaultProvider should always return
On 2011/10/19 07:44:31, marja wrote:
> Why was this DCHECK removed, isn't it valid still?

When looking up content settings with a non-empty resource identifier, it's
possible that we don't find a content setting. |GetContentSetting| did not have
that check.

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:206:
scoped_ptr<base::Value> setting;
On 2011/10/19 07:44:31, marja wrote:
> Would it make sense to call this "value" instead of "setting"?

Done.

http://codereview.chromium.org/8334020/diff/3001/chrome/renderer/content_sett...
File chrome/renderer/content_settings_observer.cc (right):

http://codereview.chromium.org/8334020/diff/3001/chrome/renderer/content_sett...
chrome/renderer/content_settings_observer.cc:81:
DCHECK_NE(CONTENT_SETTINGS_TYPE_PLUGINS, type);
On 2011/10/19 07:44:31, marja wrote:
> The tests still seem to call this for plug-ins.

Fixed.

[marja, 2011-10-19 10:23:16]

LGTM.

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (left):

http://codereview.chromium.org/8334020/diff/3001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:296: // For simple
content types, the DefaultProvider should always return
On 2011/10/19 09:53:50, Bernhard Bauer wrote:
> On 2011/10/19 07:44:31, marja wrote:
> > Why was this DCHECK removed, isn't it valid still?
> 
> When looking up content settings with a non-empty resource identifier, it's
> possible that we don't find a content setting. |GetContentSetting| did not
have
> that check.

Ok.

[commit-bot: I haz the power, 2011-10-19 10:27:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/8334020/6002

[commit-bot: I haz the power, 2011-10-19 10:29:02]

Try job failure for 8334020-6002 on mac_rel for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2011-10-19 11:04:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/8334020/6002

[commit-bot: I haz the power, 2011-10-19 12:59:22]

Change committed as 106270