Attaching current state at r409749

Deleted: Screen Shot 2016-08-04 at 22.24.45.png 155 KB

	Screen Shot 2016-08-04 at 22.24.45.png 155 KB View Download

Attaching current state as of r417913

Deleted: Screen Shot 2016-09-12 at 8.36.45 AM.png 63.1 KB

	Screen Shot 2016-09-12 at 8.36.45 AM.png 63.1 KB View Download

Review notes: 
https://groups.google.com/a/google.com/forum/#!topic/chrome-harmony/xqTrAqnJ-JE

Just want to add that the dialog title casing should follow the platform.

Spec is here:

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Spec/Phase%202#%2FSPEC-Phase%202-Cookies%20in%20use.png%3Fz=width

Initial comments:

* Button title should be "Done", not "Close"
* Done button should be 16pt from right edge and bottom of dialog
* Block & remove buttons should be 16pt below scrollview
* Distance from bottom of Block and remove buttons and first data field should be 16pt
* There's too much space between the dialog title and the top of the tabview
* Distance from left edge of dialog to controls should be 16pt
* Seems like not enough vertical space between data fields (looks like you need about 7pt more)
* Field titles should be left-justified and not have colons
* Too much space between the tab view bar and the top of the "The following cookies" text
* Title of the dialog should be "Cookies in Use" (note title case for Mac)

* Dialog should be 450pt x 568pt

* Sorry, that should be 448pt x 568pt

Work screenshots for https://codereview.chromium.org/2511163002/

before = before that CL
after-old = after that CL, without harmony
after-new = after that CL, with harmony

Deleted: before.png 24.0 KB

	before.png 24.0 KB View Download

Deleted: after-old.png 23.8 KB

	after-old.png 23.8 KB View Download

Deleted: after-new.png 23.7 KB

	after-new.png 23.7 KB View Download

This is coming along!

Looking at after-new, some observations:

