Close the email confirmation dialog when its web contents are killed.

Close the email confirmation dialog when its web contents are killed.

This CL adds an observer for the web contents of the sign-in email
confirmation dialog and closes the sign-in confirmation dialog when
the renderer process is killed.

BUG=671983
Committed: https://crrev.com/179af9bcbb6bbf6c72fc8eed7d26659bfa12e6e1
Cr-Commit-Position: refs/heads/master@{#441359}

[msarda, 2016-12-23 15:57:06]

Description was changed from

==========
Close the email confirmation dialog when its web contents are killed

BUG=
==========

to

==========
Close the email confirmation dialog when its web contents are killed.

BUG=671983
==========

[msarda, 2016-12-23 16:03:16]

Patchset #1 (id:1) has been deleted

[msarda, 2016-12-23 16:05:03]

Description was changed from

==========
Close the email confirmation dialog when its web contents are killed.

BUG=671983
==========

to

==========
Close the email confirmation dialog when its web contents are killed.

This CL adds an observer for the web contents of the sign-in email
confirmation dialog and closes the sign-in confirmation dialog when
the renderer process is killed.

BUG=671983
==========

[msarda, 2016-12-23 16:08:19]

msarda@chromium.org changed reviewers:
+ anthonyvd@chromium.org

[msarda, 2016-12-23 16:08:20]

Please take a look.

[anthonyvd, 2017-01-02 15:09:22]

It looks to me like the better overall experience would be to disallow the
context menu in that dialog (like we do for the other sign in flow dialogs, see
https://cs.chromium.org/chromium/src/chrome/browser/ui/signin_view_controller...)
and make the dialog browser-modal instead of tab-modal (which you did in a
previous CL IIRC).

This would make this edge-case effectively irrelevant, while providing the added
benefit of the dialog behaving a bit more like native UI and the dialogs that
precede it. It also looks less complex than the solution in this patch.

WDYT?

[msarda, 2017-01-02 16:54:49]

That was my initial thought as well, however it is not possible in the way I
have implemented this dialog:
* SigninEmailConfirmationDialog does not subclass content::WebContentsDelegate
* The actual dialog is in fact created and presented by
ShowConstrainedWebDialogWithAutoResize. This method internally creates a views
object of type ConstrainedWebDialogDelegateViewViews, that has inside an impl_
object of type ConstrainedWebDialogDelegateViews that actually subclasses
content::WebContentsDelegate  but does not implement the HandleContextMenu
method (see
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/constrained_web_...
):

So avoiding to display the context menu is not possible in the current API of
the ConstainedWebDialog.

I think there are some advantages to the current implementation though:
* it allows us to debug the dialog by inspecting it
* I suppose that if the renderrer process used for the dialog is killed by the
OS, then the dialog is correctly closed.

[anthonyvd, 2017-01-03 18:18:24]

Ah that's unfortunate. LGTM then.

[msarda, 2017-01-04 10:02:12]

The CQ bit was checked by msarda@chromium.org

[commit-bot: I haz the power, 2017-01-04 10:02:22]

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-01-04 10:26:18]

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

[commit-bot: I haz the power, 2017-01-04 10:26:19]

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_...)

[msarda, 2017-01-04 12:07:39]

The CQ bit was checked by msarda@chromium.org

[msarda, 2017-01-04 12:07:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
anthonyvd@chromium.org
Link to the patchset: https://codereview.chromium.org/2603493003/#ps40001
(title: "Fix compile")

[commit-bot: I haz the power, 2017-01-04 12:07:46]

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-01-04 12:56:11]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483531659752520,
"parent_rev": "3e8a41aaf75e5ebca93300b9a7c6000c93664563", "commit_rev":
"f0eb3fb58bfb89529f1905b40f35a161c72198f7"}

[commit-bot: I haz the power, 2017-01-04 12:56:40]

Description was changed from

==========
Close the email confirmation dialog when its web contents are killed.

This CL adds an observer for the web contents of the sign-in email
confirmation dialog and closes the sign-in confirmation dialog when
the renderer process is killed.

BUG=671983
==========

to

==========
Close the email confirmation dialog when its web contents are killed.

This CL adds an observer for the web contents of the sign-in email
confirmation dialog and closes the sign-in confirmation dialog when
the renderer process is killed.

BUG=671983

Review-Url: https://codereview.chromium.org/2603493003
==========

[commit-bot: I haz the power, 2017-01-04 12:56:41]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2017-01-04 13:01:50]

Description was changed from

==========
Close the email confirmation dialog when its web contents are killed.

This CL adds an observer for the web contents of the sign-in email
confirmation dialog and closes the sign-in confirmation dialog when
the renderer process is killed.

BUG=671983

Review-Url: https://codereview.chromium.org/2603493003
==========

to

==========
Close the email confirmation dialog when its web contents are killed.

This CL adds an observer for the web contents of the sign-in email
confirmation dialog and closes the sign-in confirmation dialog when
the renderer process is killed.

BUG=671983

Committed: https://crrev.com/179af9bcbb6bbf6c72fc8eed7d26659bfa12e6e1
Cr-Commit-Position: refs/heads/master@{#441359}
==========

[commit-bot: I haz the power, 2017-01-04 13:01:51]

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