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

commit 5763588fd12942f8829b5c21300ad094bb0e5203
Author: ellyjones <ellyjones@chromium.org>
Date: Wed May 25 10:03:35 2016

MacViews: support Views permission bubble

The existing PermissionBubbleViewViews class needs an anchor view or point, so
this CL adds some machinery to pass an anchor point down from the Cocoa layer,
and adds a delegate so platforms can produce their own anchors.

BUG= 605667 

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

[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm
[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.h
[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/cocoa/website_settings/permission_bubble_controller_unittest.mm
[add] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/cocoa/website_settings/permission_bubble_view_cocoa_views.mm
[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/views/website_settings/permissions_bubble_view.h
[add] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/browser/ui/views/website_settings/permissions_bubble_view_views.cc
[modify] https://crrev.com/5763588fd12942f8829b5c21300ad094bb0e5203/chrome/chrome_browser_ui.gypi

Attaching current state at r409749

Deleted: Screen Shot 2016-08-04 at 22.33.31.png 39.6 KB

	Screen Shot 2016-08-04 at 22.33.31.png 39.6 KB View Download

As of r417913

Deleted: Screen Shot 2016-09-12 at 8.44.39 AM.png 14.5 KB

	Screen Shot 2016-09-12 at 8.44.39 AM.png 14.5 KB View Download

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

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-HTTP%20auto.png%3Fz=width

Current state.

Deleted: Screen Shot 2016-10-11 at 12.58.52 PM.png 14.2 KB

	Screen Shot 2016-10-11 at 12.58.52 PM.png 14.2 KB View Download

This is the correct spec:

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-Permission.png%3Fz=width


Initial comments:

* Buttons should be 16pt from bottom and right edge of dialog
* Distance between buttons should be 16pt
* Title string should not end in a colon
* Should be about 24pt from bottom of the bell icon to top of buttons
* Icon should be left-aligned with dialog title
* Dialog title should be 16pt from left edge of dialog
* Double-check distance between dialog title and icon/message
* Dialog should be 320pt wide, 132pt tall

https://codereview.chromium.org/2654123004/ uploaded.
#8:

* Buttons from bottom/right: fixed as of the bubble layout fixes
* Distance between buttons: the distance between the buttons looks like 8pt in the linked spec? They are Related, I think, so it should be 8pt.
* Title string with colon: in the linked CL
* 24pt from bottom of bell icon to top of buttons: how come this is 24 instead of 16? I would have expected the usual unrelated control spacing.
* Icon left-aligned with dialog title: in the linked CL
* Dialog title 16pt from left edge: fixed as of the bubble layout fixes
* Dialog title -> icon: looks like 16pt already to me, although it looks a couple pts bigger because of a little bit of whitespace on the icon image
* 320pt x 132pt: the width was set to 320 in https://codereview.chromium.org/1671143002 but that looks like a hack; a proper fix to bubble width still needs to be done.

Can you attach a screenshot?

As of that CL.

Deleted: Screen Shot 2017-01-30 at 3.43.55 PM.png 11.9 KB

	Screen Shot 2017-01-30 at 3.43.55 PM.png 11.9 KB View Download

Thank you. Seems like it's about done. Only change is maybe increase the distance between the bottom of the "Show notifications" text and the buttons (I think that should generally be 16pt). This change would of course make the dialog slightly taller.

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

commit 2e942b323369f872f50f42cf5623f3265470861d
Author: ellyjones <ellyjones@chromium.org>
Date: Fri Feb 03 00:00:16 2017

harmony: partly harmonize permissions bubble

This change:
1) Changes the title string to not include the trailing colon (this affects the
   non-Harmony version too)
2) Switches from 9px to 8px for the spacing between items (affects non-Harmony)
3) Makes the indent on permission entries conditional on non-Harmony

BUG= 605667 

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

[modify] https://crrev.com/2e942b323369f872f50f42cf5623f3265470861d/chrome/app/generated_resources.grd
[modify] https://crrev.com/2e942b323369f872f50f42cf5623f3265470861d/chrome/browser/ui/views/harmony/harmony_layout_delegate.cc
[modify] https://crrev.com/2e942b323369f872f50f42cf5623f3265470861d/chrome/browser/ui/views/harmony/layout_delegate.cc
[modify] https://crrev.com/2e942b323369f872f50f42cf5623f3265470861d/chrome/browser/ui/views/harmony/layout_delegate.h
[modify] https://crrev.com/2e942b323369f872f50f42cf5623f3265470861d/chrome/browser/ui/views/website_settings/permission_prompt_impl.cc

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

commit 38424d53080c397f8cc13c6581fe937070922ac0
Author: tapted <tapted@chromium.org>
Date: Thu Mar 16 01:16:45 2017

Add "interactive" UI integration tests for PermissionRequestManager.

Existing PermissionRequestManager tests use a
MockPermissionPromptFactory which replaces the code that generates
actual UI with stubs. To get coverage of the UI creation code, and to
allow the set of permission prompts to be easily "invoked" for UI
review, extend PermissionRequestManagerBrowserTest with one that uses
"real" UI rather than a mock.

Then, add "Dialog" BrowserTests for a range of permission types that can
be requested. These are run on the bots or (optionally) interactively
with a command such as `browser_tests
--gtest_filter=BrowserDialogTest.Invoke --interactive
--dialog=PermissionDialogTest.InvokeDialog_multiple`

BUG= 605667 , 663935

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

[modify] https://crrev.com/38424d53080c397f8cc13c6581fe937070922ac0/chrome/browser/media/webrtc/media_stream_devices_controller.h
[modify] https://crrev.com/38424d53080c397f8cc13c6581fe937070922ac0/chrome/browser/permissions/permission_request_manager_browsertest.cc

Compared to the mock,
 - title text font size seems to have been overridden to be small

And other discrepancies that just be due to the mock being old
 - buttons are slightly narrower than the mock
 - icon is different
 - close [x] looks different


Note we swap the order of Allow/Block on Mac.

Deleted: Screen Shot 2017-08-09 at 12.10.22 pm.png 31.9 KB

	Screen Shot 2017-08-09 at 12.10.22 pm.png 31.9 KB View Download

Thanks tapted@ - Is this ready for a UX review? Or are more changes required?

https://chromium-review.googlesource.com/c/chromium/src/+/667320

This CL fixes the title size - I don't see any other differences from the mock, so this is probably ready for review. See the attached screenshot (taken on Mac, so the button order is swapped).

Deleted: Screen Shot 2017-09-14 at 10.43.27 AM.png 26.8 KB

	Screen Shot 2017-09-14 at 10.43.27 AM.png 26.8 KB View Download

the pre-Harmony bubble with the normal title font

Deleted: views-new.png 15.5 KB

	views-new.png 15.5 KB View Download

elly - that one might belong under the class of dialogs in  http://crbug.com/700196  . Although I somehow missed it when hunting for instances of that UI. I *hope* it's sharing the same bubble class, since it looks pretty similar to the other content settings bubbles!

(basically, if it can be shown by the dialog test in cs.chromium.org/chrome/browser/ui/views/location_bar/content_setting_bubble_dialog_browsertest.cc then it probably belongs on  Issue 700196 , unless it has its own bug already)

Hm, yeah, this does actually belong in 700196. I'll dupe it.

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

commit c733e7534eed37af87b9ae18eeda784e673f4abc
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Sep 20 18:09:10 2017

views: use title font for permission bubble title in harmony mode

Bug:  605667 
Change-Id: Ie98cdba0eff81d492d1ab069a1e39896bdde64fd
Reviewed-on: https://chromium-review.googlesource.com/667320
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503198}
[modify] https://crrev.com/c733e7534eed37af87b9ae18eeda784e673f4abc/chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

The NextAction date has arrived: 2017-09-29