* Distance between the Block and Remove buttons should be 8pt (looks like it's 16pt)
* Close button should be 16pt from dialog right and bottom edges (looks like it's 20pt for both currently)
* The close button should be 16pt below the text above (looks like it's about 11pt)

The labels should be left-aligned rather than right-aligned - should that be a parameter in your layout delegate?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d

commit bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d
Author: ellyjones <ellyjones@chromium.org>
Date: Tue Nov 22 16:17:26 2016

harmony: convert collected cookies dialog

The collected cookies dialog now uses Harmony layout constants and spacing
when in Harmony mode.

BUG=610428

Review-Url: https://codereview.chromium.org/2511163002
Cr-Commit-Position: refs/heads/master@{#433879}

[modify] https://crrev.com/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d/chrome/browser/ui/views/collected_cookies_views.h
[modify] https://crrev.com/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
[modify] https://crrev.com/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d/chrome/browser/ui/views/harmony/harmony_layout_delegate.h
[modify] https://crrev.com/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d/chrome/browser/ui/views/harmony/layout_delegate.cc
[modify] https://crrev.com/bdb391bae9fd0c8f4ffdd706c1dabf510c291a4d/chrome/browser/ui/views/harmony/layout_delegate.h

#14:

Block/remove distance will be fixed by https://codereview.chromium.org/2526863004/
Close button right/bottom inset was fixed by https://codereview.chromium.org/2505973003/
Close button top inset will be fixed by https://codereview.chromium.org/2526863004/

After those changes, this dialog should be good to go.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0c6c3336fd015d6ffa3c7efff3a7b264eaf5d48

commit b0c6c3336fd015d6ffa3c7efff3a7b264eaf5d48
Author: ellyjones <ellyjones@chromium.org>
Date: Wed Nov 30 16:14:54 2016

harmony: fix collected cookies dialog spacing and button insets

This change:
1) Swaps the block/allow/remove button spacing over to RELATED_BUTTON
   instead of RELATED_CONTROL (-2px margin in non-Harmony mode)
2) Changes DialogClientView to use the ViewsDelegate's dialog button
   insets more. Only Harmony has a non-zero top inset for the buttons,
   so behavior in non-Harmony mode is unaffected.

BUG=610428

Review-Url: https://codereview.chromium.org/2526863004
Cr-Commit-Position: refs/heads/master@{#435278}

[modify] https://crrev.com/b0c6c3336fd015d6ffa3c7efff3a7b264eaf5d48/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/b0c6c3336fd015d6ffa3c7efff3a7b264eaf5d48/ui/views/window/dialog_client_view.cc

This is looking better. A few nits:

* The title should be 16pt from the left edge of the dialog.
* Dialog title should be "Cookies in use" (title case for Mac and ChromeOS(?))
* The Block / Remove buttons should be the same width, per  Issue 671820 
* The cookie field names should be left-aligned and without colons

#18
First point: https://codereview.chromium.org/2550323004/
Fourth point: https://codereview.chromium.org/2556563005/

Second should not be difficult to fix; third I am not sure about at the moment.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9bf7c586548ab8b4616553b9fbe1d75f0f96b652

commit 9bf7c586548ab8b4616553b9fbe1d75f0f96b652
Author: ellyjones <ellyjones@chromium.org>
Date: Wed Dec 07 18:22:05 2016

views: support Harmony title insets

This change adds a method to ViewsDelegate called GetDialogFrameViewInsets,
which returns the insets that should be applied around a dialog's FrameView
title.

BUG=610428

Review-Url: https://codereview.chromium.org/2550323004
Cr-Commit-Position: refs/heads/master@{#437010}

[modify] https://crrev.com/9bf7c586548ab8b4616553b9fbe1d75f0f96b652/chrome/browser/ui/views/chrome_views_delegate.cc
[modify] https://crrev.com/9bf7c586548ab8b4616553b9fbe1d75f0f96b652/chrome/browser/ui/views/chrome_views_delegate.h
[modify] https://crrev.com/9bf7c586548ab8b4616553b9fbe1d75f0f96b652/ui/views/views_delegate.cc
[modify] https://crrev.com/9bf7c586548ab8b4616553b9fbe1d75f0f96b652/ui/views/views_delegate.h
[modify] https://crrev.com/9bf7c586548ab8b4616553b9fbe1d75f0f96b652/ui/views/window/dialog_delegate.cc

Remaining issues:

* Labels should not end in colons
* The Block / Remove buttons should be the same width, per  Issue 671820 
* Get rid of the <no cookie selected>s

And one more:

* Dialog title should be Title Case

I discovered this dialog can also be shown via a content settings bubble (see attached). Hooking that up in https://codereview.chromium.org/2534743002

Still need to:
 * remove colons, make buttons the same width, remove <no cookie selected>, rename title+titlecase as "Cookies in use"

(context screenshot for #c24)

Deleted: Screen Shot 2017-01-11 at 11.21.05.png 57.6 KB

	Screen Shot 2017-01-11 at 11.21.05.png 57.6 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3888fbf6c8994f26f65ba506341a475b0c2f4e1f

commit 3888fbf6c8994f26f65ba506341a475b0c2f4e1f
Author: tapted <tapted@chromium.org>
Date: Thu Jan 12 02:55:21 2017

MacViews: Harmony for TabDialogs' dialogs (starting with Collected Cookies)

Collected cookies when invoked from the site info bubble (padlock ->
Cookies -> click the "X in use" link) is already hooked up to
--secondary-ui-md. However, it can also be shown via the content
settings bubble when a cookie is blocked.

We also need TabDialogs' Signin confirmation dialog switched to Harmony,
so set up a way to show Harmony dialogs from the TabDialogs interface.

Opt collected cookies into DialogBrowserTest to make it easy to invoke
for testing.

BUG= 654151 ,  677012 , 610428

Review-Url: https://codereview.chromium.org/2534743002
Cr-Commit-Position: refs/heads/master@{#443130}

[delete] https://crrev.com/590f8e6768702a8172c67b3cda7745107b6b1e0d/chrome/browser/collected_cookies_browsertest.cc
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_cocoa.h
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_cocoa.mm
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_views_mac.h
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/cocoa/tab_dialogs_views_mac.mm
[add] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/browser/ui/collected_cookies_browsertest.cc
[modify] https://crrev.com/3888fbf6c8994f26f65ba506341a475b0c2f4e1f/chrome/test/BUILD.gn

Issues that still remain:

* Button title should be "Done", not "Close"
* Title of the dialog should be "Cookies in Use" (note title case for Mac)
* Labels should not end in colons
* The Block / Remove buttons should be the same width, per  Issue 671820 
* Get rid of the <no cookie selected>s
* There should be 16pt of space from the bottom of the expires textfield to the top of the Done button

rpop@ points out that there is a spec for the tableview, so we are not blocked on that control for shipping this dialog. Only question is should we be using Mac folder icons (which are in the screenshot) on Windows.

The folder icons are currently being used in the bookmarks bar, so look there for relevant code. Note that the icon should not change as you expand/collapse the item.

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview#%2FP%20-%20Cookies%20in%20use.png%3Fz=width

bettes@ question for you: can you confirm the mock in #c28 is how the dialog should look on all platforms?

Other notes:

It looks like the selection should extend the entire row on all platforms, not just on mac?

My folder icons actually look quite different on Windows (attached). And it's different again on ChromeOS (a monochrome vector icon).

On Mac, we currently use [[NSWorkspace sharedWorkspace] iconForFileType:NSFileTypeForHFSTypeCode(hfs_type)]; (in resource_delegate_mac.mm) for the toolkit-views treeview icons, but not the toolbar. That is, we use the icon from the OS (which, e.g., is probably different on 10.9 vs 10.12).

We probably also want a new vector icon for the arrow - I think it's currently a raster asset and it looks kinda naff on hidpi.

Deleted: Screen Shot 2017-02-10 at 9.39.39 am.png 4.4 KB

	Screen Shot 2017-02-10 at 9.39.39 am.png 4.4 KB View Download

Folder icons are expected (and desired) to look different on different platforms.

For non-Mac, don't worry about having the right folder icon for now.  There is some confusion with folder icons, but in general, the code today is doing approximately what we want, and we can do further cleanup later.

For Mac, I can't say what the current/desired behavior are.

I think I had a question out to Alan on another bug about opening/closing folders, and when we should do it if ever, since various places are inconsistent today.  I need to dig that up and forward it to him to get a confirmed answer.  If you need to move forward before that, go with Jayson's comments about never showing an open folder icon, as that's a reasonable default.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81aeead4d36ae4401894ab0b059d9721a30c9521

commit 81aeead4d36ae4401894ab0b059d9721a30c9521
Author: tapted <tapted@chromium.org>
Date: Tue Mar 07 23:20:45 2017

Implement Harmony-style consistent button widths for Collected Cookies.

Adds a member to GridLayout's ColumnSet which imposes a threshold to
ignore a column when unifying same sized columns.

This also needs to be done for DialogClientView, perhaps via
ViewsDelegate, which may be influenced by  http://crbug.com/687349 .

BUG= 671820 , 610428
TEST=GridLayoutTest.TwoColumnsLinkedSizes

Review-Url: https://codereview.chromium.org/2625083003
Cr-Commit-Position: refs/heads/master@{#455281}

[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/chrome/browser/ui/views/harmony/layout_delegate.cc
[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/chrome/browser/ui/views/harmony/layout_delegate.h
[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/ui/views/layout/grid_layout.cc
[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/ui/views/layout/grid_layout.h
[modify] https://crrev.com/81aeead4d36ae4401894ab0b059d9721a30c9521/ui/views/layout/grid_layout_unittest.cc

Is there more to do on this dialog that we want to block Harmony for?

Deleted: Screen Shot 2017-06-27 at 3.35.57 pm.png 98.7 KB

	Screen Shot 2017-06-27 at 3.35.57 pm.png 98.7 KB View Download

The RTL on the TabbedPane control is broken:  Issue 733115 . Other than that, I think this dialog is close to good.

Here are the remaining issues I see. This dialog will not be ready for review until they are fixed:

* Button title should be "Done", not "Close"
* Title of the dialog should be "Cookies in Use" (note title case for Mac)
* Labels should not end in colons
* Get rid of the <no cookie selected>s
* There should be 16pt of space from the bottom of the expires textfield to the top of the Done button
* The folder icon should be Material Design style (like in the bookmarks bar)
* Remove the close button
* Remove the colon from the end of "The following cookies were set when you viewed this page"

I'll try to look at this next week and fix the issues from #34.

Clarifying title since this is cross-platform

Also elevating to P1 as this is part of the "primary/page info/permissions" group we want to get done first

The NextAction date has arrived: 2017-08-11

 Issue 658355  has been merged into this issue.

New previews and specs available:  
https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Cookies%20in%20use#%3Fz=width

Proposing new interaction if possible: expanding/collapsing the height of the dialog depending on the hide/show of cookie details. Ideally we use 1 dialog that transitions in heights, and not 2 dialogs with different heights. If that's not the case, please let me know. 

The previous HTML redlines are out-of-date and hard to update so please don't refer to them for layout guidance. 

Deleted: 01 - CIU - allow - A.png 1009 KB

	01 - CIU - allow - A.png 1009 KB View Download

Deleted: 01 - CIU - allow - C.png 1020 KB

	01 - CIU - allow - C.png 1020 KB View Download

Deleted: Spec - CIU.png 191 KB

	Spec - CIU.png 191 KB View Download

Also note, the infobar has updated strings to match with other content settings strings

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Cookies%20in%20use#%2F04%20-%20CIU%20-%20infobar.png%3Fz=width

Deleted: 04 - CIU - infobar.png 1005 KB

	04 - CIU - infobar.png 1005 KB View Download

I think comment 43 is maybe doable, but please try to avoid proposing behavioral changes at this stage.  We are well past time to lock down the scope of what we're trying to ship; Harmony v1 was intended to be a re-skin, not a rework of behavior.  We've already seen nontrivial scope creep in other areas (e.g. content settings bubbles), and I'd like to discourage more.

So if we have time to get to this, we may, but I won't block shipping on it.  I suggest we start collecting behavioral changes we'd like into a list of what we want to revisit in v2.

Trent notes that it sounds like this calls for animating a dialog size transition.  I don't know whether there's a facility for easily doing that in views without potential jank.  That part is almost certainly out of scope for v1.

https://chromium-review.googlesource.com/c/chromium/src/+/663402 (dialog changes)
https://chromium-review.googlesource.com/c/chromium/src/+/663405 (treeview icon)

cookies-old-unsel: the dialog as of now, with no cookie selected
cookies-old-sel: the dialog as of now, with a cookie selected
cookies-new-unsel: the dialog with those two changes, with no cookie selected
cookies-new-sel: the dialog with those two changes, with a cookie selected

There's two unresolved questions:

1) The bottom part of cookies-new-unsel looks very "unbalanced" to me. I think the proposal in #43 was one idea for mitigating this, but perhaps we need some other approach if we don't want to animate. I talked to tapted@ and it sounds like we can't do non-janky programmatic resizes of dialogs in MacViews right now.

2) The infobar in this dialog (see infobar.png) still looks pretty bad. I'm not sure how to get rid of it or fix it. It's communicating that the Block button actually *did* something, but it causes the dialog to need a bunch of spare vertical space to fit the it in without resizing the dialog when it appears. Perhaps as a first pass we could remove the border around it? See infobar-white and infobar-grey for some examples of how that looks.

Deleted: cookies-old-sel.png 37.1 KB

	cookies-old-sel.png 37.1 KB View Download

Deleted: cookies-old-unsel.png 35.5 KB

	cookies-old-unsel.png 35.5 KB View Download

Deleted: cookies-new-sel.png 37.2 KB

	cookies-new-sel.png 37.2 KB View Download

Deleted: cookies-new-unsel.png 29.2 KB

	cookies-new-unsel.png 29.2 KB View Download

Deleted: infobar.png 32.6 KB

	infobar.png 32.6 KB View Download

Deleted: infobar-white.png 22.5 KB

	infobar-white.png 22.5 KB View Download

Deleted: infobar-grey.png 22.4 KB

	infobar-grey.png 22.4 KB View Download

ellyjones@ - what's up with the row selection in the table? The gray selection background should extend the entire width of the table.

#48: sorry, I should've mentioned - the screenshots are from my Linux workstation, not my Mac. On Mac they do, and the title is titlecase.

Note that #43 introduces a new pattern that would make the infobars redundant. 

Preferred: 
Use the right-handed text to communicate state. Applicable for blocked, removed, allowed, and clear on exit.

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)/Preview/11-Permissions/Cookies%20in%20use#%2F04%20-%20CIU%20-%20infobar.png%3Fz=width

Fall-back: 
Use the infobar pattern seen in infobar-white.png (with modifications) 

Deleted: Screen Shot 2017-09-13 at 10.38.14 AM.png 106 KB

	Screen Shot 2017-09-13 at 10.38.14 AM.png 106 KB View Download

Screenshot of proposed pattern

Deleted: Screen Shot 2017-09-13 at 10.40.07 AM.png 41.3 KB

	Screen Shot 2017-09-13 at 10.40.07 AM.png 41.3 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/efec5839674a3443f4648437c261456dfffc10d1

commit efec5839674a3443f4648437c261456dfffc10d1
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Sep 20 16:04:58 2017

views: factor out TreeView drawing options

Currently, it's difficult for consumers of TreeView to customize its visuals.
All the painting methods are private and there's no easy way to subclass
TreeView to change their behavior. This change:

1) Adds an explicit extension point for TreeView painting, named
   TreeView::DrawingProvider,
2) Moves some of the existing painting logic into the base DrawingProvider
   implementation,
3) Adds a sample DrawingProvider to TreeViewExample

This will be required to customize TreeView behavior in the collected cookies
dialog (https://crbug.com/610428).

Bug: 610428
Change-Id: Ia1c1c41f3f54065b30ed276f7ee8c390aab3d0d6
Reviewed-on: https://chromium-review.googlesource.com/667206
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503165}
[modify] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/BUILD.gn
[modify] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/controls/tree/tree_view.cc
[modify] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/controls/tree/tree_view.h
[add] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/controls/tree/tree_view_drawing_provider.cc
[add] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/controls/tree/tree_view_drawing_provider.h
[modify] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/examples/tree_view_example.cc
[modify] https://crrev.com/efec5839674a3443f4648437c261456dfffc10d1/ui/views/examples/tree_view_example.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe64540ca12c99af81b692529c69140a6c3f065d

commit fe64540ca12c99af81b692529c69140a6c3f065d
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Sep 20 16:58:55 2017

views: refresh collected cookies for Harmony

This change:
1) Removes trailing colons from labels
2) Removes the '<no cookie selected>' string in
   Harmony mode
3) Changes "Close" to "Done" on the blue button
4) Makes the title titlecase on macOS

Bug: 610428
Change-Id: I57ac357496ba9617e2ab4912b376748f7e3588f7
Reviewed-on: https://chromium-review.googlesource.com/663402
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503185}
[modify] https://crrev.com/fe64540ca12c99af81b692529c69140a6c3f065d/chrome/app/generated_resources.grd
[modify] https://crrev.com/fe64540ca12c99af81b692529c69140a6c3f065d/chrome/browser/ui/views/collected_cookies_views.cc

