Get rid of some lingering MD-specific raster assets.

Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953, 610793
TBR=groby@chromium.org

Committed: https://crrev.com/a0b361e58098a446de60b2e911587f9076bd1f25
Cr-Commit-Position: refs/heads/master@{#393369}

[Evan Stade, 2016-05-10 19:30:37]

Description was changed from

==========
Get rid of some lingering MD-specific raster assets.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953
==========

to

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953
==========

[Evan Stade, 2016-05-10 19:31:07]

estade@chromium.org changed reviewers:
+ oshima@chromium.org, rdevlin.cronin@chromium.org, tdanderson@chromium.org

[Evan Stade, 2016-05-10 19:31:08]

+tdanderson, oshima for resources changes
+rdevlin.cronin for extensions changes

[Evan Stade, 2016-05-10 19:35:41]

Description was changed from

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953
==========

to

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
==========

[oshima, 2016-05-10 19:51:11]

can you look into test failures?

[Devlin, 2016-05-10 22:15:42]

On 2016/05/10 19:51:11, oshima wrote:
> can you look into test failures?

+1.

Also, there's a fair amount of chevron-y code left (even on views) - are you
aiming to kill it all here, or only enough to remove the assets?  (Either is
fine - I can clean up the rest later - but want to make sure of your intent.)

[Evan Stade, 2016-05-10 23:16:01]

On 2016/05/10 22:15:42, Devlin wrote:
> On 2016/05/10 19:51:11, oshima wrote:
> > can you look into test failures?
> 
> +1.

will do

> 
> Also, there's a fair amount of chevron-y code left (even on views) - are you
> aiming to kill it all here, or only enough to remove the assets?  (Either is
> fine - I can clean up the rest later - but want to make sure of your intent.)

what did I miss? The delegate overrides are not easy to remove without also
updating osx.

[Evan Stade, 2016-05-10 23:25:20]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-10 23:25:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971463002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971463002/20001

[Devlin, 2016-05-10 23:25:53]

On 2016/05/10 23:16:01, Evan Stade wrote:
> what did I miss? The delegate overrides are not easy to remove without also
> updating osx.
ChevronMenuButton (c/b/ui/views/toolbar/chevron_menu_button.*) is a big one, and
you can probably find a few stragglers from there.

[Evan Stade, 2016-05-10 23:53:52]

On 2016/05/10 23:25:53, Devlin wrote:
> On 2016/05/10 23:16:01, Evan Stade wrote:
> > what did I miss? The delegate overrides are not easy to remove without also
> > updating osx.
> ChevronMenuButton (c/b/ui/views/toolbar/chevron_menu_button.*) is a big one,
and
> you can probably find a few stragglers from there.

removed that one, updated some tests.

[Evan Stade, 2016-05-10 23:53:58]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-10 23:54:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971463002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971463002/40001

[Devlin, 2016-05-11 00:01:02]

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc (left):

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc:280:
EXPECT_FALSE(browser_actions_bar()->IsChevronShowing());
Instead of removing these pieces, can you do:
ToolbarActionsBar* toolbar_actions_bar =
    browser_actions_bar()->GetToolbarActionsBar();
and replace instances of 
browser_actions_bar()->IsChevronShowing()
with
toolbar_actions_bar->NeedsOverflow()
?

Do the tests pass with that?

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/chevron_menu_button.cc (left):

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/chevron_menu_button.cc:48:
view_controller()->set_icon_observer(this);
I think ExtensionActionViewController::set_icon_observer (and icon_observer_)
can be removed now.

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/chevron_menu_button.h (left):

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/chevron_menu_button.h:1: // Copyright 2014 The
Chromium Authors. All rights reserved.
Yay!

[commit-bot: I haz the power, 2016-05-11 00:38:33]

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

[commit-bot: I haz the power, 2016-05-11 00:38:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2016-05-11 21:05:35]

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc (left):

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc:280:
EXPECT_FALSE(browser_actions_bar()->IsChevronShowing());
On 2016/05/11 00:01:02, Devlin wrote:
> Instead of removing these pieces, can you do:
> ToolbarActionsBar* toolbar_actions_bar =
>     browser_actions_bar()->GetToolbarActionsBar();
> and replace instances of 
> browser_actions_bar()->IsChevronShowing()
> with
> toolbar_actions_bar->NeedsOverflow()
> ?
> 
> Do the tests pass with that?

yes

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/chevron_menu_button.cc (left):

https://codereview.chromium.org/1971463002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/chevron_menu_button.cc:48:
view_controller()->set_icon_observer(this);
On 2016/05/11 00:01:02, Devlin wrote:
> I think ExtensionActionViewController::set_icon_observer (and icon_observer_)
> can be removed now.

Done.

[Devlin, 2016-05-11 22:00:27]

lgtm

[oshima, 2016-05-11 22:02:56]

resources lgtm

[Evan Stade, 2016-05-11 22:20:27]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-05-11 22:20:45]

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

