Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and add an
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
Review-Url: https://codereview.chromium.org/2916753004
Cr-Commit-Position: refs/heads/master@{#477871}
Committed: https://chromium.googlesource.com/chromium/src/+/712c1b8ba3219ffe2408837d7b2f2f1253a55f26

[tapted, 2017-06-02 06:34:07]

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

[commit-bot: I haz the power, 2017-06-02 06:34:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-06-02 06:43:39]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac.

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be show interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

[tapted, 2017-06-02 06:43:40]

tapted@chromium.org changed reviewers:
+ benwells@chromium.org

[tapted, 2017-06-02 06:45:03]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac.

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be show interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be show interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

[tapted, 2017-06-02 06:47:30]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be show interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be show interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

[commit-bot: I haz the power, 2017-06-02 08:26:08]

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

[commit-bot: I haz the power, 2017-06-02 08:26:09]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-06-05 00:06:26]

Hiya Ben, please take a look

[benwells, 2017-06-06 03:31:50]

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/bookmark_app_helper_browsertest.cc (right):

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/bookmark_app_helper_browsertest.cc:82:
base::Unretained(this)));
Why unretained?

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/test_bookmark_app_helper.h (right):

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/test_bookmark_app_helper.h:13: class
TestBookmarkAppHelper : public extensions::BookmarkAppHelper {
What's the point of this class?

[tapted, 2017-06-06 07:17:26]

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

[commit-bot: I haz the power, 2017-06-06 07:17:38]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-06-06 07:44:13]

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/bookmark_app_helper_browsertest.cc (right):

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/bookmark_app_helper_browsertest.cc:82:
base::Unretained(this)));
On 2017/06/06 03:31:50, benwells wrote:
> Why unretained?

Good point :). I forgot this was scoped_refptr. So `this` works.

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/test_bookmark_app_helper.h (right):

https://codereview.chromium.org/2916753004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/test_bookmark_app_helper.h:13: class
TestBookmarkAppHelper : public extensions::BookmarkAppHelper {
On 2017/06/06 03:31:50, benwells wrote:
> What's the point of this class?

Added a comment.

(OnDidGetManifest and OnIconsDownloaded are private in BookmarkAppHelper)

[commit-bot: I haz the power, 2017-06-06 08:48:32]

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

[commit-bot: I haz the power, 2017-06-06 08:48:33]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-06-07 01:31:39]

lgtm

[tapted, 2017-06-07 05:31:13]

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

[commit-bot: I haz the power, 2017-06-07 05:31:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-06-07 06:55:17]

On 2017/06/07 01:31:39, benwells wrote:
> lgtm

Removed the friend, as discussed. Too easy.

[tapted, 2017-06-07 06:58:15]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be show interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

[tapted, 2017-06-07 06:59:02]

tapted@chromium.org changed reviewers:
+ sky@chromium.org

[tapted, 2017-06-07 06:59:03]

+sky for OWNERS in
    chrome/browser/ui/browser_dialogs.h
    chrome/browser/ui/browser_window.h
    chrome/browser/ui/views/frame/browser_view.cc
    chrome/browser/ui/views/frame/browser_view.h
    chrome/test/base/test_browser_window.h

Thanks!

[commit-bot: I haz the power, 2017-06-07 07:10:50]

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

[commit-bot: I haz the power, 2017-06-07 07:10:52]

Dry run: This issue passed the CQ dry run.

[sky, 2017-06-07 16:27:43]

LGTM

https://codereview.chromium.org/2916753004/diff/60001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_dialogs.h (right):

https://codereview.chromium.org/2916753004/diff/60001/chrome/browser/ui/brows...
chrome/browser/ui/browser_dialogs.h:109: typedef base::Callback<void(bool, const
WebApplicationInfo&)>
typedef->using and OnceCallback.

[tapted, 2017-06-07 23:47:56]

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

[commit-bot: I haz the power, 2017-06-07 23:48:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-06-08 00:25:57]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and and
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and add
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

[tapted, 2017-06-08 00:26:09]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and add
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and add an
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

[tapted, 2017-06-08 01:52:31]

https://codereview.chromium.org/2916753004/diff/60001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_dialogs.h (right):

https://codereview.chromium.org/2916753004/diff/60001/chrome/browser/ui/brows...
chrome/browser/ui/browser_dialogs.h:109: typedef base::Callback<void(bool, const
WebApplicationInfo&)>
On 2017/06/07 16:27:43, sky wrote:
> typedef->using and OnceCallback.

Done.

[tapted, 2017-06-08 02:33:47]

The CQ bit was unchecked by tapted@chromium.org

[tapted, 2017-06-08 02:33:55]

The CQ bit was checked by tapted@chromium.org

[tapted, 2017-06-08 02:33:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2916753004/#ps80001
(title: "using, OnceCallback")

[commit-bot: I haz the power, 2017-06-08 02:34:20]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 03:10:12]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496889235171230,
"parent_rev": "aa2a07d8232fba92dadf65375d6327adce7a9a28", "commit_rev":
"712c1b8ba3219ffe2408837d7b2f2f1253a55f26"}

[commit-bot: I haz the power, 2017-06-08 03:10:21]

Description was changed from

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and add an
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153
==========

to

==========
Use BookmarkAppConfirmationView on Mac. Delete the NSAlert Mac uses currently.

This dialog can only be shown on Mac by turning on a flag. The NSAlert
is ugly and buggy, so just delete it.

Remove a bunch of unnecessary plumbing. Renames ShowBookmarkAppBubble to
ShowBookmarkAppDialog (since it's not a bubble).

Adds a test harness for BookmarkAppHelper integration tests and add an
end-to-end test for showing the dialog.

Adds BookmarkAppHelperTest.InvokeDialog_create to browser_tests. The
dialog can be shown interactively with something like

browser_tests --gtest_filter=BrowserDialogTest.Invoke --interactive \
--dialog=BookmarkAppHelperTest.InvokeDialog_create

BUG=728153

Review-Url: https://codereview.chromium.org/2916753004
Cr-Commit-Position: refs/heads/master@{#477871}
Committed:
https://chromium.googlesource.com/chromium/src/+/712c1b8ba3219ffe2408837d7b2f...
==========

[commit-bot: I haz the power, 2017-06-08 03:10:23]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/712c1b8ba3219ffe2408837d7b2f...