Screenshots of the new auxiliary text & fading.

Deleted: allowed.png 24.8 KB

	allowed.png 24.8 KB View Download

Deleted: blocked.png 23.6 KB

	blocked.png 23.6 KB View Download

We should not show the words Blocked or Allowed because those words also appear in the tabs just above (one is info, the other is a control), which is confusing. If we need to show this info we should use some kind of icon instead.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86a324e6362fa1606fb3131848e913f6b2991135

commit 86a324e6362fa1606fb3131848e913f6b2991135
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Sep 21 11:53:24 2017

views: add blocked state to collected cookies

This change:
1) Colors rows that have been modified more lightly than usual, and
2) Adds descriptive text explaining how they have been modified

See the bug for screenshots.

Bug: 610428
Change-Id: Ie107a28d4a6d7d8a76fbdc59426933b994c2bd36
Reviewed-on: https://chromium-review.googlesource.com/676223
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503418}
[modify] https://crrev.com/86a324e6362fa1606fb3131848e913f6b2991135/chrome/app/generated_resources.grd
[modify] https://crrev.com/86a324e6362fa1606fb3131848e913f6b2991135/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/86a324e6362fa1606fb3131848e913f6b2991135/chrome/browser/ui/views/collected_cookies_views.h

#55: Those only appear in response to the user pressing the "Block" button, so they serve as confirmation that the button took effect. They aren't ever shown otherwise. Does that alleviate your concern about confusion?

