Always use FrameNavigationDisabler during DocumentLoader detach.

Always use FrameNavigationDisabler during DocumentLoader detach.

BUG=617495
Committed: https://crrev.com/be655fd4fb9ab3291a855a939496111674037a2f
Cr-Commit-Position: refs/heads/master@{#400558}

[Nate Chapin, 2016-06-16 22:58:20]

japhet@chromium.org changed reviewers:
+ dcheng@chromium.org

[Nate Chapin, 2016-06-16 22:58:20]

I'm not aware of a case where we should allow navigations to begin during
DocumentLoader detach, and the trybots seem to agree. Is this a reasonable
solution to the security whack-a-mole game? The alternative is adding a third
FrameNavigationDisabler use in FrameLoader (in loadInSameDocument)

[dcheng, 2016-06-17 20:03:46]

LGTM =)

https://codereview.chromium.org/2079473002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/2079473002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/loader/FrameLoader.cpp:1116: if
(m_documentLoader) {
Unrelated nit: I wonder if we should just remove this if () while we're at it to
be consistent?

(I guess we have to keep the braces for TemporaryChange)

[Nate Chapin, 2016-06-17 20:36:27]

On 2016/06/17 20:03:46, dcheng wrote:
> LGTM =)
> 
>
https://codereview.chromium.org/2079473002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right):
> 
>
https://codereview.chromium.org/2079473002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/loader/FrameLoader.cpp:1116: if
> (m_documentLoader) {
> Unrelated nit: I wonder if we should just remove this if () while we're at it
to
> be consistent?
> 
> (I guess we have to keep the braces for TemporaryChange)

I thought about it, but decided it was more readable to leave the if() there,
rather than just the {}.

[Nate Chapin, 2016-06-17 20:36:38]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2016-06-17 20:37:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2079473002/1

[commit-bot: I haz the power, 2016-06-18 01:02:52]

Committed patchset #1 (id:1)

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

Description was changed from

==========
Always use FrameNavigationDisabler during DocumentLoader detach.

BUG=617495
==========

to

==========
Always use FrameNavigationDisabler during DocumentLoader detach.

BUG=617495

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

[commit-bot: I haz the power, 2016-06-18 01:05:25]

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