Teach the cookie tree view / model about appcaches. Not hooked up to real dat...

Teach the cookie tree view and model about appcaches. Not hooked up to real data yet, but the view and model pieces are in place for windows and gtk (not yet done for the mac).

Also adds a 'name' attribute to database details pane, cleans up the layout of the detail panes on windows.

BUG=25977
TEST=manual


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

[michaeln, 2010-02-24 21:59:48]

Peter, are you a good person to review this ui stuff?

http://codereview.chromium.org/650110/diff/1/5
File chrome/browser/cookies_tree_model.h (right):

http://codereview.chromium.org/650110/diff/1/5#newcode51
chrome/browser/cookies_tree_model.h:51: TYPE_APPCACHE,  // This is used for
CookieTreeAppCacheNode.
These repeated comments about node type probably don't need to be here, its
stating the obvious (over and over again). Any objections if I delete all of
these comments?

http://codereview.chromium.org/650110/diff/3001/4007
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/650110/diff/3001/4007#newcode249
chrome/browser/cookies_tree_model.cc:249: // maybe sort by display name
automagically??
This node is being placed at the last which is certainly not right... what
should the order be of these nodes... how 'bout alpha ordering?

* Application Caches
* Cookies
* Local Storage
* Web Databases

but that may sort differently in different locales... so how 'bout string
compares to find out?

[Peter Kasting, 2010-02-24 22:03:27]

On 2010/02/24 21:59:48, michaeln wrote:
> Peter, are you a good person to review this ui stuff?

Probably jochen and ifette would be better?  Dunno who on the GTK side.

[michaeln, 2010-02-24 22:04:57]

http://codereview.chromium.org/650110/diff/3070/3077
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/650110/diff/3070/3077#newcode280
chrome/browser/cookies_tree_model.cc:280:
CookieTreeAppCacheNode::CookieNodeComparator());
oh... i can probably just use the TitleComparator here

[michaeln, 2010-02-24 23:29:16]

hello

[jochen (gone - plz use gerrit), 2010-02-25 07:53:38]

http://codereview.chromium.org/650110/diff/3099/4059
File chrome/browser/browsing_data_appcache_helper.cc (right):

http://codereview.chromium.org/650110/diff/3099/4059#newcode60
chrome/browser/browsing_data_appcache_helper.cc:60:
completion_callback_.reset();
could you also patch the other helpers to reset the callback after running?

http://codereview.chromium.org/650110/diff/3099/4061
File chrome/browser/browsing_data_appcache_helper.h (right):

http://codereview.chromium.org/650110/diff/3099/4061#newcode67
chrome/browser/browsing_data_appcache_helper.h:67: scoped_ptr<Callback0::Type>
completion_callback_;
the other helpers return the info list in the callback. I think you should do
the same.

http://codereview.chromium.org/650110/diff/3099/4065
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/650110/diff/3099/4065#newcode148
chrome/browser/cookies_tree_model.cc:148:
l10n_util::GetString(IDS_COOKIES_WEB_DATABASE_UNNAMED_NAME) :
I don't think this is a good idea, since this will collapse all extensions
databases to a single node

[michaeln, 2010-02-25 20:53:43]

thnx for looking...

http://codereview.chromium.org/650110/diff/3099/4059
File chrome/browser/browsing_data_appcache_helper.cc (right):

http://codereview.chromium.org/650110/diff/3099/4059#newcode60
chrome/browser/browsing_data_appcache_helper.cc:60:
completion_callback_.reset();
On 2010/02/25 07:53:38, jochen wrote:
> could you also patch the other helpers to reset the callback after running?

sure... will do

http://codereview.chromium.org/650110/diff/3099/4061
File chrome/browser/browsing_data_appcache_helper.h (right):

http://codereview.chromium.org/650110/diff/3099/4061#newcode67
chrome/browser/browsing_data_appcache_helper.h:67: scoped_ptr<Callback0::Type>
completion_callback_;
On 2010/02/25 07:53:38, jochen wrote:
> the other helpers return the info list in the callback. I think you should do
> the same.

It pains me to make unnecessary copies of these collections... why is that a
good thing? Being consistent is good, but being consistently wasteful, is that
good ;)

I showed this ui to my office mate, first thing he asked is 'could you also show
the list of urls in each appcache', which of course we could, but that would
really inflate the size of these collections (and my pain) by a couple orders of
magnitude.

Maybe I should alter the others to not make copies here as well?

http://codereview.chromium.org/650110/diff/3099/4065
File chrome/browser/cookies_tree_model.cc (right):

http://codereview.chromium.org/650110/diff/3099/4065#newcode148
chrome/browser/cookies_tree_model.cc:148:
l10n_util::GetString(IDS_COOKIES_WEB_DATABASE_UNNAMED_NAME) :
On 2010/02/25 07:53:38, jochen wrote:
> this will collapse all extensions databases to a single node

I didn't think this would have that effect? I was assuming that each extension
got a unique 'origin' node in the cookie tree, is that not the case?

* This only applies to unnamed databases, those opened with an empty string for
their database name. I would expect most to actually be named in some way by the
creators.

* These are the leaf nodes in the tree. Multiple nodes with the same name can
exist, so they won't be 'collapsed' into one.

If ALL extensions are lumped into a single 'origin' directory in the cookie
tree, then named and unnamed databses have a similar problem. Extension
databases from different extensions with the same name (including no name) will
be indistinguishable in the tree (without examing the details).

[michaeln, 2010-02-25 23:23:28]

> > this will collapse all extensions databases to a single node
> 
> I didn't think this would have that effect? I was assuming that each extension
> got a unique 'origin' node in the cookie tree, is that not the case?

Each extension gets its own 'origin' node. These node have inscrutable names
(extension.id, which is just gibberish), there is no mention of the user
friendly extension.name attribute in the tree view.

But really, I don't think its appropriate to display extension data in this
'cookie and other site data' view at all. Feels like what resource an extension
is using belongs on the 'Extensions' page?

[michaeln, 2010-02-26 02:42:06]

ian, ping?

http://codereview.chromium.org/650110/diff/3099/4059
File chrome/browser/browsing_data_appcache_helper.cc (right):

http://codereview.chromium.org/650110/diff/3099/4059#newcode60
chrome/browser/browsing_data_appcache_helper.cc:60:
completion_callback_.reset();
On 2010/02/25 07:53:38, jochen wrote:
> could you also patch the other helpers to reset the callback after running?

Done

[jochen (gone - plz use gerrit), 2010-02-26 20:23:27]

LGTM

On 2010/02/26 02:42:06, michaeln wrote:
> ian, ping?
> 
> http://codereview.chromium.org/650110/diff/3099/4059
> File chrome/browser/browsing_data_appcache_helper.cc (right):
> 
> http://codereview.chromium.org/650110/diff/3099/4059#newcode60
> chrome/browser/browsing_data_appcache_helper.cc:60:
> completion_callback_.reset();
> On 2010/02/25 07:53:38, jochen wrote:
> > could you also patch the other helpers to reset the callback after running?
> 
> Done