(This is also live on Canary now so you're welcome to try it out and see how it feels)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f16fdcf9c8c1e6cb5077130e621fcaa9e03ca40

commit 2f16fdcf9c8c1e6cb5077130e621fcaa9e03ca40
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Sep 26 13:38:34 2017

views: remove infobar border in collected cookies

This isn't wanted for Harmony, and it looks a bit out of place even in the
non-Harmony version of this dialog.

Bug: 610428
Change-Id: I82429d8317d392eaf29803bd1870f1a07420dba1
Reviewed-on: https://chromium-review.googlesource.com/681837
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504353}
[modify] https://crrev.com/2f16fdcf9c8c1e6cb5077130e621fcaa9e03ca40/chrome/browser/ui/views/collected_cookies_views.cc

> #55: Those only appear in response to the user pressing the "Block" button, so they serve as confirmation that the button took effect. They aren't ever shown otherwise. Does that alleviate your concern about confusion?

This dialog needs to be rethought, but this is way beyond the scope of what we're trying to do right now.

Okay - I think this dialog is good to go (ish), except that  issue 733115  (TabbedPane doesn't support RTL) affects it. What's my next step here? It doesn't conform to a mock since there isn't an updated mock for it - can I just send it off for review?

A couple things I'm seeing:

* The folder icons are not the Mac MD style
* There should not be a close (x) button
* The Block and Remove buttons should be flush right
* If you select the first item in the tableview there's a thin strip of white between the top of that item and the scrollview border - the selection background should be flush with that edge. There's a similar problem with selecting the last item and the bottom scrollview edge.
* When you choose Blocked the "Clear on exit" button should be Title case on Mac
* The Done button is not blue, but looking at the mock it's not blue there either - do you know why we don't want the Return keyboard shortcut to work here?
* I had been saying that the "Done" button should have the title Done but looking in the mock in c#1 it says Close. I don't know anymore and neither title is good, so switching it back to Close might be one last change to bring it up to the spec.

The NextAction date has arrived: 2017-09-29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d51f1314d79b6beea525e6f3a6c8d118fec53bf6

commit d51f1314d79b6beea525e6f3a6c8d118fec53bf6
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Mon Oct 02 13:22:22 2017

views: partly harmonize collected cookies

This change:
1) Makes "Clear on exit" titlecase on Mac
2) Right-justifies the cookie management buttons
3) Hides the top-right close button
4) Changes the text on the bottom-right close button to "Close"
5) Makes the bottom-right close button the default