[commit-bot: I haz the power, 2016-05-11 22:28:23]

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

[commit-bot: I haz the power, 2016-05-11 22:28:25]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Evan Stade, 2016-05-12 01:20:14]

estade@chromium.org changed reviewers:
+ groby@chromium.org, msw@chromium.org, sadrul@chromium.org

[Evan Stade, 2016-05-12 01:20:15]

OWNERS:
+sadrul for ui/views/painter.cc
+msw for c/b/ui/views/ (sans extensions)
+groby for cocoa

[msw, 2016-05-12 19:01:34]

lgtm with nits/q.

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc:277: // should
overflow.
nit: 'should not', right?

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc
(right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc:156: if
(!reference_view)
nit/q: should this now also check if the view isn't visible?

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/browser_actions_container.cc (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:37: #include
"ui/base/theme_provider.h"
nit: remove

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/browser_actions_container.h (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.h:66: // it be handled
by the parent view, is so that when the the user starts
nit: fix 'the the'

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.h:208: //
ToolbarActionsBarDelegate:
q: will you cleanup this interface later? (chevron references, etc.)

[Devlin, 2016-05-12 19:03:01]

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/browser_actions_container.h (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.h:208: //
ToolbarActionsBarDelegate:
On 2016/05/12 19:01:34, msw wrote:
> q: will you cleanup this interface later? (chevron references, etc.)

Evan: feel free to punt that to me, since I'll be cleaning up the Mac side of
this.

[Evan Stade, 2016-05-12 19:15:32]

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc:277: // should
overflow.
On 2016/05/12 19:01:34, msw wrote:
> nit: 'should not', right?

Done.

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc
(right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/extensions/extension_installed_bubble_view.cc:156: if
(!reference_view)
On 2016/05/12 19:01:34, msw wrote:
> nit/q: should this now also check if the view isn't visible?

Done.

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/browser_actions_container.cc (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:37: #include
"ui/base/theme_provider.h"
On 2016/05/12 19:01:34, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
File chrome/browser/ui/views/toolbar/browser_actions_container.h (right):

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.h:66: // it be handled
by the parent view, is so that when the the user starts
On 2016/05/12 19:01:34, msw wrote:
> nit: fix 'the the'

Done.

https://codereview.chromium.org/1971463002/diff/60001/chrome/browser/ui/views...
chrome/browser/ui/views/toolbar/browser_actions_container.h:208: //
ToolbarActionsBarDelegate:
On 2016/05/12 19:03:01, Devlin wrote:
> On 2016/05/12 19:01:34, msw wrote:
> > q: will you cleanup this interface later? (chevron references, etc.)
> 
> Evan: feel free to punt that to me, since I'll be cleaning up the Mac side of
> this.

yea, the plan was to make it part of the mac cleanup that Devlin has in his
queue.

[Evan Stade, 2016-05-12 19:15:50]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-12 19:16:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1971463002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1971463002/80001

[commit-bot: I haz the power, 2016-05-12 20:05:05]

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

[commit-bot: I haz the power, 2016-05-12 20:05:06]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-05-12 21:45:25]

Description was changed from

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
==========

to

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
TBR=groby@chromium.org
==========

[Evan Stade, 2016-05-12 21:45:35]

groby to tbr

[Evan Stade, 2016-05-12 21:45:46]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-05-12 21:45:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, oshima@chromium.org, msw@chromium.org
Link to the patchset: https://codereview.chromium.org/1971463002/#ps80001
(title: "msw review")

[commit-bot: I haz the power, 2016-05-12 21:46:23]

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

[commit-bot: I haz the power, 2016-05-12 21:55:08]

Description was changed from

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
TBR=groby@chromium.org
==========

to

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
TBR=groby@chromium.org
==========

[commit-bot: I haz the power, 2016-05-12 21:55:10]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-12 21:56:19]

Description was changed from

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
TBR=groby@chromium.org
==========

to

==========
Get rid of some lingering MD-specific raster assets.

Part of this is deleting obsolete bookmark action bar chevron code on Views.

Only pdf.png remains (on Views at least) and I've pinged sgabriel/bettes
for a replacement.

BUG=505953,610793
TBR=groby@chromium.org

Committed: https://crrev.com/a0b361e58098a446de60b2e911587f9076bd1f25
Cr-Commit-Position: refs/heads/master@{#393369}
==========

[commit-bot: I haz the power, 2016-05-12 21:56:21]

Patchset 5 (id:??) landed as
https://crrev.com/a0b361e58098a446de60b2e911587f9076bd1f25
Cr-Commit-Position: refs/heads/master@{#393369}

[groby-ooo-7-16, 2016-05-12 22:22:58]

LGTM ex post facto