Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/764db137cad662c0b40399de0486b39048aa02b0
Cr-Commit-Position: refs/heads/master@{#381713}

[clamy, 2016-03-03 12:00:43]

Description was changed from

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
==========

to

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-03-15 10:57:52]

Description was changed from

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-03-15 10:57:53]

clamy@chromium.org changed reviewers:
+ nasko@chromium.org

[clamy, 2016-03-15 10:58:29]

@nasko: PTAL. This is a temporary patch similar to what you did for the
~NavigationParams patch.

[nasko, 2016-03-15 14:10:34]

Couple of minor questions.

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:613: // TODO(clamy): The
NavigationHandle should always be reset here.
Why not add a CHECK to verify this?

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1102: if
(navigation_handle_.get())
Shouldn't the handle be destroyed once we've committed?

[clamy, 2016-03-15 14:16:16]

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:613: // TODO(clamy): The
NavigationHandle should always be reset here.
On 2016/03/15 14:10:34, nasko (slow) wrote:
> Why not add a CHECK to verify this?

Because I know for a fact that this isn't the case and I plan to fix it, but not
in that patch.

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1759123002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1102: if
(navigation_handle_.get())
On 2016/03/15 14:10:34, nasko (slow) wrote:
> Shouldn't the handle be destroyed once we've committed?

No, hence the TODO in Navigator.

[clamy, 2016-03-17 12:56:08]

@nasko: ping

[nasko, 2016-03-17 14:10:40]

LGTM

[nasko, 2016-03-17 14:10:47]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2016-03-17 14:10:58]

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

[commit-bot: I haz the power, 2016-03-17 15:25:26]

Description was changed from

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-03-17 15:25:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-17 15:27:18]

Description was changed from

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Ensure RenderFrameHost & NavigationHandle are not destroyed during commit

This CL adds a boolean to RenderFrameHost and NavigationHandle to check if they
are destroyed during a navigation commit, which would lead to a use-after-free
bug.

BUG=589365
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/764db137cad662c0b40399de0486b39048aa02b0
Cr-Commit-Position: refs/heads/master@{#381713}
==========

[commit-bot: I haz the power, 2016-03-17 15:27:18]

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

[clamy, 2016-04-04 12:15:48]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1856693002/ by clamy@chromium.org.

The reason for reverting is: Now that the underlying reason for issue 589365 has
been found and fixed, this investigation code can be removed..