Removing It2MeConfirmationDialogFactory

Removing It2MeConfirmationDialogFactory

Since the It2MeConfirmationDialog is now supported on all platforms, we can
remove the factory used to create dialog instances and instead use a static
method on the class itself.

BUG=649016
Committed: https://crrev.com/7fcc469912720fdcbc26429b850666778a4ccbb7
Cr-Commit-Position: refs/heads/master@{#421333}

[joedow, 2016-09-26 22:25:52]

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

[commit-bot: I haz the power, 2016-09-26 22:26:46]

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-26 22:41:04]

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

[commit-bot: I haz the power, 2016-09-26 22:41:16]

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-26 23:53:28]

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

[commit-bot: I haz the power, 2016-09-26 23:53:28]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[joedow, 2016-09-27 01:12:35]

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

[joedow, 2016-09-27 01:12:36]

PTAL!

[joedow, 2016-09-27 14:30:52]

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

[commit-bot: I haz the power, 2016-09-27 14:31:08]

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-27 14:32:48]

Updating some formatting.

[commit-bot: I haz the power, 2016-09-27 16:33:23]

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

[commit-bot: I haz the power, 2016-09-27 16:33:24]

Dry run: This issue passed the CQ dry run.

[Sergey Ulanov, 2016-09-27 17:48:41]

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
File remoting/host/it2me/it2me_confirmation_dialog_chromeos.cc (right):

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_confirmation_dialog_chromeos.cc:71:
std::unique_ptr<It2MeConfirmationDialog> It2MeConfirmationDialog::Create() {
add "// static" on the line above to make it clear that this is a static
function. Same in other files that define this function.

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
File remoting/host/it2me/it2me_host.cc (right):

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_host.cc:519: host_context_->ui_task_runner(),
std::move(confirmation_dialog_)));
It2MeConfirmationDialogProxy creation can be moved to the constructor - this
would allow to remove confirmation_dialog_.

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_host.cc:519: host_context_->ui_task_runner(),
std::move(confirmation_dialog_)));
not related to this CL:
Constructor initializes task_runner_ = host_context_->ui_task_runner(),
but then host_context_->ui_task_runner() is still used in some places. While you
are here maybe remove |task_runner_| or use it consistently throughout this
class?
(I'd prefer removing task_runner_)

[Sergey Ulanov, 2016-09-27 17:48:57]

lgtm after my comments are addressed. Thanks for the cleanup!

[joedow, 2016-09-27 20:05:38]

Thanks!

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
File remoting/host/it2me/it2me_confirmation_dialog_chromeos.cc (right):

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_confirmation_dialog_chromeos.cc:71:
std::unique_ptr<It2MeConfirmationDialog> It2MeConfirmationDialog::Create() {
On 2016/09/27 17:48:40, Sergey Ulanov wrote:
> add "// static" on the line above to make it clear that this is a static
> function. Same in other files that define this function.

Done.

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
File remoting/host/it2me/it2me_host.cc (right):

https://codereview.chromium.org/2372833002/diff/40001/remoting/host/it2me/it2...
remoting/host/it2me/it2me_host.cc:519: host_context_->ui_task_runner(),
std::move(confirmation_dialog_)));
On 2016/09/27 17:48:40, Sergey Ulanov wrote:
> It2MeConfirmationDialogProxy creation can be moved to the constructor - this
> would allow to remove confirmation_dialog_.

I looked into this, it turns out the proxy class makes some assumptions on how
it will be run based on the thread it is created on, it also affects how unit
tests interact with it as well.

My inclination is to tag this with a new bug and address it in a separate CL.

[joedow, 2016-09-27 20:07:10]

The CQ bit was checked by joedow@chromium.org

[joedow, 2016-09-27 20:07:10]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2372833002/#ps60001
(title: "Addressing feedback")

[commit-bot: I haz the power, 2016-09-27 20:07:39]

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-27 21:01:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-27 21:03:37]

Description was changed from

==========
Removing It2MeConfirmationDialogFactory

Since the It2MeConfirmationDialog is now supported on all platforms, we can
remove the factory used to create dialog instances and instead use a static
method on the class itself.

BUG=649016
==========

to

==========
Removing It2MeConfirmationDialogFactory

Since the It2MeConfirmationDialog is now supported on all platforms, we can
remove the factory used to create dialog instances and instead use a static
method on the class itself.

BUG=649016

Committed: https://crrev.com/7fcc469912720fdcbc26429b850666778a4ccbb7
Cr-Commit-Position: refs/heads/master@{#421333}
==========

[commit-bot: I haz the power, 2016-09-27 21:03:37]

Patchset 4 (id:??) landed as
https://crrev.com/7fcc469912720fdcbc26429b850666778a4ccbb7
Cr-Commit-Position: refs/heads/master@{#421333}