Add a JavaScript dialog tab helper.

Add a JavaScript dialog tab helper.

No functionality change.

BUG=629964
TEST=none

Committed: https://crrev.com/6082932158d1756e3d12764df3e4c2f2507bef50
Cr-Commit-Position: refs/heads/master@{#416023}

[Avi (use Gerrit), 2016-07-25 19:47:41]

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

[commit-bot: I haz the power, 2016-07-25 19:48:28]

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-07-25 20:27:08]

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

[commit-bot: I haz the power, 2016-07-25 20:27:10]

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_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Avi (use Gerrit), 2016-07-25 21:09:46]

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

[commit-bot: I haz the power, 2016-07-25 21:10:33]

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-07-25 21:45:14]

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

[commit-bot: I haz the power, 2016-07-25 21:45:15]

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

[Avi (use Gerrit), 2016-08-10 22:18:20]

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

[commit-bot: I haz the power, 2016-08-10 22:18:59]

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-08-10 23:01:02]

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

[commit-bot: I haz the power, 2016-08-10 23:01:03]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Avi (use Gerrit), 2016-08-11 14:59:23]

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

[commit-bot: I haz the power, 2016-08-11 14:59:40]

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-08-11 15:44:37]

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

[commit-bot: I haz the power, 2016-08-11 15:44:38]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Avi (use Gerrit), 2016-08-17 22:55:13]

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

[commit-bot: I haz the power, 2016-08-17 22:55:42]

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-08-17 23:44:51]

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

[commit-bot: I haz the power, 2016-08-17 23:44:52]

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2016-08-31 15:46:05]

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

[Avi (use Gerrit), 2016-08-31 15:46:06]

Scott, you love JavaScript dialogs, right?

[sky, 2016-08-31 16:53:10]

LGTM

[Avi (use Gerrit), 2016-09-01 01:04:35]

avi@chromium.org changed reviewers:
+ raymes@chromium.org

[Avi (use Gerrit), 2016-09-01 01:04:36]

raymes, guest stuff

[Avi (use Gerrit), 2016-09-01 01:27:06]

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

[commit-bot: I haz the power, 2016-09-01 01:27:29]

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

[raymes, 2016-09-01 01:28:30]

https://codereview.chromium.org/2179923002/diff/80001/extensions/browser/gues...
File extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
(right):

https://codereview.chromium.org/2179923002/diff/80001/extensions/browser/gues...
extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc:217:
owner_web_contents());
Could you explain why you've changed this? Thanks!

[Avi (use Gerrit), 2016-09-01 01:38:02]

https://codereview.chromium.org/2179923002/diff/80001/extensions/browser/gues...
File extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
(right):

https://codereview.chromium.org/2179923002/diff/80001/extensions/browser/gues...
extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc:217:
owner_web_contents());
On 2016/09/01 01:28:30, raymes wrote:
> Could you explain why you've changed this? Thanks!

Rather than having a single manager globally, I want to have the ability to have
the dialog code in a tab helper, living on the tab. And since the Browser is the
delegate for all of the tabs, the only way it knows which tab's tab helper to
use is that parameter.

Currently, the delegate gets the call "what's the dialog manager for this
WebContents that you're not delegate for?" With a global manager, that doesn't
matter, but with per-tab managers, that becomes a problem.

Once we have the manager, the guest WebContents can pass its pointer to the
manager, but to get the manager itself the guest WebContents has to pretend that
it's its parent, because the delegate doesn't know that the guest WebContents
exists.

[raymes, 2016-09-01 01:53:24]

raymes@chromium.org changed reviewers:
+ paulmeyer@chromium.org

[raymes, 2016-09-01 01:53:25]

I defer to someone in extensions/browser/guest_view/ (+paulmeyer) since I don't
understand the implications well enough. Sorry!

[commit-bot: I haz the power, 2016-09-01 02:48:53]

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

[commit-bot: I haz the power, 2016-09-01 02:48:54]

Dry run: This issue passed the CQ dry run.

[paulmeyer, 2016-09-01 15:45:53]

LGTM for mime_handler_view_guest.cc change, with one nit.

https://codereview.chromium.org/2179923002/diff/80001/extensions/browser/gues...
File extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
(right):

https://codereview.chromium.org/2179923002/diff/80001/extensions/browser/gues...
extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc:217:
owner_web_contents());
On 2016/09/01 01:38:02, Avi wrote:
> On 2016/09/01 01:28:30, raymes wrote:
> > Could you explain why you've changed this? Thanks!
> 
> Rather than having a single manager globally, I want to have the ability to
have
> the dialog code in a tab helper, living on the tab. And since the Browser is
the
> delegate for all of the tabs, the only way it knows which tab's tab helper to
> use is that parameter.
> 
> Currently, the delegate gets the call "what's the dialog manager for this
> WebContents that you're not delegate for?" With a global manager, that doesn't
> matter, but with per-tab managers, that becomes a problem.
> 
> Once we have the manager, the guest WebContents can pass its pointer to the
> manager, but to get the manager itself the guest WebContents has to pretend
that
> it's its parent, because the delegate doesn't know that the guest WebContents
> exists.

Nit: Could you please add a short comment here to this effect? (particularly
about the guest pretending to be its parent and why)

[Avi (use Gerrit), 2016-09-01 18:04:52]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-09-01 18:04:52]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
paulmeyer@chromium.org
Link to the patchset: https://codereview.chromium.org/2179923002/#ps100001
(title: "comment")

[commit-bot: I haz the power, 2016-09-01 18:05:24]

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-01 19:47:33]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-01 19:50:42]

Description was changed from

==========
Add a JavaScript dialog tab helper.

No functionality change.

BUG=629964
TEST=none
==========

to

==========
Add a JavaScript dialog tab helper.

No functionality change.

BUG=629964
TEST=none

Committed: https://crrev.com/6082932158d1756e3d12764df3e4c2f2507bef50
Cr-Commit-Position: refs/heads/master@{#416023}
==========

[commit-bot: I haz the power, 2016-09-01 19:50:43]

Patchset 6 (id:??) landed as
https://crrev.com/6082932158d1756e3d12764df3e4c2f2507bef50
Cr-Commit-Position: refs/heads/master@{#416023}