Buttons for the collected cookies dialog to add content exceptions.

Buttons for the collected cookies dialog to add content exceptions.

BUG=45230
TEST=CollectedCookiesTest.*

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

[jochen (gone - plz use gerrit), 2010-07-19 16:45:08]

please review.

here's what the dialog now looks like on Windows http://imgur.com/XeWu6

this change finalizes the replacement of the cookie modal prompt.

Bernhard is doing the mac version.

[Peter Kasting, 2010-07-19 16:52:26]

Views LGTM.  By the way, "Session only" as an isolated string is pretty lousy. 
We should get a better set of strings for this dialog (maybe better button
strings _and_ more explanatory lines or something).  Work with the UI leads.

http://codereview.chromium.org/2808053/diff/1/5
File chrome/browser/views/collected_cookies_win.cc (right):

http://codereview.chromium.org/2808053/diff/1/5#newcode90
chrome/browser/views/collected_cookies_win.cc:90: // Buttons.
Nit: I think this would be clearer if you created each button right where you're
adding it to the layout, instead of in a block here.

http://codereview.chromium.org/2808053/diff/1/5#newcode181
chrome/browser/views/collected_cookies_win.cc:181: if (sender ==
block_allowed_button_) {
Nit: No {}

http://codereview.chromium.org/2808053/diff/1/5#newcode235
chrome/browser/views/collected_cookies_win.cc:235:
block_allowed_button_->SetEnabled(enable_allowed_buttons);
Nit: Move these two lines up to the block that sets |enable_allowed_buttons|.

[Evan Martin, 2010-07-19 18:47:58]

http://codereview.chromium.org/2808053/diff/1/3
File chrome/browser/gtk/collected_cookies_gtk.cc (right):

http://codereview.chromium.org/2808053/diff/1/3#newcode173
chrome/browser/gtk/collected_cookies_gtk.cc:173:
for_session_blocked_cookie_button_);
I feel like this is sorta copy and paste code, but I also think that refactoring
it into a function would require so many parameters to the function that it's
better to just have it copied.  Maybe since you've been staring at it longer you
could see a way to refactor it.

http://codereview.chromium.org/2808053/diff/1/3#newcode243
chrome/browser/gtk/collected_cookies_gtk.cc:243: continue;
This repeated loop can definitely be refactored to reduce duplicate code.

http://codereview.chromium.org/2808053/diff/1/4
File chrome/browser/gtk/collected_cookies_gtk.h (right):

http://codereview.chromium.org/2808053/diff/1/4#newcode40
chrome/browser/gtk/collected_cookies_gtk.h:40: void EnableControls();
Please, comments on all functions.

[Paweł Hajdan Jr., 2010-07-19 19:21:18]

Drive-by.

http://codereview.chromium.org/2808053/diff/1/2
File chrome/browser/collected_cookies_uitest.cc (right):

http://codereview.chromium.org/2808053/diff/1/2#newcode24
chrome/browser/collected_cookies_uitest.cc:24: TEST_F(CollectedCookiesTest,
FLAKY_DoubleDisplay) {
Oh, this has been flipped from FAILS to FLAKY. Do we have a FlakyTest bug for
this?

[jochen (gone - plz use gerrit), 2010-07-19 19:38:50]

fixed all comments.

I modified the buttons to reflect the discussion with the ui leads.

For allowed cookies, there's only a block button. For blocked cookies, there's a
allow and an allow for session button.

[Evan Martin, 2010-07-19 19:54:14]

linux lgtm