Add an extra guard to replaceDocument()

Add an extra guard to replaceDocument()

The key of this change is to add an extra guard against rude JS in unload event
handlers.

To add the change in a sane way, this change also includes some refactoring:

 * Moving |shouldReuseDefaultView| local variable, which should be computed
   beforehand to DocumentInit.
 * Splitting replaceDocument into DocuentLoader and FrameLoader.
 * Renaming them to replaceDocumentWriteExecutingJavaScriptURL() for clarity.
 * Using the FrameLoader API instead of DocumentLoader one from ScriptController.

TEST=unload-mutation-crash.html
R=esprehn@chromium.org, dglazkov@chromium.org, abarth@chromium.org
BUG=405745
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180918

[dglazkov, 2014-08-26 16:03:33]

https://codereview.chromium.org/495743003/diff/1/Source/core/dom/DocumentInit.h
File Source/core/dom/DocumentInit.h (right):

https://codereview.chromium.org/495743003/diff/1/Source/core/dom/DocumentInit...
Source/core/dom/DocumentInit.h:90: // In some rare cases, we'll re-used a
LocalDOMWindow for a new Document. For example,
"we'll reuse"

https://codereview.chromium.org/495743003/diff/1/Source/core/dom/DocumentInit...
Source/core/dom/DocumentInit.h:97: // FIXME: This is for DocumentWrite creation,
not for one of Document.
DocumentWriter?

https://codereview.chromium.org/495743003/diff/1/Source/core/loader/DocumentL...
File Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/495743003/diff/1/Source/core/loader/DocumentL...
Source/core/loader/DocumentLoader.cpp:826: // This is only called by
FraemLoader::replaceDocument()
"FrameLoader"

https://codereview.chromium.org/495743003/diff/1/Source/core/loader/FrameLoad...
File Source/core/loader/FrameLoader.h (right):

https://codereview.chromium.org/495743003/diff/1/Source/core/loader/FrameLoad...
Source/core/loader/FrameLoader.h:105: void replaceDocument(const String& source,
Document* ownerDocument);
If it only has one intended callsite, let's name it something specific, like
replaceDocumentWhileExecutingJavascriptURL?

[Hajime Morrita, 2014-08-26 17:13:01]

Thanks for the review Dimitri!
I updated the patch. PTAL?

[dglazkov, 2014-08-26 17:22:43]

LGTM.

https://codereview.chromium.org/495743003/diff/20001/Source/core/loader/Docum...
File Source/core/loader/DocumentLoader.cpp (right):

https://codereview.chromium.org/495743003/diff/20001/Source/core/loader/Docum...
Source/core/loader/DocumentLoader.cpp:779:
PassRefPtrWillBeRawPtr<DocumentWriter>
DocumentLoader::createWriterFor(LocalFrame* frame, const Document*
ownerDocument, const KURL& url, const AtomicString& mimeType, const
AtomicString& encoding, bool dispatch)
Just thinking outloud: since now there are two createWriteFor functions, and
both of them really operate on DocumentInit, maybe we could:
1) make DocumentInit of both of them, and/or
2) rename the newly-created one to sound a lot more ominous, since it's doing
nasty things.

[Hajime Morrita, 2014-08-26 18:21:22]

On 2014/08/26 17:22:43, dglazkov wrote:
> LGTM.
> 
>
https://codereview.chromium.org/495743003/diff/20001/Source/core/loader/Docum...
> File Source/core/loader/DocumentLoader.cpp (right):
> 
>
https://codereview.chromium.org/495743003/diff/20001/Source/core/loader/Docum...
> Source/core/loader/DocumentLoader.cpp:779:
> PassRefPtrWillBeRawPtr<DocumentWriter>
> DocumentLoader::createWriterFor(LocalFrame* frame, const Document*
> ownerDocument, const KURL& url, const AtomicString& mimeType, const
> AtomicString& encoding, bool dispatch)
> Just thinking outloud: since now there are two createWriteFor functions, and
> both of them really operate on DocumentInit, maybe we could:
> 1) make DocumentInit of both of them, and/or
> 2) rename the newly-created one to sound a lot more ominous, since it's doing
> nasty things.
Right. I inlined the non-DocumentInit version into its only one caller.

[Hajime Morrita, 2014-08-26 18:26:29]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2014-08-26 18:27:35]

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

[commit-bot: I haz the power, 2014-08-26 21:41:35]

Committed patchset #3 (40001) as 180918