Speculative fix for DelegatedFrameHost destructor crashes

Speculative fix for DelegatedFrameHost destructor crashes

It appears that RenderWidgetHostViewMac::Destroy is never getting
called in some instances, and that is resulting in crashes when we end
up in its destructor, which is called by Objective C's runtime.

I suspect that this is a leak somewhere, and that on other platforms we
simply never call the destructor.

Call the necessary bits of RenderWidgetHostViewMac::Destroy to avoid
crash when a RenderWidgetHostImpl reaches its destructor before its
view_ has been reset.

Also, turn the weak pointers to RenderWidgetHostViewBase in
RenderWidgetHostImpl and RenderWidgetHostViewGuest
to use base::WeakPtr to prevent the many dangling instances.

BUG=404828
Committed: https://crrev.com/c54cc7cb4a7910f83b6501f8122b06ba5c8b3a45
Cr-Commit-Position: refs/heads/master@{#293573}

[ccameron, 2014-09-04 04:45:59]

ccameron@chromium.org changed reviewers:
+ piman@chromium.org

[ccameron, 2014-09-04 04:45:59]

This is the smallest (potential) fix I could think of. This needs to get merged
to M38, so the larger efforts to clean this up (see, for instance,
https://codereview.chromium.org/536063002/), are out of the question (not to
mention the fact that they don't work).

[piman, 2014-09-04 04:52:15]

LGTM, seems reasonable, provided something will not try to reach into the
RWHVMac and expect the DFH to still exist.

[ccameron, 2014-09-04 07:07:42]

On 2014/09/04 04:52:15, piman (OOO) wrote:
> LGTM, seems reasonable, provided something will not try to reach into the
> RWHVMac and expect the DFH to still exist.

Even this blows everything up -- the view_ pointer there is a dangling
non-NULL-but-invalid value ... what a mess.

[ccameron, 2014-09-04 21:09:15]

On 2014/09/04 07:07:42, ccameron1 wrote:
> On 2014/09/04 04:52:15, piman (OOO) wrote:
> > LGTM, seems reasonable, provided something will not try to reach into the
> > RWHVMac and expect the DFH to still exist.
> 
> Even this blows everything up -- the view_ pointer there is a dangling
> non-NULL-but-invalid value ... what a mess.

Okay, looks like changing everything to use weak pointers makes everything
green. Sending it through.

[ccameron, 2014-09-04 21:09:18]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-09-04 21:10:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/538883002/40001

[piman, 2014-09-04 23:56:02]

lgtm

[commit-bot: I haz the power, 2014-09-05 03:10:57]

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

[commit-bot: I haz the power, 2014-09-05 03:10:58]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[ccameron, 2014-09-05 04:38:38]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-09-05 04:39:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/538883002/40001

[ccameron, 2014-09-05 04:47:19]

On 2014/09/05 04:39:56, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-status.appspot.com/cq/ccameron@chromium.org/538883002/40001

I have a hard time believing this would be legit. If it is, I'll add a Mac-only
weak ptr (running out of racetrack to merge).

[commit-bot: I haz the power, 2014-09-05 06:35:49]

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

[commit-bot: I haz the power, 2014-09-05 06:35:50]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[ccameron, 2014-09-05 17:23:14]

On 2014/09/05 06:35:50, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   android_dbg_tests_recipe on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

Okay, this is a basically-mac-only version of the patch, because the android
failures are completely opaque to me.

[piman, 2014-09-05 17:45:59]

On 2014/09/05 17:23:14, ccameron1 wrote:
> On 2014/09/05 06:35:50, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   android_dbg_tests_recipe on tryserver.chromium.linux
> >
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
> 
> Okay, this is a basically-mac-only version of the patch, because the android
> failures are completely opaque to me.

Do you want to #ifdef mac the code? It makes TPMs more amenable to changes...

[piman, 2014-09-05 17:46:55]

(LGTM either way)


On Fri, Sep 5, 2014 at 10:45 AM, <piman@chromium.org> wrote:

> On 2014/09/05 17:23:14, ccameron1 wrote:
>
>> On 2014/09/05 06:35:50, I haz the power (commit-bot) wrote:
>> > Try jobs failed on following builders:
>> >   android_dbg_tests_recipe on tryserver.chromium.linux
>> >
>>
>
> (http://build.chromium.org/p/tryserver.chromium.linux/
> builders/android_dbg_tests_recipe/builds/5561)
>
>  Okay, this is a basically-mac-only version of the patch, because the
>> android
>> failures are completely opaque to me.
>>
>
> Do you want to #ifdef mac the code? It makes TPMs more amenable to
> changes...
>
> https://codereview.chromium.org/538883002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ccameron, 2014-09-05 19:37:37]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-09-05 19:38:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/538883002/100001

[commit-bot: I haz the power, 2014-09-05 20:14:04]

Committed patchset #6 (id:100001) as 0fb966bd89bc617839113cd7943280a00c8aeb6f

[commit-bot: I haz the power, 2014-09-10 03:41:20]

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