Convert location bar decorations to Material Design (Mac).

Convert location bar decorations to Material Design (Mac).

BUG=588152
Committed: https://crrev.com/0a107cab0c81072822eb517e569641f7eb15d6e7
Cr-Commit-Position: refs/heads/master@{#381567}

[shrike, 2016-03-15 00:15:37]

Description was changed from

==========
(WIP) Convert location bar decorations to Material Design.

BUG=588152
==========

to

==========
Convert location bar decorations to Material Design (Mac).

BUG=588152
==========

[shrike, 2016-03-15 00:15:37]

shrike@chromium.org changed reviewers:
+ avi@chromium.org, cpu@chromium.org

[shrike, 2016-03-15 00:16:10]

Hello,

PTAL:

cpu@ -
  components/omnibox/browser/autocomplete_match.cc
  components/omnibox/browser/omnibox_view.cc
  components/toolbar/toolbar_model_impl.cc

avi@ - everything else

[Avi (use Gerrit), 2016-03-15 16:03:06]

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm:350: //
trivial bit of refactoring.
Maybe in a followup CL... :)

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/save_credit_card_decoration.mm
(right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/save_credit_card_decoration.mm:26: if
(icon_initialized_)
What happens if they change themes? This bool locks out any change in decoration
with a difference in the bool parameter.

Now that I realize it, do you test these CL with a theme change to make sure
everything gets redrawn properly?

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/translate_decoration.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/translate_decoration.mm:30: NSImage
*theImage =
space after the *

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm:111: NSImage *theImage =
space after the *

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:613: // ALlow the
|locationBarView_| to update itself to match the browser window
s/ALlow/Allow/

[shrike, 2016-03-15 17:54:56]

PTAL

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/content_setting_decoration.mm:350: //
trivial bit of refactoring.
On 2016/03/15 16:03:06, Avi wrote:
> Maybe in a followup CL... :)

https://crbug.com/595014

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/save_credit_card_decoration.mm
(right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/save_credit_card_decoration.mm:26: if
(icon_initialized_)
On 2016/03/15 16:03:06, Avi wrote:
> What happens if they change themes? This bool locks out any change in
decoration
> with a difference in the bool parameter.
> 
> Now that I realize it, do you test these CL with a theme change to make sure
> everything gets redrawn properly?

Thank you for catching this.

I have been testing switching themes with my changes. I don't think I tested the
decorations with theme changes because the location bar does not change color to
match the custom theme color (i.e. it will have a white background in a light
theme or a dark theme). So theoretically the decorations only need to be
configured for their initial theme darkness. However your question had led me to
realize that if I set a custom theme, create an Incognito window (the location
bar will be white because it's a custom theme), and then reset to the default
theme, the location bar is still white in the Incognito window. So right now the
location bar has the assumption that it need never change color once initially
configured. I need to fix that bug (in a separate cl) and from there see if the
decorations have problems as well.

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/translate_decoration.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/translate_decoration.mm:30: NSImage
*theImage =
On 2016/03/15 16:03:06, Avi wrote:
> space after the *

Thank you.

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/zoom_decoration.mm:111: NSImage *theImage =
On 2016/03/15 16:03:06, Avi wrote:
> space after the *

Done.

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm (right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/toolbar/toolbar_controller.mm:613: // ALlow the
|locationBarView_| to update itself to match the browser window
On 2016/03/15 16:03:06, Avi wrote:
> s/ALlow/Allow/

Thanks.

[Avi (use Gerrit), 2016-03-15 20:01:33]

lgtm

👍

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/location_bar/save_credit_card_decoration.mm
(right):

https://codereview.chromium.org/1718563002/diff/120001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/location_bar/save_credit_card_decoration.mm:26: if
(icon_initialized_)
On 2016/03/15 17:54:56, shrike wrote:
> On 2016/03/15 16:03:06, Avi wrote:
> > What happens if they change themes? This bool locks out any change in
> decoration
> > with a difference in the bool parameter.
> > 
> > Now that I realize it, do you test these CL with a theme change to make sure
> > everything gets redrawn properly?
> 
> Thank you for catching this.
> 
> I have been testing switching themes with my changes. I don't think I tested
the
> decorations with theme changes because the location bar does not change color
to
> match the custom theme color (i.e. it will have a white background in a light
> theme or a dark theme). So theoretically the decorations only need to be
> configured for their initial theme darkness. However your question had led me
to
> realize that if I set a custom theme, create an Incognito window (the location
> bar will be white because it's a custom theme), and then reset to the default
> theme, the location bar is still white in the Incognito window. So right now
the
> location bar has the assumption that it need never change color once initially
> configured. I need to fix that bug (in a separate cl) and from there see if
the
> decorations have problems as well.

Thanks for checking.

[cpu_(ooo_6.6-7.5), 2016-03-16 02:05:27]

lgtm

[shrike, 2016-03-16 16:58:18]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2016-03-16 16:58:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1718563002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1718563002/140001

[commit-bot: I haz the power, 2016-03-16 17:10:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-16 17:10:21]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[shrike, 2016-03-16 17:27:28]

The CQ bit was checked by shrike@chromium.org

[shrike, 2016-03-16 17:27:28]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
cpu@chromium.org
Link to the patchset: https://codereview.chromium.org/1718563002/#ps160001
(title: "Fix unit test.")

[commit-bot: I haz the power, 2016-03-16 17:27:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1718563002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1718563002/160001

[commit-bot: I haz the power, 2016-03-16 18:00:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-16 18:00:45]

Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[shrike, 2016-03-16 18:09:00]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2016-03-16 18:09:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1718563002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1718563002/160001

[commit-bot: I haz the power, 2016-03-16 19:00:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-16 19:00:52]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[shrike, 2016-03-16 21:24:50]

The CQ bit was checked by shrike@chromium.org

[shrike, 2016-03-16 21:24:51]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
cpu@chromium.org
Link to the patchset: https://codereview.chromium.org/1718563002/#ps180001
(title: "Fix issue from browser test.")

[commit-bot: I haz the power, 2016-03-16 21:25:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1718563002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1718563002/180001

[commit-bot: I haz the power, 2016-03-16 22:21:44]

Description was changed from

==========
Convert location bar decorations to Material Design (Mac).

BUG=588152
==========

to

==========
Convert location bar decorations to Material Design (Mac).

BUG=588152
==========

[commit-bot: I haz the power, 2016-03-16 22:21:46]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-03-16 22:27:03]

Description was changed from

==========
Convert location bar decorations to Material Design (Mac).

BUG=588152
==========

to

==========
Convert location bar decorations to Material Design (Mac).

BUG=588152

Committed: https://crrev.com/0a107cab0c81072822eb517e569641f7eb15d6e7
Cr-Commit-Position: refs/heads/master@{#381567}
==========

[commit-bot: I haz the power, 2016-03-16 22:27:04]

Patchset 10 (id:??) landed as
https://crrev.com/0a107cab0c81072822eb517e569641f7eb15d6e7
Cr-Commit-Position: refs/heads/master@{#381567}