Bug: 610428
Change-Id: I66440d887e2063d10ea30e4ca3f9a07846c0fc42
Reviewed-on: https://chromium-review.googlesource.com/693221
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505593}
[modify] https://crrev.com/d51f1314d79b6beea525e6f3a6c8d118fec53bf6/chrome/app/generated_resources.grd
[modify] https://crrev.com/d51f1314d79b6beea525e6f3a6c8d118fec53bf6/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/d51f1314d79b6beea525e6f3a6c8d118fec53bf6/chrome/browser/ui/views/collected_cookies_views.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bd7d272833a2cc559e8940202809f4e3578ded1

commit 0bd7d272833a2cc559e8940202809f4e3578ded1
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Oct 05 18:32:28 2017

views: remove TreeView vertical margin

This change is needed for the Harmony treeview, but it also looks better in the
non-Harmony treeview, so it's unconditional in this CL.

This change also removes the special background/foreground colors from the
example treeview. While those were useful as a demonstration, they are quite
garish and distracting when screenshotting the treeview :).

Bug: 610428
Change-Id: Icf14b2e1d5b8c20a6d72550cfe57436b3f31cce4
Reviewed-on: https://chromium-review.googlesource.com/695563
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506800}
[modify] https://crrev.com/0bd7d272833a2cc559e8940202809f4e3578ded1/ui/views/controls/tree/tree_view.cc
[modify] https://crrev.com/0bd7d272833a2cc559e8940202809f4e3578ded1/ui/views/examples/tree_view_example.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/60969ece461f1771494128e427aebfde3bbcbd44

commit 60969ece461f1771494128e427aebfde3bbcbd44
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Oct 05 19:28:11 2017

views: use MD folder icon for treeviews in Harmony mode on Mac

This change:
1) Moves the MD folder icon into //components
2) Makes the Views TreeView use it in Harmony mode

Note that there is no separate "open folder" icon in MD,
so the open and close icons are identical.

This change was previously reviewed as
<https://chromium-review.googlesource.com/c/chromium/src/+/663405>.

Bug: 610428
Change-Id: Ife816d911bef2a92497bb687c5aa59345359aa74
Reviewed-on: https://chromium-review.googlesource.com/692817
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506822}
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/chrome/app/vector_icons/BUILD.gn
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/chrome/browser/BUILD.gn
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/chrome/browser/chrome_browser_main_mac.h
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/chrome/browser/chrome_browser_main_mac.mm
[delete] https://crrev.com/c74acf2507bf54f87a72e4140e4e8e994dda84cd/chrome/browser/resource_delegate_mac.h
[delete] https://crrev.com/c74acf2507bf54f87a72e4140e4e8e994dda84cd/chrome/browser/resource_delegate_mac.mm
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/chrome/browser/ui/bookmarks/bookmark_utils.cc
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/components/vector_icons/BUILD.gn
[rename] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/components/vector_icons/folder.1x.icon
[rename] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/components/vector_icons/folder.icon
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/ui/views/controls/tree/tree_view.cc
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/ui/views/style/platform_style.cc
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/ui/views/style/platform_style.h
[modify] https://crrev.com/60969ece461f1771494128e427aebfde3bbcbd44/ui/views/style/platform_style_mac.mm

alan, over to you :)

Thanks Elly! LMK if you have any questions/concerns.

1. There should be 16pt between title and tabs (to match the spacing above the title)
2. A few things: 
   - The tab height should measure 32pt, not 40pt
   - allowed text color: #4285F4 
   - Blocked text color: #757575 
   - grey rule line underneath tabs: #000000 0.12a (Because the rule line is 1pt not 1px)
3. font size: 13pt not 12
4. A few things
   - Can we round the corners of this frame? To match the button corner radii
   - Can we make the outline of this 1px and not 1pt? (rendered as 1px on 2x) 
