Ensure a guest WebContent's view is removed from the InputEventRouter.

Ensure a guest WebContent's view is removed from the InputEventRouter.

If a guest WebContents goes away before its RenderWidgetHostView is
notified of its detaching, then the RenderWidgetHostView will be unable
to access the top-level WebContents to remove itself from the
RenderWidgetHostInputEventRouter's owner map. This can lead to stale
entries in the owners map, and is a potential cause for this issue.

BUG=570646

[wjmaclean, 2015-12-21 19:21:10]

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

[commit-bot: I haz the power, 2015-12-21 19:21:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541913002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541913002/1

[wjmaclean, 2015-12-21 20:06:45]

Description was changed from

==========
Ensure a guest WebContent's view is removed from the InputEventRouter.

If a guest WebContents goes away before its RenderWidgetHostView is
notified of its detaching, then the RenderWidgetHostView will be unable
to access the top-level WebContents to remove itself from the
RenderWidgetHostInputEventRouter's owner map. This can lead to stale
entries in the owners map, and is a potential cause for this issue.

BUG=570646
==========

to

==========
Ensure a guest WebContent's view is removed from the InputEventRouter.

If a guest WebContents goes away before its RenderWidgetHostView is
notified of its detaching, then the RenderWidgetHostView will be unable
to access the top-level WebContents to remove itself from the
RenderWidgetHostInputEventRouter's owner map. This can lead to stale
entries in the owners map, and is a potential cause for this issue.

BUG=570646
==========

[wjmaclean, 2015-12-21 20:06:45]

wjmaclean@chromium.org changed reviewers:
+ kenrb@chromium.org, nasko@chromium.org

[wjmaclean, 2015-12-21 20:07:47]

Here's a speculative fix for the RenderWidgetHostInputEventRouter crasher.

PTAL? If it solves the problem, then I can work on developing a test to go with
it.

[wjmaclean, 2015-12-21 20:08:19]

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

[commit-bot: I haz the power, 2015-12-21 20:08:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541913002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541913002/20001

[commit-bot: I haz the power, 2015-12-21 20:15:56]

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

[commit-bot: I haz the power, 2015-12-21 20:15:57]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[kenrb, 2015-12-21 21:51:03]

Thinking about this after our discussion earlier, it looks like we could still
have the same problem if there are nested RWHVChildFrames underneath the
RWHVGuest. It might be worth landing this to see if it resolves the problem for
touch event handling in non-site-per-process, but if it does fix it, this will
need revisiting later.

https://codereview.chromium.org/1541913002/diff/20001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1541913002/diff/20001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:429: auto rwhvb =
nit: I don't think variable names are supposed to be abbreviated like that under
the style guide.

[wjmaclean, 2015-12-22 12:43:38]

Agreed, this is not the ultimate fix in the case of multiply nested guests that
don't destruct in order.

I'm starting to think that the RWHVx should store a weak pointer to the
InputEventRouter it registered with, so it can be guaranteed access later to
remove the entry.

Or, as I mentioned earlier, we could simplify by just having the IER store a
weak pointer to the RWHVB.

https://codereview.chromium.org/1541913002/diff/20001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1541913002/diff/20001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:429: auto rwhvb =
On 2015/12/21 21:51:02, kenrb wrote:
> nit: I don't think variable names are supposed to be abbreviated like that
under
> the style guide.

No, probably not ... I was just following other usages of rwhvb in the file.

Done.

[wjmaclean, 2015-12-22 12:43:46]

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

[commit-bot: I haz the power, 2015-12-22 12:43:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1541913002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1541913002/40001

[commit-bot: I haz the power, 2015-12-22 13:54:54]

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

[commit-bot: I haz the power, 2015-12-22 13:54:55]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2015-12-22 15:29:56]

Closing this issue in favour of https://codereview.chromium.org/1545683002