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

Side by Side 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: Minor test cleanup Created 5 years, 8 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 unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « Source/core/frame/LocalFrame.h ('k') | Source/core/loader/DocumentLoader.cpp » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org> 2 * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
3 * 1999 Lars Knoll <knoll@kde.org> 3 * 1999 Lars Knoll <knoll@kde.org>
4 * 1999 Antti Koivisto <koivisto@kde.org> 4 * 1999 Antti Koivisto <koivisto@kde.org>
5 * 2000 Simon Hausmann <hausmann@kde.org> 5 * 2000 Simon Hausmann <hausmann@kde.org>
6 * 2000 Stefan Schimanski <1Stein@gmx.de> 6 * 2000 Stefan Schimanski <1Stein@gmx.de>
7 * 2001 George Staikos <staikos@kde.org> 7 * 2001 George Staikos <staikos@kde.org>
8 * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All r ights reserved. 8 * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All r ights reserved.
9 * Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com> 9 * Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com>
10 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies) 10 * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
(...skipping 114 matching lines...) Expand 10 before | Expand all | Expand 10 after
125 PassRefPtrWillBeRawPtr<LocalFrame> LocalFrame::create(FrameLoaderClient* client, FrameHost* host, FrameOwner* owner) 125 PassRefPtrWillBeRawPtr<LocalFrame> LocalFrame::create(FrameLoaderClient* client, FrameHost* host, FrameOwner* owner)
126 { 126 {
127 RefPtrWillBeRawPtr<LocalFrame> frame = adoptRefWillBeNoop(new LocalFrame(cli ent, host, owner)); 127 RefPtrWillBeRawPtr<LocalFrame> frame = adoptRefWillBeNoop(new LocalFrame(cli ent, host, owner));
128 InspectorInstrumentation::frameAttachedToParent(frame.get()); 128 InspectorInstrumentation::frameAttachedToParent(frame.get());
129 return frame.release(); 129 return frame.release();
130 } 130 }
131 131
132 void LocalFrame::setView(PassRefPtrWillBeRawPtr<FrameView> view) 132 void LocalFrame::setView(PassRefPtrWillBeRawPtr<FrameView> view)
133 { 133 {
134 ASSERT(!m_view || m_view != view); 134 ASSERT(!m_view || m_view != view);
135 detachView(); 135 ASSERT(!document() || !document()->isActive());
136
137 // Prepare for destruction now, so any unload event handlers get run and the LocalDOMWindow is
138 // notified. If we wait until the view is destroyed, then things won't be ho oked up enough for
139 // these calls to work.
140 if (!view && document() && document()->isActive()) {
141 // FIXME: We don't call willRemove here. Why is that OK?
142 document()->prepareForDestruction();
143 }
144 136
145 eventHandler().clear(); 137 eventHandler().clear();
146 138
147 m_view = view; 139 m_view = view;
148 } 140 }
149 141
150 void LocalFrame::createView(const IntSize& viewportSize, const Color& background Color, bool transparent, 142 void LocalFrame::createView(const IntSize& viewportSize, const Color& background Color, bool transparent,
151 ScrollbarMode horizontalScrollbarMode, bool horizontalLock, 143 ScrollbarMode horizontalScrollbarMode, bool horizontalLock,
152 ScrollbarMode verticalScrollbarMode, bool verticalLock) 144 ScrollbarMode verticalScrollbarMode, bool verticalLock)
153 { 145 {
(...skipping 106 matching lines...) Expand 10 before | Expand all | Expand 10 after
260 // detached, so protect a reference to it. 252 // detached, so protect a reference to it.
261 RefPtrWillBeRawPtr<LocalFrame> protect(this); 253 RefPtrWillBeRawPtr<LocalFrame> protect(this);
262 m_loader.stopAllLoaders(); 254 m_loader.stopAllLoaders();
263 m_loader.dispatchUnloadEvent(); 255 m_loader.dispatchUnloadEvent();
264 detachChildren(); 256 detachChildren();
265 // stopAllLoaders() needs to be called after detachChildren(), because detac hChildren() 257 // stopAllLoaders() needs to be called after detachChildren(), because detac hChildren()
266 // will trigger the unload event handlers of any child frames, and those eve nt 258 // will trigger the unload event handlers of any child frames, and those eve nt
267 // handlers might start a new subresource load in this frame. 259 // handlers might start a new subresource load in this frame.
268 m_loader.stopAllLoaders(); 260 m_loader.stopAllLoaders();
269 m_loader.detach(); 261 m_loader.detach();
262 document()->prepareForDestruction();
263 m_loader.clear();
270 if (!client()) 264 if (!client())
271 return; 265 return;
272 266
273 client()->willBeDetached(); 267 client()->willBeDetached();
274 // Notify ScriptController that the frame is closing, since its cleanup ends up calling 268 // Notify ScriptController that the frame is closing, since its cleanup ends up calling
275 // back to FrameLoaderClient via WindowProxy. 269 // back to FrameLoaderClient via WindowProxy.
276 script().clearForClose(); 270 script().clearForClose();
277 ScriptForbiddenScope forbidScript; 271 ScriptForbiddenScope forbidScript;
278 setView(nullptr); 272 setView(nullptr);
279 willDetachFrameHost(); 273 willDetachFrameHost();
280 InspectorInstrumentation::frameDetachedFromParent(this); 274 InspectorInstrumentation::frameDetachedFromParent(this);
281 Frame::detach(); 275 Frame::detach();
282 // Clear the FrameLoader right here rather than during
283 // finalization. Too late to access various heap objects at that
284 // stage.
285 m_loader.clear();
286 276
287 // Signal frame destruction here rather than in the destructor. 277 // Signal frame destruction here rather than in the destructor.
288 // Main motivation is to avoid being dependent on its exact timing (Oilpan.) 278 // Main motivation is to avoid being dependent on its exact timing (Oilpan.)
289 LocalFrameLifecycleNotifier::notifyContextDestroyed(); 279 LocalFrameLifecycleNotifier::notifyContextDestroyed();
290 m_supplements.clear(); 280 m_supplements.clear();
291 } 281 }
292 282
293 SecurityContext* LocalFrame::securityContext() const 283 SecurityContext* LocalFrame::securityContext() const
294 { 284 {
295 return document(); 285 return document();
(...skipping 531 matching lines...) Expand 10 before | Expand all | Expand 10 after
827 , m_inViewSourceMode(false) 817 , m_inViewSourceMode(false)
828 , m_shouldSendDPRHint(false) 818 , m_shouldSendDPRHint(false)
829 , m_shouldSendRWHint(false) 819 , m_shouldSendRWHint(false)
830 { 820 {
831 if (isLocalRoot()) 821 if (isLocalRoot())
832 m_instrumentingAgents = InstrumentingAgents::create(); 822 m_instrumentingAgents = InstrumentingAgents::create();
833 else 823 else
834 m_instrumentingAgents = localFrameRoot()->m_instrumentingAgents; 824 m_instrumentingAgents = localFrameRoot()->m_instrumentingAgents;
835 } 825 }
836 826
837 void LocalFrame::detachView()
838 {
839 // We detach the FrameView's custom scroll bars as early as
840 // possible to prevent m_doc->detach() from messing with the view
841 // such that its scroll bars won't be torn down.
842 //
843 // FIXME: We should revisit this.
844 if (m_view)
845 m_view->prepareForDetach();
846 }
847
848 } // namespace blink 827 } // namespace blink
OLDNEW
« 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