Fix a use-after-free in WebsiteSettings::OnUIClosing.

Fix a use-after-free in WebsiteSettings::OnUIClosing.

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
Committed: https://crrev.com/bdd53b5fbfbbb32d6ec8e5d8107040f3fa779868
Cr-Commit-Position: refs/heads/master@{#421408}

[dominickn, 2016-09-22 08:08:44]

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

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

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

[dominickn, 2016-09-22 08:14:24]

Description was changed from

==========
Make WebsiteSettings a WebContentsObserver.

This fixes a use-after-free/race at browser shutdown, where UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel. Now WebsiteSettings
accesses the WebContents via the web_contents() method, which will
return a nullptr after the contents is freed.

BUG=640571
==========

to

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
==========

[dominickn, 2016-09-22 08:14:24]

dominickn@chromium.org changed reviewers:
+ felt@chromium.org

[dominickn, 2016-09-22 08:25:06]

Description was changed from

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
==========

to

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing.

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
==========

[dominickn, 2016-09-22 08:31:46]

Hi felt, PTAL thanks!

[commit-bot: I haz the power, 2016-09-22 08:48:41]

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

[commit-bot: I haz the power, 2016-09-22 08:48:42]

Dry run: This issue passed the CQ dry run.

[felt, 2016-09-27 11:39:20]

lgtm - assuming there are no issues with Android (I notice some of the code this
is replacing is ifdef'ed for Android; the change looks fine to me for Android,
but presumably you have double checked)

[dominickn, 2016-09-27 12:14:59]

On 2016/09/27 11:39:20, felt wrote:
> lgtm - assuming there are no issues with Android (I notice some of the code
this
> is replacing is ifdef'ed for Android; the change looks fine to me for Android,
> but presumably you have double checked)

Thanks! The ifdefs were introduced to fix a compile warning about unused member
variables on Android. Now there are no unused member variables since the
web_contents is used for the observer; there doesn't seem to have been any
changes in functionality as a result.

[dominickn, 2016-09-28 00:17:09]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2016-09-28 00:17:28]

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-28 01:08:33]

Description was changed from

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing.

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
==========

to

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing.

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
==========

[commit-bot: I haz the power, 2016-09-28 01:08:34]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-28 01:09:55]

Description was changed from

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing.

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571
==========

to

==========
Fix a use-after-free in WebsiteSettings::OnUIClosing.

This CL makes WebsiteSettings a WebContentsObserver, fixing a
use-after-free/race at browser shutdown. The exploit triggers when UI
destruction occurs after the WebContents that WebsiteSettings holds a
pointer to has been freed by the TabStripModel, triggering a deref of
the now invalid pointer.

WebsiteSettings now uses its inherited web_contents() method, which
will return a nullptr after the contents is freed. This prevents the
use-after-free.

BUG=640571

Committed: https://crrev.com/bdd53b5fbfbbb32d6ec8e5d8107040f3fa779868
Cr-Commit-Position: refs/heads/master@{#421408}
==========

[commit-bot: I haz the power, 2016-09-28 01:09:56]

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