Limiting the "Cookies and site data" form to "web safe" schemes.

Limiting the "Cookies and site data" form to valid Browsing Data schemes.

This change will prevent extension and devtools (as well as data from any other
internal `chrome-*` URL schemes) from showing up in the list of browsing data.

BUG=121441
TEST=unit_test

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=130598

[Mike West, 2012-04-03 07:32:45]

Hi Jochen!

WDYT?

[Mike West, 2012-04-03 09:16:59]

Let's try this again.

I'm not modifying the cookies helper, as cookies don't have schemes. Not sure
whether there's a reasonable test that I can do there? Any ideas?

[jochen (gone - plz use gerrit), 2012-04-03 09:41:21]

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
File chrome/browser/browsing_data_appcache_helper.cc (right):

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_appcache_helper.cc:88: if
(policy->IsWebSafeScheme(current->first.scheme()))
forgot ! ?

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
File chrome/browser/browsing_data_database_helper.cc (right):

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_database_helper.cc:17: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
nit. W > p

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_database_helper.cc:100: if
(policy->IsWebSafeScheme(origin.scheme())) {
forgot ! ?

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
File chrome/browser/browsing_data_local_storage_helper.cc (right):

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_local_storage_helper.cc:16: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
W > p

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_local_storage_helper.cc:163:
pending_local_storage_info_.insert(origin);
why no check here?

[Mike West, 2012-04-03 14:45:51]

Addressed comments, and rebased this on top of
http://codereview.chromium.org/9958111/

Can't land until that lands. :)

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
File chrome/browser/browsing_data_appcache_helper.cc (right):

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_appcache_helper.cc:88: if
(policy->IsWebSafeScheme(current->first.scheme()))
On 2012/04/03 09:41:21, jochen wrote:
> forgot ! ?

Gah.

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
File chrome/browser/browsing_data_database_helper.cc (right):

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_database_helper.cc:17: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
On 2012/04/03 09:41:21, jochen wrote:
> nit. W > p

Not according to vim! :)

Fixed.

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_database_helper.cc:100: if
(policy->IsWebSafeScheme(origin.scheme())) {
On 2012/04/03 09:41:21, jochen wrote:
> forgot ! ?

Done.

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
File chrome/browser/browsing_data_local_storage_helper.cc (right):

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_local_storage_helper.cc:16: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
On 2012/04/03 09:41:21, jochen wrote:
> W > p

Done.

https://chromiumcodereview.appspot.com/9958107/diff/2002/chrome/browser/brows...
chrome/browser/browsing_data_local_storage_helper.cc:163:
pending_local_storage_info_.insert(origin);
On 2012/04/03 09:41:21, jochen wrote:
> why no check here?

Because I didn't see it. You'll note also the lack of a test. Bleh.

Fixed both.

[jochen (gone - plz use gerrit), 2012-04-03 14:56:05]

lgtm

[commit-bot: I haz the power, 2012-04-04 08:59:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkwst@chromium.org/9958107/7

[commit-bot: I haz the power, 2012-04-04 11:49:08]

Try job failure for 9958107-7 (retry) on win_rel for step "browser_tests".
It's a second try, previously, steps "browser_tests, unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Use mkwst_at_chromium.org plz., 2012-04-04 12:02:14]

On 2012/04/04 11:49:08, I haz the power (commit-bot) wrote:
> Try job failure for 9958107-7 (retry) on win_rel for step "browser_tests".
> It's a second try, previously, steps "browser_tests, unit_tests" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

I hate the commit queue. Landing manually.