Adding a confirmation dialog for It2Me on Mac

Adding a confirmation dialog for It2Me on Mac

This change adds a confirmation dialog for the It2Me Mac host.  This
dialog will be displayed when an incoming It2Me connection and will wait
up to 60 seconds for the user to accept/reject it.  If the user accepts
the connection then we will complete the conneciton process, otherwise
the incoming request will be rejected.

BUG=645540
Committed: https://crrev.com/873700139f1834cd79da404cc4ceeb5c264b7703
Cr-Commit-Position: refs/heads/master@{#419676}

[joedow, 2016-09-15 21:47:20]

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

[commit-bot: I haz the power, 2016-09-15 21:48:10]

Dry run: 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, 2016-09-15 22:57:52]

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

[commit-bot: I haz the power, 2016-09-15 22:57:53]

Dry run: This issue passed the CQ dry run.

[joedow, 2016-09-15 23:06:09]

joedow@chromium.org changed reviewers:
+ sergeyu@chromium.org

[joedow, 2016-09-15 23:06:09]

PTAL!

[Sergey Ulanov, 2016-09-15 23:48:42]

https://codereview.chromium.org/2344003002/diff/1/remoting/host/it2me/it2me_c...
File remoting/host/it2me/it2me_confirmation_dialog_mac.mm (right):

https://codereview.chromium.org/2344003002/diff/1/remoting/host/it2me/it2me_c...
remoting/host/it2me/it2me_confirmation_dialog_mac.mm:63: ResultCallback
result_callback_;
result_callback_ is stored here and also in
It2MeConfirmationDialogMacController. The problem is that it's hard to guarantee
that it's called only once if you store it in two different places. Maybe remove
it from here and call the callback in  [It2MeConfirmationDialogMacController
hide] instead of  It2MeConfirmationDialogMac::OnDialogAction()?

https://codereview.chromium.org/2344003002/diff/1/remoting/host/it2me/it2me_c...
remoting/host/it2me/it2me_confirmation_dialog_mac.mm:99: void
It2MeConfirmationDialogMac::OnDialogAction(Result result) {
call this OnTimerExpired to make it clear that this is used only for expired
timer.
Also result is always Result::CANCEL, so you don't need the argument

[joedow, 2016-09-16 00:21:42]

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

[commit-bot: I haz the power, 2016-09-16 00:22:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[joedow, 2016-09-16 00:23:00]

Addressed feedback, PTAL!

https://codereview.chromium.org/2344003002/diff/1/remoting/host/it2me/it2me_c...
File remoting/host/it2me/it2me_confirmation_dialog_mac.mm (right):

https://codereview.chromium.org/2344003002/diff/1/remoting/host/it2me/it2me_c...
remoting/host/it2me/it2me_confirmation_dialog_mac.mm:63: ResultCallback
result_callback_;
On 2016/09/15 23:48:41, Sergey Ulanov wrote:
> result_callback_ is stored here and also in
> It2MeConfirmationDialogMacController. The problem is that it's hard to
guarantee
> that it's called only once if you store it in two different places. Maybe
remove
> it from here and call the callback in  [It2MeConfirmationDialogMacController
> hide] instead of  It2MeConfirmationDialogMac::OnDialogAction()?

They are different callbacks (though they share the same name).  I will rename
the callback in the controller to make it more obvious.

I wanted to keep the actual result callback reference in this class as it
simplified the timer code.  Otherwise I'd agree that I could just pass the
callback reference on to the controller class.

https://codereview.chromium.org/2344003002/diff/1/remoting/host/it2me/it2me_c...
remoting/host/it2me/it2me_confirmation_dialog_mac.mm:99: void
It2MeConfirmationDialogMac::OnDialogAction(Result result) {
On 2016/09/15 23:48:42, Sergey Ulanov wrote:
> call this OnTimerExpired to make it clear that this is used only for expired
> timer.
> Also result is always Result::CANCEL, so you don't need the argument

This method is bound and passed to the controller and is called back when a
button is pressed.  I think the callback name I chose is causing some confusion
so I will update that in my next revision.

[commit-bot: I haz the power, 2016-09-16 01:48:40]

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

[commit-bot: I haz the power, 2016-09-16 01:48:41]

Dry run: This issue passed the CQ dry run.

[joedow, 2016-09-19 17:03:00]

Ping!

[Sergey Ulanov, 2016-09-19 21:16:26]

lgtm

[joedow, 2016-09-19 21:16:43]

The CQ bit was checked by joedow@chromium.org

[commit-bot: I haz the power, 2016-09-19 21:17:38]

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, 2016-09-20 00:39:35]

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

[commit-bot: I haz the power, 2016-09-20 00:39:36]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[joedow, 2016-09-20 03:22:15]

The CQ bit was checked by joedow@chromium.org

[commit-bot: I haz the power, 2016-09-20 03:22:34]

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, 2016-09-20 04:09:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-20 04:11:49]

Description was changed from

==========
Adding a confirmation dialog for It2Me on Mac

This change adds a confirmation dialog for the It2Me Mac host.  This
dialog will be displayed when an incoming It2Me connection and will wait
up to 60 seconds for the user to accept/reject it.  If the user accepts
the connection then we will complete the conneciton process, otherwise
the incoming request will be rejected.

BUG=645540
==========

to

==========
Adding a confirmation dialog for It2Me on Mac

This change adds a confirmation dialog for the It2Me Mac host.  This
dialog will be displayed when an incoming It2Me connection and will wait
up to 60 seconds for the user to accept/reject it.  If the user accepts
the connection then we will complete the conneciton process, otherwise
the incoming request will be rejected.

BUG=645540

Committed: https://crrev.com/873700139f1834cd79da404cc4ceeb5c264b7703
Cr-Commit-Position: refs/heads/master@{#419676}
==========

[commit-bot: I haz the power, 2016-09-20 04:11:50]

Patchset 2 (id:??) landed as
https://crrev.com/873700139f1834cd79da404cc4ceeb5c264b7703
Cr-Commit-Position: refs/heads/master@{#419676}