This includes changes to the highlighting of the extension icon. Current screenshot attached: it's too wide and the opaque yellow at the top looks like a bug (fine to fix as part of harmony work and not before).

Deleted: unnamed.png 111 KB

	unnamed.png 111 KB View Download

Unassigning my Harmony bugs pending re-triage of who should own what.

This is how it will look on Mac after https://codereview.chromium.org/2630473003/

On Mac, it's currently super-wide. This is because it's sized to "55 characters" (IDS_EXTENSION_TOOLBAR_REDESIGN_NOTIFICATION_BUBBLE_WIDTH_CHARS) according to

int Widget::GetLocalizedContentsWidth(int col_resource_id) {
  return ui::GetLocalizedContentsWidthForFont(
      col_resource_id, ResourceBundle::GetSharedInstance().GetFontWithDelta(
                           ui::kMessageFontSizeDelta));
}

kMessageFontSizeDelta gets a 13pt font on Mac - too big - so the whole thing is wide.


Fix for that is to use LayoutDelegate::GetDialogPreferredWidth() instead, which can probably be the responsibility of DialogClientView -- (this bubble will instead not specify a preferred width, and DialogClientView can just round up).

The other thing missing is the default button. This was removed as part of  Issue 621122 . It needs to be added back on a case-by-case basis for these dialogs.

Also the typography needs to be fixed. That's  Issue 691891  and is next in my queue.

So "more screenshots coming later" :)

Deleted: Screen Shot 2017-03-07 at 3.10.57 pm.png 118 KB

	Screen Shot 2017-03-07 at 3.10.57 pm.png 118 KB View Download

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

commit 0400d0a14218e82b41e2be64fb3174e70352b4e7
Author: tapted <tapted@chromium.org>
Date: Mon Mar 13 06:09:17 2017

MacViews: Harmony for toolbar actions bubbles.

These are used for
 - DevModeBubbleDelegate
 - NtpOverriddenBubbleDelegate
 - ProxyOverriddenBubbleDelegate
 - SettingsApiBubbleDelegate
 - SuspiciousExtensionBubbleDelegate
 - BlockedActionBubbleDelegate

Adds a ToolbarActionsBarBubbleViewsPresenter for showing the
toolkit-views bubble on a Cocoa browser. It observes the Views bubble
and notifies the Cocoa BrowserActionsController when it closes.

One issue: the Cocoa bubble uses a `BOOL anchoredToAction;` data member
whereas the Views bubble inspects the anchor_view's ID. The latter
doesn't work for a Cocoa browser, so make these consistent by adding the
same data member to the views bubble.

Adds a TestBrowserDialog case for showing the DevModeBubble with Views
on all desktop platforms. These need to be added for the other bubble
types.

Brings up all the views-based toolbar action bubble tests on Mac by
factoring out two functions that depend on whether the Browser window is
Cocoa or toolkit-views.

Screenshot at  http://crbug.com/654128#c5  (it's currently too wide under
Harmony on Mac, but there are framework fixes coming for that).

The interactive bubble test can be invoked with `browser_tests
--gtest_filter=BrowserDialogTest.Invoke --interactive
--dialog=ExtensionMessageBubbleViewBrowserTest.InvokeDialog_devmode_warning`

BUG= 654128 ,  654126 ,  654121 

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

[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/cocoa/extensions/browser_actions_controller.h
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm
[add] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/cocoa/extensions/extension_message_bubble_views_browsertest_mac.mm
[add] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_views_presenter.h
[add] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_views_presenter.mm
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/extensions/extension_message_bubble_browsertest.h
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/test/test_browser_dialog.cc
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc
[add] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest_nonmac.cc
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/views/toolbar/browser_actions_container.cc
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc
[modify] https://crrev.com/0400d0a14218e82b41e2be64fb3174e70352b4e7/chrome/test/BUILD.gn

 Issue 681201  has been merged into this issue.

This dialog is still super wide, and I don't think it's because of the message font size. The dialog is sizing itself using views::Widget::GetLocalizedContentsWidth(55), which is returning 362 on my Linux workstation and 605 on my Mac. In particular, digging into PlatformFontLinux, that thinks the average width of a glyph is around 7px, while PlatformFontMac thinks average_width_ is 11. I think the "average" width is 11 because we're using -[NSFont boundingRectForGlyph:[font glyphWithName:@"x"]], but experimentally, we get back 11 for any glyph I supply (even very narrow ones like "i"), which suggests that -[NSFont boundingRectForGlyph:] is very conservative about what it returns.

I don't think that NSFont exposes a notion of an average width. We can have a guess using NSAttributedString and some sample set of characters - I tried it with the alphabet in lowercase and got back 374, which is a not unreasonable value, and certainly the dialog looks better this way than 605.

tapted@, what do you think? I've put up a CL if you want to take a look: https://codereview.chromium.org/2839873003/

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

commit 3ff0361da27135ec64b787f2630af4ae960399b0
Author: ellyjones <ellyjones@chromium.org>
Date: Thu May 04 15:06:12 2017

PlatformFontMac: better guess for average width

PlatformFontMac::GetExpectedTextWidth() was wildly over-estimating text widths,
causing dialogs that try to size using Widget::GetLocalizedContentsWidth to be
extremely wide on Mac. This change replaces the guess in PlatformFontMac's
CalculateMetricsAndInitRenderParams() with a more empirically accurate guess.

BUG= 654128 

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

[modify] https://crrev.com/3ff0361da27135ec64b787f2630af4ae960399b0/ui/gfx/platform_font_mac.h
[modify] https://crrev.com/3ff0361da27135ec64b787f2630af4ae960399b0/ui/gfx/platform_font_mac.mm

 Issue 654121  has been merged into this issue.

 Issue 654126  has been merged into this issue.

 Issue 766296  has been merged into this issue.

The NextAction date has arrived: 2017-11-10

Load balancing

Current screenshot attached.

Deleted: Untitled.png 11.5 KB

	Untitled.png 11.5 KB View Download

The most obvious things that stick out to me are the use of "Learn more" instead of "(?)", the width of the dialog, and the highlight shape and color (shape may be an artifact of the test, color may be intentional).

Oh, also, since this is a popover, I think it should have a close X?

I think this overlaps (maybe a lot) with issue 671656, which is currently marked for review.

Yeah, these look like the same bug.  Duping against the one where you've been actively doing work.