5. We wanted to remove these page icons 
6. The folder icon has 2 states: open and close. The open icon does not appear when i use the expand arrow, but only when I select the folder label. Can we show show the open folder for both? 
7. Replace icon with: https://cs.chromium.org/chromium/src/chrome/browser/ui/page_info/page_info_ui.cc?q=%E2%80%93
8. The mocks have Block and Remove at the bottom of the dialog. Are there limitations to not doing this for v1? 
9. How do we define the spacing between columns? In the mocks, I assumed there was a max-label width for the left column (https://screenshot.googleplex.com/SKm6zzPWN1G), but that may not be the most logical. I essentially want 96px between the two labels, can you suggest how we can get there? 
10. Just confirming that we cannot collapse this space when (i)info is not present? 
11. "Close" should be "Done" 
12. the padding to the left of the icon should be 16pt, same as padding b/t icon and text
13. no punctuation on these strings

Deleted: CC - review.png 359 KB

	CC - review.png 359 KB View Download

Deleted: CC - comparison.png 246 KB

	CC - comparison.png 246 KB View Download

X-reffing video for CL https://chromium-review.googlesource.com/c/chromium/src/+/736329

Deleted: cookies_view.mp4 710 KB

	cookies_view.mp4 710 KB View Download

#67:

(1): caused by the builtin vertical size of the TabbedPane, more or less - reducing the vertical size there will help, but it may still not be 16pt.
(2): TabbedPane height will be <https://chromium-review.googlesource.com/c/chromium/src/+/735072>, TabbedPane colors will be <https://chromium-review.googlesource.com/c/chromium/src/+/736329>
(3): Will be <https://chromium-review.googlesource.com/c/chromium/src/+/746367>

Still working down the rest of the list.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8502b9514b78dfd69c4d9c02214c44cdae1b5353

commit 8502b9514b78dfd69c4d9c02214c44cdae1b5353
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Oct 31 14:42:35 2017

views: reduce Harmony TabbedPane height

Bug: 610428
Change-Id: I1df1aec018917de3e7a79112f9a6b68fdaf41073
Reviewed-on: https://chromium-review.googlesource.com/735072
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512826}
[modify] https://crrev.com/8502b9514b78dfd69c4d9c02214c44cdae1b5353/ui/views/controls/tabbed_pane/tabbed_pane.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09afc263e0ab7323c8db3dd80f1e04f3ac09bb09

commit 09afc263e0ab7323c8db3dd80f1e04f3ac09bb09
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Oct 31 19:40:10 2017

views: increase font size on collected cookies top labels

These are meant to be 13pt, not 12pt.

Bug: 610428
Change-Id: I3718197e67e2eef26611572dfcbfdb40ffcce405
Reviewed-on: https://chromium-review.googlesource.com/746367
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512908}
[modify] https://crrev.com/09afc263e0ab7323c8db3dd80f1e04f3ac09bb09/chrome/browser/ui/views/collected_cookies_views.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3c4d1127f45408e260b5810c3caafa3eddba067

commit a3c4d1127f45408e260b5810c3caafa3eddba067
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Nov 02 17:26:02 2017

views: update TabbedPane colors

Bug: 610428
Change-Id: Ic2db53e479ec97cda7e6090b7ede0d957ff4b69d
Reviewed-on: https://chromium-review.googlesource.com/736329
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513534}
[modify] https://crrev.com/a3c4d1127f45408e260b5810c3caafa3eddba067/chrome/browser/ui/libgtkui/native_theme_gtk3.cc
[modify] https://crrev.com/a3c4d1127f45408e260b5810c3caafa3eddba067/ui/native_theme/common_theme.cc
[modify] https://crrev.com/a3c4d1127f45408e260b5810c3caafa3eddba067/ui/native_theme/native_theme.h
[modify] https://crrev.com/a3c4d1127f45408e260b5810c3caafa3eddba067/ui/native_theme/native_theme_dark_aura.cc
[modify] https://crrev.com/a3c4d1127f45408e260b5810c3caafa3eddba067/ui/views/controls/tabbed_pane/tabbed_pane.cc

(5) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/757035>
(6) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/757074>
(7) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/757056>

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe0365f4f914850811e812bb0478b0263656b021

commit fe0365f4f914850811e812bb0478b0263656b021
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Nov 07 22:37:13 2017

views: use open folder icon for open treeview folders

Bug: 610428
Change-Id: I91726bc0f69193914e373f34618436f3d175fac0
Reviewed-on: https://chromium-review.googlesource.com/757074
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514630}
[modify] https://crrev.com/fe0365f4f914850811e812bb0478b0263656b021/ui/views/controls/tree/tree_view.cc

(8) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/757218>
(9) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/758596>
(10) not easily, no. It will require some toolkit work.
(11) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/758557)
(12) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/758558>
(13) will be fixed by <https://chromium-review.googlesource.com/c/chromium/src/+/758557>

That leaves (4), the rounded corners / border. Making the corners rounded is quite difficult, because the TreeView is secretly wrapped in another view to make it scroll-able, and also I worry that it would look bad, since the scrollbars are very rectilinear and the rounded corners will round off just one corner of each of the scrollbar's buttons.

Making the border a 1px stroke in 2x shouldn't be terribly difficult, but won't that make it quite difficult to see? It's already a fairly light color. Perhaps a 1px stroke in solid black?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fdef810d4983e1c7640bee65fcf9591e5442fe4

commit 3fdef810d4983e1c7640bee65fcf9591e5442fe4
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Nov 08 18:48:21 2017

views: relocate collected cookies buttons

This change:
1) Moves the buttons on the collected cookies dialog to the bottom-left,
   which is where the Harmony spec calls for them to be

Bug: 610428
Change-Id: Ic28e1a7256ded5b2fdd6d9ce1a1c4d252034f293
Reviewed-on: https://chromium-review.googlesource.com/757218
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514896}
[modify] https://crrev.com/3fdef810d4983e1c7640bee65fcf9591e5442fe4/chrome/browser/ui/views/collected_cookies_views.cc
[modify] https://crrev.com/3fdef810d4983e1c7640bee65fcf9591e5442fe4/chrome/browser/ui/views/collected_cookies_views.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9244aa3d489a8b6f28db3c14d87e40939c31040f

