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

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

Issue 603193005: Move the Widget hierarchy to the Oilpan heap. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Rebase needed again Created 6 years, 2 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/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index d1ecd0e3d8646ec5e40a986d3267757a11098927..0b98466c0ce3b327b1d48d647a94c77ef52ba82a 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -122,8 +122,11 @@ FrameView::FrameView(LocalFrame* frame)
, m_inputEventsScaleFactorForEmulation(1)
, m_layoutSizeFixedToFrameSize(true)
, m_didScrollTimer(this, &FrameView::didScrollTimerFired)
- , m_needsUpdateWidgetPositions(false)
, m_topControlsViewportAdjustment(0)
+ , m_needsUpdateWidgetPositions(false)
+#if ENABLE(OILPAN) && ENABLE(ASSERT)
+ , m_hasBeenDisposed(false)
+#endif
{
ASSERT(m_frame);
init();
@@ -135,16 +138,16 @@ FrameView::FrameView(LocalFrame* frame)
ScrollableArea::setHorizontalScrollElasticity(ScrollElasticityAllowed);
}
-PassRefPtr<FrameView> FrameView::create(LocalFrame* frame)
+PassRefPtrWillBeRawPtr<FrameView> FrameView::create(LocalFrame* frame)
{
- RefPtr<FrameView> view = adoptRef(new FrameView(frame));
+ RefPtrWillBeRawPtr<FrameView> view = adoptRefWillBeNoop(new FrameView(frame));
view->show();
return view.release();
}
-PassRefPtr<FrameView> FrameView::create(LocalFrame* frame, const IntSize& initialSize)
+PassRefPtrWillBeRawPtr<FrameView> FrameView::create(LocalFrame* frame, const IntSize& initialSize)
{
- RefPtr<FrameView> view = adoptRef(new FrameView(frame));
+ RefPtrWillBeRawPtr<FrameView> view = adoptRefWillBeNoop(new FrameView(frame));
view->Widget::setFrameRect(IntRect(view->location(), initialSize));
view->setLayoutSizeInternal(initialSize);
@@ -154,6 +157,18 @@ PassRefPtr<FrameView> FrameView::create(LocalFrame* frame, const IntSize& initia
FrameView::~FrameView()
{
+#if ENABLE(OILPAN)
+ ASSERT(m_hasBeenDisposed);
+#else
+ // Verify that the LocalFrame has a different FrameView or
+ // that it is being detached and destructed.
+ ASSERT(frame().view() != this || !renderView());
+ dispose();
+#endif
+}
+
+void FrameView::dispose()
+{
if (m_postLayoutTasksTimer.isActive())
m_postLayoutTasksTimer.stop();
@@ -171,12 +186,29 @@ FrameView::~FrameView()
ASSERT(!m_scrollCorner);
- ASSERT(m_frame);
- ASSERT(m_frame->view() != this || !m_frame->contentRenderer());
// FIXME: Do we need to do something here for OOPI?
HTMLFrameOwnerElement* ownerElement = m_frame->deprecatedLocalOwner();
if (ownerElement && ownerElement->ownedWidget() == this)
ownerElement->setWidget(nullptr);
+
+ disposeAutoSizeInfo();
+#if ENABLE(OILPAN) && ENABLE(ASSERT)
+ m_hasBeenDisposed = true;
+#endif
+}
+
+void FrameView::trace(Visitor* visitor)
+{
+#if ENABLE(OILPAN)
+ visitor->trace(m_widgetUpdateSet);
+ visitor->trace(m_widgets);
+ visitor->trace(m_frame);
+ visitor->trace(m_nodeToDraw);
+ visitor->trace(m_maintainScrollPositionAnchor);
+ visitor->trace(m_scrollCorner);
+ visitor->trace(m_autoSizeInfo);
+#endif
+ ScrollView::trace(visitor);
}
void FrameView::reset()
@@ -252,6 +284,13 @@ void FrameView::prepareForDetach()
if (ScrollingCoordinator* scrollingCoordinator = m_frame->page()->scrollingCoordinator())
scrollingCoordinator->willDestroyScrollableArea(this);
}
+
+#if ENABLE(OILPAN)
+ // FIXME: once/if dust settles, do this always (non-Oilpan)?
+ //
+ // FIXME: Oilpan: is this safe to dispose() if there are FrameView protections on the stack?
sof 2014/10/08 11:52:25 This is one of the main issues left to resolve.
sof 2014/10/08 14:43:41 cf. https://codereview.chromium.org/603193005/diff
haraken 2014/10/08 15:15:43 Are there many protecting pointers? If there are o
+ dispose();
+#endif
}
void FrameView::detachCustomScrollbars()
@@ -407,7 +446,7 @@ bool FrameView::shouldUseCustomScrollbars(Element*& customScrollbarElement, Loca
return false;
}
-PassRefPtr<Scrollbar> FrameView::createScrollbar(ScrollbarOrientation orientation)
+PassRefPtrWillBeRawPtr<Scrollbar> FrameView::createScrollbar(ScrollbarOrientation orientation)
{
Element* customScrollbarElement = 0;
LocalFrame* customScrollbarFrame = 0;
@@ -678,7 +717,7 @@ inline void FrameView::forceLayoutParentViewIfNeeded()
// FrameView for a layout. After that the RenderEmbeddedObject (ownerRenderer) carries the
// correct size, which RenderSVGRoot::computeReplacedLogicalWidth/Height rely on, when laying
// out for the first time, or when the RenderSVGRoot size has changed dynamically (eg. via <script>).
- RefPtr<FrameView> frameView = ownerRenderer->frame()->view();
+ RefPtrWillBeRawPtr<FrameView> frameView = ownerRenderer->frame()->view();
// Mark the owner renderer as needing layout.
ownerRenderer->setNeedsLayoutAndPrefWidthsRecalcAndFullPaintInvalidation();
@@ -790,7 +829,7 @@ void FrameView::layout(bool allowSubtree)
TRACE_EVENT_SCOPED_SAMPLING_STATE("blink", "Layout");
// Protect the view from being deleted during layout (in recalcStyle)
- RefPtr<FrameView> protector(this);
+ RefPtrWillBeRawPtr<FrameView> protector(this);
// Every scroll that happens during layout is programmatic.
TemporaryChange<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
@@ -815,10 +854,12 @@ void FrameView::layout(bool allowSubtree)
performPreLayoutTasks();
+#if !ENABLE(OILPAN)
// If there is only one ref to this view left, then its going to be destroyed as soon as we exit,
// so there's no point to continuing to layout
if (protector->hasOneRef())
return;
+#endif
Document* document = m_frame->document();
bool inSubtreeLayout = isSubtreeLayout();
@@ -1500,7 +1541,7 @@ void FrameView::updateLayersAndCompositingAfterScrollIfNeeded()
if (!hasViewportConstrainedObjects())
return;
- RefPtr<FrameView> protect(this);
+ RefPtrWillBeRawPtr<FrameView> protect(this);
// If there fixed position elements, scrolling may cause compositing layers to change.
// Update widget and layer positions after scrolling, but only if we're not inside of
@@ -1882,7 +1923,7 @@ bool FrameView::updateWidgets()
void FrameView::updateWidgetsTimerFired(Timer<FrameView>*)
{
ASSERT(!isInPerformLayout());
- RefPtr<FrameView> protect(this);
+ RefPtrWillBeRawPtr<FrameView> protect(this);
m_updateWidgetsTimer.stop();
for (unsigned i = 0; i < maxUpdateWidgetsIterations; ++i) {
if (updateWidgets())
@@ -1915,7 +1956,7 @@ void FrameView::performPostLayoutTasks()
// We should ASSERT(isActive()); or at least return early if we can!
ASSERT(!isInPerformLayout()); // Always before or after performLayout(), part of the highest-level layout() call.
TRACE_EVENT0("blink", "FrameView::performPostLayoutTasks");
- RefPtr<FrameView> protect(this);
+ RefPtrWillBeRawPtr<FrameView> protect(this);
m_postLayoutTasksTimer.stop();
@@ -2362,9 +2403,9 @@ Color FrameView::documentBackgroundColor() const
bool FrameView::hasCustomScrollbars() const
{
- const HashSet<RefPtr<Widget> >* viewChildren = children();
- HashSet<RefPtr<Widget> >::const_iterator end = viewChildren->end();
- for (HashSet<RefPtr<Widget> >::const_iterator current = viewChildren->begin(); current != end; ++current) {
+ const WillBeHeapHashSet<RefPtrWillBeMember<Widget> >* viewChildren = children();
+ WillBeHeapHashSet<RefPtrWillBeMember<Widget> >::const_iterator end = viewChildren->end();
+ for (WillBeHeapHashSet<RefPtrWillBeMember<Widget> >::const_iterator current = viewChildren->begin(); current != end; ++current) {
Widget* widget = current->get();
if (widget->isFrameView()) {
if (toFrameView(widget)->hasCustomScrollbars())
@@ -2543,7 +2584,7 @@ void FrameView::updateWidgetPositionsIfNeeded()
void FrameView::updateLayoutAndStyleForPainting()
{
// Updating layout can run script, which can tear down the FrameView.
- RefPtr<FrameView> protector(this);
+ RefPtrWillBeRawPtr<FrameView> protector(this);
updateLayoutAndStyleIfNeededRecursive();
@@ -2591,7 +2632,7 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
// FIXME: Calling layout() shouldn't trigger scripe execution or have any
// observable effects on the frame tree but we're not quite there yet.
- Vector<RefPtr<FrameView> > frameViews;
+ WillBeHeapVector<RefPtrWillBeMember<FrameView> > frameViews;
for (Frame* child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling()) {
if (!child->isLocalFrame())
continue;
@@ -2599,8 +2640,8 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
frameViews.append(view);
}
- const Vector<RefPtr<FrameView> >::iterator end = frameViews.end();
- for (Vector<RefPtr<FrameView> >::iterator it = frameViews.begin(); it != end; ++it)
+ const WillBeHeapVector<RefPtrWillBeMember<FrameView> >::iterator end = frameViews.end();
+ for (WillBeHeapVector<RefPtrWillBeMember<FrameView> >::iterator it = frameViews.begin(); it != end; ++it)
(*it)->updateLayoutAndStyleIfNeededRecursive();
// When an <iframe> gets composited, it triggers an extra style recalc in its containing FrameView.
@@ -2641,9 +2682,28 @@ void FrameView::invalidateTreeIfNeededRecursive()
void FrameView::enableAutoSizeMode(const IntSize& minSize, const IntSize& maxSize)
{
if (!m_autoSizeInfo)
- m_autoSizeInfo = adoptPtr(new FrameViewAutoSizeInfo(this));
+ m_autoSizeInfo = FrameViewAutoSizeInfo::create(this);
m_autoSizeInfo->configureAutoSizeMode(minSize, maxSize);
+ setLayoutSizeFixedToFrameSize(true);
+ setNeedsLayout();
+ scheduleRelayout();
+}
+
+void FrameView::disposeAutoSizeInfo()
+{
+ if (!m_autoSizeInfo)
+ return;
+
+ setLayoutSizeFixedToFrameSize(false);
+ setNeedsLayout();
+ scheduleRelayout();
+
+ // Since autosize mode forces the scrollbar mode, change them to being auto.
+ setVerticalScrollbarLock(false);
+ setHorizontalScrollbarLock(false);
+ setScrollbarModes(ScrollbarAuto, ScrollbarAuto);
+ m_autoSizeInfo.clear();
}
void FrameView::forceLayout(bool allowSubtree)

Powered by Google App Engine
This is Rietveld 408576698