Show apps protecting local data in chrome://settings/cookies.

Show apps protecting local data in chrome://settings/cookies.

Related cleanup:
* Add a method to ExtensionSpecialStoragePolicy that returns the set of extensions protecting an origin.
* Rename CookieTreeOriginNode to CookieTreeHostNode, because that's what it is.
* Modify |origin| in CookieTreeModel::DetailedInfo to actually contain the origin.
* Remove |origin| from CocoaCookieDetails, because
  - it's a domain, not an origin, and
  - it can be inferred from the cookie information.
* Change std::wstring usage in CookieTreeModel to string16.


BUG=136487
TEST=none


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

[Bernhard Bauer, 2012-07-06 12:13:38]

Please review.

Marja: CookieTreeModel, ExtensionSpecialStoragePolicy
Evan: WebUI options

I'll add OWNERS for the rest later.

[marja, 2012-07-09 08:11:43]

I'm not a good reviewer for CookieTreeModel, I haven't authored almost any of it
and if I knew something about it, I have forgotten it...

ExtensionSpecialStoragePolicy LGTM modulo the nits:

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/cookies_tre...
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/cookies_tre...
chrome/browser/cookies_tree_model.cc:38: return false;
Hmm, didn't jochen@ just make it so that cookies are protected, too..?

(So even if cookies are set to "clear on exit", the cookies for the protected
apps are not cleared. Or is this function about something else?)

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_special_storage_policy.h (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
chrome/browser/extensions/extension_special_storage_policy.h:47: const
ExtensionSet* ExtensionsProtectingOrigin(const GURL& origin);
Caller doesn't take ownership of the returned pointer, right? Should you add a
comment about that? It might be a bit surprising, since in so many places caller
does take the ownership (not sure if the "const pointer -> ownership not
transferred" convention is followed / well-known in chromium)...

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_special_storage_policy_unittest.cc
(right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
chrome/browser/extensions/extension_special_storage_policy_unittest.cc:125: void
ExpectProtectedBy(const ExtensionSet& expected_extensions,
Nit: There's no test which would assert that something is *not* protected, so,
e.g., if there was a bug that all the URLs appear as protected, the test
wouldn't catch it.. (There is EXPECT_FALSE(IsStorageUnlimited(...)) types of
tests though.)

-> bool ProtectedBy(...) and EXPECT_TRUE(ProtectedBy(...));
EXPECT_FALSE(ProtectedBy(...)) ?

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/ui/gtk/coll...
File chrome/browser/ui/gtk/collected_cookies_gtk.cc (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/ui/gtk/coll...
chrome/browser/ui/gtk/collected_cookies_gtk.cc:226: NULL,
Why NULL here?

[Bernhard Bauer, 2012-07-09 09:50:48]

Jochen, can you take a look at CookiesTreeModel?

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/cookies_tre...
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/cookies_tre...
chrome/browser/cookies_tree_model.cc:38: return false;
On 2012/07/09 08:11:43, marja wrote:
> Hmm, didn't jochen@ just make it so that cookies are protected, too..?
> 
> (So even if cookies are set to "clear on exit", the cookies for the protected
> apps are not cleared. Or is this function about something else?)

Yeah, Clear On Exit does something different from Clear Browsing Data at the
moment :-/ It would probably make sense to reconcile the behavior, but here I'm
simply reflecting what CBD does.

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_special_storage_policy.h (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
chrome/browser/extensions/extension_special_storage_policy.h:47: const
ExtensionSet* ExtensionsProtectingOrigin(const GURL& origin);
On 2012/07/09 08:11:43, marja wrote:
> Caller doesn't take ownership of the returned pointer, right? Should you add a
> comment about that? It might be a bit surprising, since in so many places
caller
> does take the ownership (not sure if the "const pointer -> ownership not
> transferred" convention is followed / well-known in chromium)...

Hm, I usually assume that the default is that the caller doesn't take ownership,
and only add a comment if it does (and the return type isn't scoped_ptr anyway).
But a comment probably doesn't hurt.

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_special_storage_policy_unittest.cc
(right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
chrome/browser/extensions/extension_special_storage_policy_unittest.cc:125: void
ExpectProtectedBy(const ExtensionSet& expected_extensions,
On 2012/07/09 08:11:43, marja wrote:
> Nit: There's no test which would assert that something is *not* protected, so,
> e.g., if there was a bug that all the URLs appear as protected, the test
> wouldn't catch it.. (There is EXPECT_FALSE(IsStorageUnlimited(...)) types of
> tests though.)
> 
> -> bool ProtectedBy(...) and EXPECT_TRUE(ProtectedBy(...));
> EXPECT_FALSE(ProtectedBy(...)) ?

I pass in an ExtensionSet and then expect that this is *exactly* the set of
protecting extensions. I.e. if I pass in an empty set, I'm expecting the origin
not to be protected.

I'll add a comment to explain.

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/ui/gtk/coll...
File chrome/browser/ui/gtk/collected_cookies_gtk.cc (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/ui/gtk/coll...
chrome/browser/ui/gtk/collected_cookies_gtk.cc:226: NULL,
On 2012/07/09 08:11:43, marja wrote:
> Why NULL here?

At the moment we don't have UI in the collected cookies dialog to show this
information, and I'm not sure if it's worth adding some (it would probably make
more sense to show a general indication that we're in the context of an app
here).

[marja, 2012-07-09 09:55:02]

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_special_storage_policy.h (right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
chrome/browser/extensions/extension_special_storage_policy.h:47: const
ExtensionSet* ExtensionsProtectingOrigin(const GURL& origin);
On 2012/07/09 09:50:48, Bernhard Bauer wrote:
> On 2012/07/09 08:11:43, marja wrote:
> > Caller doesn't take ownership of the returned pointer, right? Should you add
a
> > comment about that? It might be a bit surprising, since in so many places
> caller
> > does take the ownership (not sure if the "const pointer -> ownership not
> > transferred" convention is followed / well-known in chromium)...
> 
> Hm, I usually assume that the default is that the caller doesn't take
ownership,
> and only add a comment if it does (and the return type isn't scoped_ptr
anyway).
> But a comment probably doesn't hurt.

Yes, true, the default might be no-ownership-transfer, since, otoh, there are so
many "Foo::GetBar() { return bar_.get();}" types of functions. Feel free to
ignore this comment. :)

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
File chrome/browser/extensions/extension_special_storage_policy_unittest.cc
(right):

http://codereview.chromium.org/10709006/diff/19001/chrome/browser/extensions/...
chrome/browser/extensions/extension_special_storage_policy_unittest.cc:125: void
ExpectProtectedBy(const ExtensionSet& expected_extensions,
On 2012/07/09 09:50:48, Bernhard Bauer wrote:
> On 2012/07/09 08:11:43, marja wrote:
> > Nit: There's no test which would assert that something is *not* protected,
so,
> > e.g., if there was a bug that all the URLs appear as protected, the test
> > wouldn't catch it.. (There is EXPECT_FALSE(IsStorageUnlimited(...)) types of
> > tests though.)
> > 
> > -> bool ProtectedBy(...) and EXPECT_TRUE(ProtectedBy(...));
> > EXPECT_FALSE(ProtectedBy(...)) ?
> 
> I pass in an ExtensionSet and then expect that this is *exactly* the set of
> protecting extensions. I.e. if I pass in an empty set, I'm expecting the
origin
> not to be protected.
> 
> I'll add a comment to explain.

Ahh, yes, I read this sloppily, it's not enough that it's protected by *some*.
Feel free to ignore this comment, too.

[jochen (gone - plz use gerrit), 2012-07-09 15:24:29]

Cramming that much cleanups into a change is very confusing :-/

I would prefer it, if you'd do the wstring -> string16 and origin -> host
changes in a separate CL

About the latter: I think the name "host" is incorrect. For stuff living on
file:// URLs, it's not the host.

I wonder what the tree will look like if you have several cookies on different
file:// URLs. The "host" nodes for those will all have the same title, but
different hosts? the host being the file url?

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/cookies_tree...
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/cookies_tree...
chrome/browser/cookies_tree_model.cc:47: return
(CanonicalizeHost(lhs->GetTitle()) <
now that the nodes are supposed to contain the actual host, you shouldn't
compare titles here

[Bernhard Bauer, 2012-07-09 15:43:46]

On 2012/07/09 15:24:29, jochen wrote:
> Cramming that much cleanups into a change is very confusing :-/
> 
> I would prefer it, if you'd do the wstring -> string16 and origin -> host
> changes in a separate CL

The wstring cleanup was to silence a presubmit warning, but I could move it to a
separate CL and rebase this one on top.

The origin -> host changes fit into this CL because I'm already changing the
meaning of the origin field in DetailedInfo, so I thought it'd be less confusing
if I'm not removing the origin field in one CL, then adding it back in another.
But maybe doing it at once *is* more confusing actually :-/

> About the latter: I think the name "host" is incorrect. For stuff living on
> file:// URLs, it's not the host.

I'd argue it's less incorrect than before ;-) I guess correct would be something
unspecific like "origin group"? I'd like to avoid that name in the code though.

> I wonder what the tree will look like if you have several cookies on different
> file:// URLs. The "host" nodes for those will all have the same title, but
> different hosts? the host being the file url?

Local data for file: URLs will all be grouped under a node with the title
"file://".

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/cookies_tree...
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/cookies_tree...
chrome/browser/cookies_tree_model.cc:47: return
(CanonicalizeHost(lhs->GetTitle()) <
On 2012/07/09 15:24:30, jochen wrote:
> now that the nodes are supposed to contain the actual host, you shouldn't
> compare titles here

Hm. The problem is that I only have generic CookieTreeNodes here, so I need
methods on CookieTreeNode, or ugly static casting. Also, as you pointed out,
it's not a host in all cases.

[Evan Stade, 2012-07-10 07:53:08]

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
File chrome/browser/resources/options2/cookies_list.js (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:225: var info = {
surely the block of code between here and L271 deserves more vertical spacing
than it has (i.e. 0 blank lines)

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:263: img.title =
loadTimeData.getString('label_protected_by_apps') +
perhaps this is part of the TODO to use a better app, but you shouldn't be
constructing strings by concatenation because that assumes all languages will be
of the form "$STRING $APPNAME", which is probably a false assumption.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:271: }
no curlies.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:469: }
\n

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:487: if (this.children.length
> 0) {
ditto in this function, needs vertical spacing.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:522:
apps.forEach(function(appInfo) {
hmm, this function looks oddly familiar

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:552: var table;
ditto in this function, needs vertical spacing.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:584: if (parent ==
this.parent)
ditto in this function, needs vertical spacing.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
File chrome/browser/ui/webui/cookies_tree_model_util.cc (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:28: const char
kKeyProtectedByApps[] = "protectedByApps";
these keys need documentation. And this key in particular needs a better name,
such as "appsThatProtect" or something, because "protectedByApps" apropos of
nothing is not very clear (sounds a lot like a boolean, or a list of apps that
this cookie "protects" somehow).

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:259: }
\n

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:264: for
(ExtensionSet::const_iterator it =
is this newline necessary?

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:273: }
\n

[Bernhard Bauer, 2012-07-10 09:50:59]

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
File chrome/browser/resources/options2/cookies_list.js (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:225: var info = {
On 2012/07/10 07:53:08, Evan Stade wrote:
> surely the block of code between here and L271 deserves more vertical spacing
> than it has (i.e. 0 blank lines)

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:263: img.title =
loadTimeData.getString('label_protected_by_apps') +
On 2012/07/10 07:53:08, Evan Stade wrote:
> perhaps this is part of the TODO to use a better app, but you shouldn't be
> constructing strings by concatenation because that assumes all languages will
be
> of the form "$STRING $APPNAME", which is probably a false assumption.

Indeed! For this CL I'm just going with the existing string though (which is
"Set by:").

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:271: }
On 2012/07/10 07:53:08, Evan Stade wrote:
> no curlies.

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:469: }
On 2012/07/10 07:53:08, Evan Stade wrote:
> \n

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:487: if (this.children.length
> 0) {
On 2012/07/10 07:53:08, Evan Stade wrote:
> ditto in this function, needs vertical spacing.

Done. I also flattened the nested if's a bit.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:522:
apps.forEach(function(appInfo) {
On 2012/07/10 07:53:08, Evan Stade wrote:
> hmm, this function looks oddly familiar

Extracted a function.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:552: var table;
On 2012/07/10 07:53:08, Evan Stade wrote:
> ditto in this function, needs vertical spacing.

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/resources/op...
chrome/browser/resources/options2/cookies_list.js:584: if (parent ==
this.parent)
On 2012/07/10 07:53:08, Evan Stade wrote:
> ditto in this function, needs vertical spacing.

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
File chrome/browser/ui/webui/cookies_tree_model_util.cc (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:28: const char
kKeyProtectedByApps[] = "protectedByApps";
On 2012/07/10 07:53:08, Evan Stade wrote:
> these keys need documentation. And this key in particular needs a better name,
> such as "appsThatProtect" or something, because "protectedByApps" apropos of
> nothing is not very clear (sounds a lot like a boolean, or a list of apps that
> this cookie "protects" somehow).

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:259: }
On 2012/07/10 07:53:08, Evan Stade wrote:
> \n

Done.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:264: for
(ExtensionSet::const_iterator it =
On 2012/07/10 07:53:08, Evan Stade wrote:
> is this newline necessary?

No. Removed.

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/ui/webui/coo...
chrome/browser/ui/webui/cookies_tree_model_util.cc:273: }
On 2012/07/10 07:53:08, Evan Stade wrote:
> \n

Done.

[jochen (gone - plz use gerrit), 2012-07-10 10:13:01]

I think splitting this up into smaller CLs would be nicer, but I don't insist on
it

lgtm with the comment adressed

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/cookies_tree...
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/10709006/diff/8058/chrome/browser/cookies_tree...
chrome/browser/cookies_tree_model.cc:47: return
(CanonicalizeHost(lhs->GetTitle()) <
I would prefer static casting for the sake of readability of the code. Invoking
CanonicalizeHost on a title feels wrong

[Evan Stade, 2012-07-11 01:45:44]

lgtm with nits

http://codereview.chromium.org/10709006/diff/10006/chrome/browser/resources/o...
File chrome/browser/resources/options2/cookies_list.js (right):

http://codereview.chromium.org/10709006/diff/10006/chrome/browser/resources/o...
chrome/browser/resources/options2/cookies_list.js:279: if (apps) {
as it turns out I don't think this check is necessary (for..in won't do anything
if !apps)

http://codereview.chromium.org/10709006/diff/10006/chrome/browser/resources/o...
chrome/browser/resources/options2/cookies_list.js:518: case 'local_storage':
you could potentially do

default:
  text = loadTimeData.getString('cookie_' + this.data.type);

if you don't think it's too hacky.

http://codereview.chromium.org/10709006/diff/10006/chrome/browser/resources/o...
chrome/browser/resources/options2/cookies_list.js:539: // Help out screen
readers and such: this is a clickable thing.
could be wrong but I think you also need to tabindex = 0

http://codereview.chromium.org/10709006/diff/10006/chrome/browser/resources/o...
chrome/browser/resources/options2/cookies_list.js:548: if (item.selectedIndex ==
index)
ternary operator

[Bernhard Bauer, 2012-07-11 08:42:11]

https://chromiumcodereview.appspot.com/10709006/diff/10006/chrome/browser/res...
File chrome/browser/resources/options2/cookies_list.js (right):

https://chromiumcodereview.appspot.com/10709006/diff/10006/chrome/browser/res...
chrome/browser/resources/options2/cookies_list.js:279: if (apps) {
On 2012/07/11 01:45:44, Evan Stade wrote:
> as it turns out I don't think this check is necessary (for..in won't do
anything
> if !apps)

Oh, cool. Removed.

https://chromiumcodereview.appspot.com/10709006/diff/10006/chrome/browser/res...
chrome/browser/resources/options2/cookies_list.js:518: case 'local_storage':
On 2012/07/11 01:45:44, Evan Stade wrote:
> you could potentially do
> 
> default:
>   text = loadTimeData.getString('cookie_' + this.data.type);
> 
> if you don't think it's too hacky.

Sure, done.

https://chromiumcodereview.appspot.com/10709006/diff/10006/chrome/browser/res...
chrome/browser/resources/options2/cookies_list.js:539: // Help out screen
readers and such: this is a clickable thing.
On 2012/07/11 01:45:44, Evan Stade wrote:
> could be wrong but I think you also need to tabindex = 0

Done.

https://chromiumcodereview.appspot.com/10709006/diff/10006/chrome/browser/res...
chrome/browser/resources/options2/cookies_list.js:548: if (item.selectedIndex ==
index)
On 2012/07/11 01:45:44, Evan Stade wrote:
> ternary operator

Done.

[Bernhard Bauer, 2012-07-11 08:45:19]

Peter, can I get a rubberstamp for chrome/browser/ui? Thanks!

[Peter Kasting, 2012-07-11 19:17:48]

LGTM

[commit-bot: I haz the power, 2012-07-11 20:25:50]

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

[commit-bot: I haz the power, 2012-07-11 20:26:15]

Failed to apply patch for chrome/browser/cookies_tree_model.cc:
While running patch -p1 --forward --force;
patching file chrome/browser/cookies_tree_model.cc
Hunk #2 FAILED at 38.
Hunk #3 succeeded at 156 (offset 30 lines).
Hunk #4 succeeded at 185 (offset 34 lines).
Hunk #5 succeeded at 212 (offset 38 lines).
Hunk #6 succeeded at 239 (offset 42 lines).
Hunk #7 succeeded at 265 (offset 46 lines).
Hunk #8 succeeded at 292 (offset 50 lines).
Hunk #9 succeeded at 319 (offset 54 lines).
Hunk #10 succeeded at 347 (offset 60 lines).
Hunk #11 succeeded at 373 (offset 64 lines).
Hunk #12 FAILED at 321.
Hunk #13 FAILED at 347.
Hunk #14 FAILED at 371.
Hunk #15 succeeded at 473 (offset 86 lines).
Hunk #16 succeeded at 482 (offset 86 lines).
Hunk #17 succeeded at 491 (offset 86 lines).
Hunk #18 succeeded at 499 (offset 86 lines).
Hunk #19 succeeded at 507 (offset 86 lines).
Hunk #20 succeeded at 515 (offset 86 lines).
Hunk #21 succeeded at 523 (offset 86 lines).
Hunk #22 succeeded at 533 (offset 86 lines).
Hunk #23 succeeded at 541 (offset 86 lines).
Hunk #24 succeeded at 556 (offset 86 lines).
Hunk #25 succeeded at 618 (offset 86 lines).
Hunk #26 succeeded at 632 (offset 86 lines).
Hunk #27 succeeded at 645 (offset 86 lines).
Hunk #28 succeeded at 659 (offset 86 lines).
Hunk #29 succeeded at 673 (offset 86 lines).
Hunk #30 succeeded at 687 (offset 86 lines).
Hunk #31 succeeded at 701 (offset 86 lines).
Hunk #32 succeeded at 716 (offset 86 lines).
Hunk #33 FAILED at 654.
Hunk #34 FAILED at 666.
Hunk #35 succeeded at 788 (offset 45 lines).
Hunk #36 FAILED at 790.
Hunk #37 succeeded at 859 with fuzz 1 (offset 50 lines).
Hunk #38 FAILED at 854.
Hunk #39 FAILED at 869.
Hunk #40 FAILED at 893.
Hunk #41 FAILED at 917.
Hunk #42 FAILED at 934.
Hunk #43 FAILED at 949.
Hunk #44 FAILED at 966.
Hunk #45 FAILED at 982.
Hunk #46 FAILED at 999.
Hunk #47 FAILED at 1014.
Hunk #48 FAILED at 1031.
Hunk #49 FAILED at 1047.
Hunk #50 FAILED at 1064.
Hunk #51 FAILED at 1080.
Hunk #52 FAILED at 1097.
Hunk #53 FAILED at 1110.
Hunk #54 FAILED at 1123.
Hunk #55 FAILED at 1139.
25 out of 55 hunks FAILED -- saving rejects to file
chrome/browser/cookies_tree_model.cc.rej

[commit-bot: I haz the power, 2012-07-12 17:29:37]

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

[commit-bot: I haz the power, 2012-07-12 17:51:30]

Try job failure for 10709006-16044 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-07-13 09:27:19]

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

[commit-bot: I haz the power, 2012-07-13 10:39:06]

Change committed as 146550