commit 9244aa3d489a8b6f28db3c14d87e40939c31040f
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Nov 09 14:01:20 2017

views: update collected cookies strings, part 2

This change:
1) Changes the button text on collected cookies from "Close"
   to "Done"
2) Removes trailing punctuation from the infobar strings

Bug: 610428
Change-Id: Idfeea9a3d45fac759a1746cd3165fa9b9479520d
Reviewed-on: https://chromium-review.googlesource.com/758557
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515162}
[modify] https://crrev.com/9244aa3d489a8b6f28db3c14d87e40939c31040f/chrome/app/generated_resources.grd
[modify] https://crrev.com/9244aa3d489a8b6f28db3c14d87e40939c31040f/chrome/browser/ui/views/collected_cookies_views.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed18fe2b20502b20ee5a2172ae42509e3cd90f8f

commit ed18fe2b20502b20ee5a2172ae42509e3cd90f8f
Author: Elly Fong-Jones <ellyjones@google.com>
Date: Mon Nov 13 21:48:52 2017

views: normalize collected cookies infobar padding

This change replaces the infobar's custom padding with the
new INSETS_DIALOG padding constant. This causes the infobar's icon
to be aligned with the other controls on the left margin.

Bug: 610428
Change-Id: Idb4c2faaa16004951eaf9988cd768b7dcc359167
Reviewed-on: https://chromium-review.googlesource.com/758558
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516067}
[modify] https://crrev.com/ed18fe2b20502b20ee5a2172ae42509e3cd90f8f/chrome/browser/ui/views/collected_cookies_views.cc

Tried to test this issue on Chrome #64.0.3268.0 on Windows 10 & Ubuntu 14.04 by referring the mocks provided. Upon navigating to chrome://settings >> Cookies, content displayed under cookies are not as seen in mocks. Attached the screenshot for reference.

@ellyjones -- Could you please let us know the exact steps to verify the issue and expected changes that needs to be verified.

Thanks in advance.

Deleted: 610428.png 104 KB

	610428.png 104 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15769c1f60d31af5909aff46bf3a15d1da9067ad

commit 15769c1f60d31af5909aff46bf3a15d1da9067ad
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Nov 16 14:20:16 2017

views: widen padding in collected cookies dialog

The Harmony mocks for this dialog call for 96pt of spacing between
label and value.

Bug: 610428
Change-Id: I1c15e1863cfadedf85f00b0799aaf6f2e5020367
Reviewed-on: https://chromium-review.googlesource.com/758596
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517078}
[modify] https://crrev.com/15769c1f60d31af5909aff46bf3a15d1da9067ad/chrome/browser/ui/views/cookie_info_view.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e207417c02e47a0d986f571c6078807b00d16c00

commit e207417c02e47a0d986f571c6078807b00d16c00
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Nov 21 16:53:15 2017

views: fix node hiding in TreeView

This change:
1) Corrects an insidious bug where TreeView::InternalNode::GetMaxWidth
   would pass the InternalNode* as the node instead of the model node,
   which would cause crashes at call sites when attempting to cast it
   back to the model node type and use it. This avoided the type system's
   notice because InternalNode is a subclass of ui::TreeModelNode. Ow.
2) Computes the foreground bounds properly for a node whose icon is
   hidden, so that the selection foreground is drawn only where the text
   is.
3) Repaints the entire node's bounds when repainting a node, not just
   its background, since if the node is being repainted, its bounds may
   also have changed - e.g., if it had its icon shown before and hidden
   now. In that case, simply repainting inside the new bounds is not
   sufficient, because the old bounds may have been greater.

Bug: 610428
Change-Id: I0bce687b80248601259aa243d38230b615a8974c
Reviewed-on: https://chromium-review.googlesource.com/756857
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518280}
[modify] https://crrev.com/e207417c02e47a0d986f571c6078807b00d16c00/ui/views/controls/tree/tree_view.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28d2ab7b1d50205d0fd6313b84e3b6b8f9d657c3

commit 28d2ab7b1d50205d0fd6313b84e3b6b8f9d657c3
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Nov 21 18:22:45 2017

views: hide host icons in collected cookies dialog

Bug: 610428
Change-Id: I314556a9c835606acc11b80da480971fe083b93c
Reviewed-on: https://chromium-review.googlesource.com/757035
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518309}
[modify] https://crrev.com/28d2ab7b1d50205d0fd6313b84e3b6b8f9d657c3/chrome/browser/ui/views/collected_cookies_views.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18ac7f17e5517f3d6ce9bebab472e87cb0292c6c

commit 18ac7f17e5517f3d6ce9bebab472e87cb0292c6c
Author: Elly Fong-Jones <ellyjones@google.com>
Date: Tue Nov 21 20:03:02 2017

views: remove auxiliary text from collected cookies

Right now, this text is redundant with the infobar, although it will need
to be re-added when this dialog is redesigned without the infobar. Worse,
it is inaccurate in the "clear on exit" case of the Blocked panel.

Bug: 610428,  787375 
Change-Id: I653ff6790f1b141a71007df38c09f8e917fcfa6b
Reviewed-on: https://chromium-review.googlesource.com/782661
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518347}
[modify] https://crrev.com/18ac7f17e5517f3d6ce9bebab472e87cb0292c6c/chrome/browser/ui/views/collected_cookies_views.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25548b8e1a7f7831c86f1e52cdc5f0ec757d1a06

commit 25548b8e1a7f7831c86f1e52cdc5f0ec757d1a06
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Mon Nov 27 14:35:28 2017

chrome: update cookie icon in cookies dialog

