[WebsiteSettings] UI updates for the permissions bubble.

[WebsiteSettings] UI updates for the permissions bubble.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278649

[Greg Billock, 2014-06-18 13:43:09]

Justin, could you have a look while Susanna is gone?

[markusheintz_, 2014-06-18 14:09:54]

Drive by review (trying to make up for past delays)

LGTM

[Justin Donnelly, 2014-06-18 14:12:48]

As much as I enjoy a colorful metaphor, I worry that the CL description is
potentially confusing to readers.  Can you change it to "clean up" (if that's
what you meant by it) and maybe more explicit about it changing spacing and the
font size setting?

https://codereview.chromium.org/338853004/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
(right):

https://codereview.chromium.org/338853004/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/website_settings/permissions_bubble_view.cc:47: const
int kPermissionIndentSize = 12;
s/Size/Spacing/ for consistency with parameter names in the box layout
constructor.

https://codereview.chromium.org/338853004/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/website_settings/permissions_bubble_view.cc:190: virtual
const gfx::FontList& GetTitleFontList() const OVERRIDE;
Do you intend to use this somewhere else?

[Greg Billock, 2014-06-19 16:42:33]

https://codereview.chromium.org/338853004/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
(right):

https://codereview.chromium.org/338853004/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/website_settings/permissions_bubble_view.cc:47: const
int kPermissionIndentSize = 12;
On 2014/06/18 14:12:47, Justin Donnelly wrote:
> s/Size/Spacing/ for consistency with parameter names in the box layout
> constructor.

Done.

https://codereview.chromium.org/338853004/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/website_settings/permissions_bubble_view.cc:190: virtual
const gfx::FontList& GetTitleFontList() const OVERRIDE;
On 2014/06/18 14:12:47, Justin Donnelly wrote:
> Do you intend to use this somewhere else?

This is an override of a (new) bubble delegate method.

[Justin Donnelly, 2014-06-19 16:55:57]

lgtm

[Greg Billock, 2014-06-19 22:24:31]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-06-19 22:27:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gbillock@chromium.org/338853004/60001

[commit-bot: I haz the power, 2014-06-20 03:09:43]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-20 08:35:22]

Change committed as 278649