Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(681)

Unified Diff: Source/core/frame/LocalFrame.cpp

Issue 1052993006: Refactor frame navigation/detach state cleanup to be more sane. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: shuffle Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/frame/LocalFrame.cpp
diff --git a/Source/core/frame/LocalFrame.cpp b/Source/core/frame/LocalFrame.cpp
index ab38e41fd122edc120dce6897b9a266dae0e3406..0213c4a482410a77d108cf7d7aebe68e5c945fe5 100644
--- a/Source/core/frame/LocalFrame.cpp
+++ b/Source/core/frame/LocalFrame.cpp
@@ -132,16 +132,9 @@ PassRefPtrWillBeRawPtr<LocalFrame> LocalFrame::create(FrameLoaderClient* client,
void LocalFrame::setView(PassRefPtrWillBeRawPtr<FrameView> view)
{
ASSERT(!m_view || m_view != view);
- detachView();
-
- // Prepare for destruction now, so any unload event handlers get run and the LocalDOMWindow is
- // notified. If we wait until the view is destroyed, then things won't be hooked up enough for
- // these calls to work.
- if (!view && document() && document()->isActive()) {
- // FIXME: We don't call willRemove here. Why is that OK?
- document()->prepareForDestruction();
- }
+ ASSERT(!document() || !document()->isActive());
+ detachView();
eventHandler().clear();
m_view = view;
@@ -267,6 +260,11 @@ void LocalFrame::detach()
// handlers might start a new subresource load in this frame.
m_loader.stopAllLoaders();
m_loader.detach();
+ {
+ ScriptForbiddenScope forbidScript;
+ document()->prepareForDestruction();
+ m_loader.clear();
dcheng 2015/04/03 14:35:45 Calling this multiple times appears to be safe, an
+ }
if (!client())
return;
@@ -279,10 +277,6 @@ void LocalFrame::detach()
willDetachFrameHost();
InspectorInstrumentation::frameDetachedFromParent(this);
Frame::detach();
- // Clear the FrameLoader right here rather than during
- // finalization. Too late to access various heap objects at that
- // stage.
- m_loader.clear();
// Signal frame destruction here rather than in the destructor.
// Main motivation is to avoid being dependent on its exact timing (Oilpan.)

Powered by Google App Engine
This is Rietveld 408576698