This change moves the collected cookies dialog from the
colorful pre-MD icon to the monochrome MD icon, and deletes
the now-unused old icon resource.

Bug: 610428
Change-Id: I71638b4ec27e17b94472fa6286949b9e50a6d09b
Reviewed-on: https://chromium-review.googlesource.com/757056
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519296}
[delete] https://crrev.com/316e035cd2bff593e99410ee786b0e956a89d1d2/chrome/app/theme/default_100_percent/common/cookie.png
[delete] https://crrev.com/316e035cd2bff593e99410ee786b0e956a89d1d2/chrome/app/theme/default_200_percent/common/cookie.png
[modify] https://crrev.com/25548b8e1a7f7831c86f1e52cdc5f0ec757d1a06/chrome/app/theme/theme_resources.grd
[modify] https://crrev.com/25548b8e1a7f7831c86f1e52cdc5f0ec757d1a06/chrome/browser/browsing_data/cookies_tree_model.cc

 Issue 786300  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c96d7df6e5c893099b4ab243e7d4f4ed8ec3d682

commit c96d7df6e5c893099b4ab243e7d4f4ed8ec3d682
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Mon Nov 27 15:48:57 2017

views: remove auxiliary text from collected cookies

Right now, this text is redundant with the infobar, although it will need
to be re-added when this dialog is redesigned without the infobar. Worse,
it is inaccurate in the "clear on exit" case of the Blocked panel.

TBR=ellyjones@google.com

(cherry picked from commit 18ac7f17e5517f3d6ce9bebab472e87cb0292c6c)

Bug: 610428,  787375 
Change-Id: I20065f6823e60c6903a65d8ab022e77674cbdd58
Reviewed-on: https://chromium-review.googlesource.com/782661
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#518347}
Reviewed-on: https://chromium-review.googlesource.com/790610
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#571}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/c96d7df6e5c893099b4ab243e7d4f4ed8ec3d682/chrome/browser/ui/views/collected_cookies_views.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eecb525d26c7e501fbca1f65671318207f271a54

commit eecb525d26c7e501fbca1f65671318207f271a54
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Nov 29 13:26:52 2017

views: reinstate auxiliary text for collected cookies

This change introduces a third auxiliary text constant for
"clear on exit", and reworks how the collected cookies tree
drawing provider works to support annotating with different
strings.

Bug: 610428
Change-Id: Ifac295209ca7d2b1f53eed4a37fecacd70690140
Reviewed-on: https://chromium-review.googlesource.com/793790
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520092}
[modify] https://crrev.com/eecb525d26c7e501fbca1f65671318207f271a54/chrome/app/generated_resources.grd
[modify] https://crrev.com/eecb525d26c7e501fbca1f65671318207f271a54/chrome/browser/ui/views/collected_cookies_views.cc

Hi, the 'kColorId_TabTitleColorActive', 'kColorId_TabTitleColorInactive', and 'kColorId_TabBottomBorder' values also need to be added in chrome/browser/ui/libgtkui/native_theme_gtk2.cc. The build currently fails when setting use_gtk3=false:

../../chrome/browser/ui/libgtkui/native_theme_gtk2.cc:132:11: error: enumeration values 'kColorId_TabTitleColorActive', 'kColorId_TabTitleColorInactive', and 'kColorId_TabBottomBorder' not handled in switch [-Werror,-Wswitch]
  switch (color_id) {
          ^
1 error generated.

marshall@: I don't know anything about gtk2 or libgtkui so I'm probably not the right person to fix that issue - I'll cc estade@ on this bug for advice about it.

We need to find places in the gtk theme to draw reasonable colors from. +thomasanderson owns gtk theming these days.

marshall@ out of curiosity, why do you need to do a gtk2 build?

@thomasanderson: You may recall our email conversation from July :)

I build Chromium as a library [1] that's included in third-party applications. Those third-party applications use GTK2, and compiling against both GTK2 and GTK3 causes problems [2]. I don't actually need GTK support in Chromium, but I depend on the /chrome targets which pull it in. Ideally, I would want to build Chromium for Desktop Linux with use_gtk2=false and use_gtk3=false (leaving only the X11 dependency). In the mean time I'm reporting GTK2 compile failures as I find them.

[1] https://bitbucket.org/chromiumembedded/cef
[2] https://bitbucket.org/chromiumembedded/cef/issues/2014

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/695f21b231fb4764612cfc02dbb5d5598c0b0d89

commit 695f21b231fb4764612cfc02dbb5d5598c0b0d89
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Thu Jan 04 00:26:02 2018

Fix Gtk2 build

Chromium Embedded Framework [1] is a project that requires a Gtk2 build.

[1] https://bitbucket.org/chromiumembedded/cef

BUG=610428
R=erg

Change-Id: Iefecaccdc026cd5ab4bd692f15926e7410909fc7
Reviewed-on: https://chromium-review.googlesource.com/849482
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526869}
[modify] https://crrev.com/695f21b231fb4764612cfc02dbb5d5598c0b0d89/chrome/browser/ui/libgtkui/gtk_util.cc
[modify] https://crrev.com/695f21b231fb4764612cfc02dbb5d5598c0b0d89/chrome/browser/ui/libgtkui/native_theme_gtk2.cc

One last thing and I think we're good here. 

From c67: 
1. There should be 16pt between title and tabs (to match the spacing above the title)

Deleted: Screen Shot 2018-03-08 at 8.04.25 PM.png 40.5 KB

	Screen Shot 2018-03-08 at 8.04.25 PM.png 40.5 KB View Download

MacViews triage: kicking this back to bsep@ and untagging MacViews.