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

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

Issue 1058403002: Revert of Refactor frame navigation/detach state cleanup to be more sane. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: 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
« no previous file with comments | « Source/core/frame/LocalFrame.h ('k') | Source/core/loader/DocumentLoader.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/frame/LocalFrame.cpp
diff --git a/Source/core/frame/LocalFrame.cpp b/Source/core/frame/LocalFrame.cpp
index 0b8649d3a2a53b63488c48b36c8bc3315946c0ea..e94f3861e8c2ca1f629e129aa3637a93f222240f 100644
--- a/Source/core/frame/LocalFrame.cpp
+++ b/Source/core/frame/LocalFrame.cpp
@@ -132,7 +132,15 @@
void LocalFrame::setView(PassRefPtrWillBeRawPtr<FrameView> view)
{
ASSERT(!m_view || m_view != view);
- ASSERT(!document() || !document()->isActive());
+ 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();
+ }
eventHandler().clear();
@@ -259,8 +267,6 @@
// handlers might start a new subresource load in this frame.
m_loader.stopAllLoaders();
m_loader.detach();
- document()->prepareForDestruction();
- m_loader.clear();
if (!client())
return;
@@ -273,6 +279,10 @@
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.)
@@ -824,4 +834,15 @@
m_instrumentingAgents = localFrameRoot()->m_instrumentingAgents;
}
+void LocalFrame::detachView()
+{
+ // We detach the FrameView's custom scroll bars as early as
+ // possible to prevent m_doc->detach() from messing with the view
+ // such that its scroll bars won't be torn down.
+ //
+ // FIXME: We should revisit this.
+ if (m_view)
+ m_view->prepareForDetach();
+}
+
} // namespace blink
« no previous file with comments | « Source/core/frame/LocalFrame.h ('k') | Source/core/loader/DocumentLoader.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698