Diff
Modified: branches/safari-534-branch/Source/WebCore/ChangeLog (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/ChangeLog 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/ChangeLog 2011-06-16 23:49:06 UTC (rev 89087)
@@ -1,5 +1,74 @@
2011-06-14 Lucas Forschler <[email protected]>
+ Merged 88982.
+
+ 2011-06-15 Beth Dakin <[email protected]>
+
+ Reviewed by Simon Fraser.
+
+ https://bugs.webkit.org/show_bug.cgi?id=62746
+ Crash possible when switching scrollbar appearance preference on Mac
+ -and corresponding-
+ <rdar://problem/9323983>
+
+ This crash happens because the current mechanism that is intended to flag
+ ScrollAnimators as being in the page cache or not does not work correctly.
+ Long-term the fix for this is to move the ScrollableArea HashSet to a more
+ appropriate place. In the meantime, this patch addresses the crash by getting
+ rid of the m_isActive bool on ScrollAnimator that was intended to represent
+ whether or not the ScrollableArea is in the page cache. Instead, ScrollableArea
+ implementations now have their own functions to compute whether they are in
+ active pages. ScrollAnimator::setIsActive() needs to be kept around even though
+ there is no bool to flip anymore because scrollbars may need to be properly
+ updated if the appearance was switched while the document was in the page cache.
+
+ No longer call FrameView::setAnimatorsAreActive() from
+ Document::setIsInPageCache(), instead call it in
+ Document::documentDidBecomeActive()
+ * dom/Document.cpp:
+ (WebCore::Document::setInPageCache):
+ (WebCore::Document::documentDidBecomeActive):
+
+ ScrollableAreas can now assess whether or not they are on active pages (ie, not
+ in the page cache).
+ * platform/ScrollableArea.h:
+ (WebCore::ScrollableArea::isOnActivePage):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::isOnActivePage):
+ * rendering/RenderLayer.h:
+ * rendering/RenderListBox.cpp:
+ (WebCore::RenderListBox::isOnActivePage):
+ * rendering/RenderListBox.h:
+
+ A FrameView cannot access its Document when it's in the page cache, so it
+ usually determines whether it's in the page cache by checking if its frame
+ points to a FrameView other than itself.
+ * page/FrameView.cpp:
+ (WebCore::FrameView::isOnActivePage):
+
+ Make sure ScrollableAreas are on active pages before setting them as
+ active. This will not be necessary when the HashSet become a per-web page
+ HashSet.
+ (WebCore::FrameView::setAnimatorsAreActive):
+ * page/FrameView.h:
+
+ ScrollAnimator no longer tracks the m_isActive bool.
+ * platform/ScrollAnimator.cpp:
+ (WebCore::ScrollAnimator::ScrollAnimator):
+ * platform/ScrollAnimator.h:
+ (WebCore::ScrollAnimator::setIsActive):
+
+ setIsActive() now exclusively calls updateScrollStyle() if there is a pending
+ need to do so.
+ * platform/mac/ScrollAnimatorMac.h:
+ * platform/mac/ScrollAnimatorMac.mm:
+ (WebCore::ScrollAnimatorMac::setIsActive):
+
+ Return early if the ScrollableArea is in the page cache.
+ (WebCore::ScrollAnimatorMac::updateScrollerStyle):
+
+2011-06-14 Lucas Forschler <[email protected]>
+
Merged 88948.
2011-06-15 Jer Noble <[email protected]>
Modified: branches/safari-534-branch/Source/WebCore/dom/Document.cpp (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/dom/Document.cpp 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/dom/Document.cpp 2011-06-16 23:49:06 UTC (rev 89087)
@@ -3938,9 +3938,6 @@
if (childNeedsStyleRecalc())
scheduleStyleRecalc();
}
-
- if (v)
- v->setAnimatorsAreActive(!m_inPageCache);
}
void Document::documentWillBecomeInactive()
@@ -3966,6 +3963,9 @@
renderView()->didMoveOnscreen();
#endif
+ if (FrameView* frameView = view())
+ frameView->setAnimatorsAreActive();
+
ASSERT(m_frame);
m_frame->loader()->client()->dispatchDidBecomeFrameset(isFrameSet());
}
Modified: branches/safari-534-branch/Source/WebCore/page/FrameView.cpp (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/page/FrameView.cpp 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/page/FrameView.cpp 2011-06-16 23:49:06 UTC (rev 89087)
@@ -2227,12 +2227,19 @@
return page->chrome()->client()->notifyScrollerThumbIsVisibleInRect(scrollerThumb);
}
+bool FrameView::isOnActivePage() const
+{
+ if (m_frame->view() != this)
+ return false;
+ return !m_frame->document()->inPageCache();
+}
+
bool FrameView::shouldSuspendScrollAnimations() const
{
return m_frame->loader()->state() != FrameStateComplete;
}
-void FrameView::setAnimatorsAreActive(bool active)
+void FrameView::setAnimatorsAreActive()
{
Page* page = m_frame->page();
if (!page)
@@ -2243,8 +2250,14 @@
return;
HashSet<ScrollableArea*>::const_iterator end = scrollableAreas->end();
- for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(); it != end; ++it)
- (*it)->scrollAnimator()->setIsActive(active);
+ for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(); it != end; ++it) {
+ // FIXME: This extra check to make sure the ScrollableArea is on an active page needs
+ // to be here as long as the ScrollableArea HashSet lives on Page. But it should really be
+ // moved to the top-level Document or a similar class that really represents a single
+ // web page. https://bugs.webkit.org/show_bug.cgi?id=62762
+ if ((*it)->isOnActivePage())
+ (*it)->scrollAnimator()->setIsActive();
+ }
}
void FrameView::notifyPageThatContentAreaWillPaint() const
Modified: branches/safari-534-branch/Source/WebCore/page/FrameView.h (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/page/FrameView.h 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/page/FrameView.h 2011-06-16 23:49:06 UTC (rev 89087)
@@ -277,7 +277,7 @@
virtual bool shouldSuspendScrollAnimations() const;
- void setAnimatorsAreActive(bool);
+ void setAnimatorsAreActive();
protected:
virtual bool scrollContentsFastPath(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect);
@@ -329,6 +329,7 @@
virtual void didCompleteAnimatedScroll() const;
virtual void scrollbarStyleChanged();
virtual void setVisibleScrollerThumbRect(const IntRect&);
+ virtual bool isOnActivePage() const;
#if USE(ACCELERATED_COMPOSITING)
virtual GraphicsLayer* layerForHorizontalScrollbar() const;
virtual GraphicsLayer* layerForVerticalScrollbar() const;
Modified: branches/safari-534-branch/Source/WebCore/platform/ScrollAnimator.cpp (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/platform/ScrollAnimator.cpp 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/platform/ScrollAnimator.cpp 2011-06-16 23:49:06 UTC (rev 89087)
@@ -52,7 +52,6 @@
: m_scrollableArea(scrollableArea)
, m_currentPosX(0)
, m_currentPosY(0)
- , m_isActive(true)
{
}
Modified: branches/safari-534-branch/Source/WebCore/platform/ScrollAnimator.h (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/platform/ScrollAnimator.h 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/platform/ScrollAnimator.h 2011-06-16 23:49:06 UTC (rev 89087)
@@ -61,8 +61,7 @@
ScrollableArea* scrollableArea() const { return m_scrollableArea; }
- virtual void setIsActive(bool active) { m_isActive = active; }
- bool isActive() const { return m_isActive; }
+ virtual void setIsActive() { }
virtual void handleWheelEvent(PlatformWheelEvent&);
#if ENABLE(GESTURE_EVENTS)
@@ -96,7 +95,6 @@
ScrollableArea* m_scrollableArea;
float m_currentPosX; // We avoid using a FloatPoint in order to reduce
float m_currentPosY; // subclass code complexity.
- bool m_isActive;
};
} // namespace WebCore
Modified: branches/safari-534-branch/Source/WebCore/platform/ScrollableArea.h (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/platform/ScrollableArea.h 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/platform/ScrollableArea.h 2011-06-16 23:49:06 UTC (rev 89087)
@@ -136,6 +136,8 @@
virtual bool shouldSuspendScrollAnimations() const { return true; }
virtual void scrollbarStyleChanged() { }
virtual void setVisibleScrollerThumbRect(const IntRect&) { }
+
+ virtual bool isOnActivePage() const { ASSERT_NOT_REACHED(); return true; }
bool isHorizontalScrollerPinnedToMinimumPosition() const { return !horizontalScrollbar() || scrollPosition(horizontalScrollbar()) <= minimumScrollPosition().x(); }
bool isHorizontalScrollerPinnedToMaximumPosition() const { return !horizontalScrollbar() || scrollPosition(horizontalScrollbar()) >= maximumScrollPosition().x(); }
Modified: branches/safari-534-branch/Source/WebCore/platform/mac/ScrollAnimatorMac.h (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/platform/mac/ScrollAnimatorMac.h 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/platform/mac/ScrollAnimatorMac.h 2011-06-16 23:49:06 UTC (rev 89087)
@@ -83,7 +83,7 @@
bool haveScrolledSincePageLoad() const { return m_haveScrolledSincePageLoad; }
- virtual void setIsActive(bool);
+ virtual void setIsActive();
#if USE(WK_SCROLLBAR_PAINTER)
void updateScrollerStyle();
Modified: branches/safari-534-branch/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2011-06-16 23:49:06 UTC (rev 89087)
@@ -1216,10 +1216,8 @@
}
#endif
-void ScrollAnimatorMac::setIsActive(bool active)
+void ScrollAnimatorMac::setIsActive()
{
- ScrollAnimator::setIsActive(active);
-
#if USE(WK_SCROLLBAR_PAINTER)
if (needsScrollerStyleUpdate())
updateScrollerStyle();
@@ -1229,7 +1227,7 @@
#if USE(WK_SCROLLBAR_PAINTER)
void ScrollAnimatorMac::updateScrollerStyle()
{
- if (!isActive()) {
+ if (!scrollableArea()->isOnActivePage()) {
setNeedsScrollerStyleUpdate(true);
return;
}
Modified: branches/safari-534-branch/Source/WebCore/rendering/RenderLayer.cpp (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/rendering/RenderLayer.cpp 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/rendering/RenderLayer.cpp 2011-06-16 23:49:06 UTC (rev 89087)
@@ -1827,6 +1827,11 @@
return view->frameView()->shouldSuspendScrollAnimations();
}
+bool RenderLayer::isOnActivePage() const
+{
+ return !m_renderer->document()->inPageCache();
+}
+
IntPoint RenderLayer::currentMousePosition() const
{
return renderer()->frame() ? renderer()->frame()->eventHandler()->currentMousePosition() : IntPoint();
Modified: branches/safari-534-branch/Source/WebCore/rendering/RenderLayer.h (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/rendering/RenderLayer.h 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/rendering/RenderLayer.h 2011-06-16 23:49:06 UTC (rev 89087)
@@ -548,6 +548,7 @@
virtual IntPoint currentMousePosition() const;
virtual void didCompleteRubberBand(const IntSize&) const;
virtual bool shouldSuspendScrollAnimations() const;
+ virtual bool isOnActivePage() const;
// Rectangle encompassing the scroll corner and resizer rect.
IntRect scrollCornerAndResizerRect() const;
Modified: branches/safari-534-branch/Source/WebCore/rendering/RenderListBox.cpp (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/rendering/RenderListBox.cpp 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/rendering/RenderListBox.cpp 2011-06-16 23:49:06 UTC (rev 89087)
@@ -803,6 +803,11 @@
return view->frameView()->shouldSuspendScrollAnimations();
}
+bool RenderListBox::isOnActivePage() const
+{
+ return !document()->inPageCache();
+}
+
PassRefPtr<Scrollbar> RenderListBox::createScrollbar()
{
RefPtr<Scrollbar> widget;
Modified: branches/safari-534-branch/Source/WebCore/rendering/RenderListBox.h (89086 => 89087)
--- branches/safari-534-branch/Source/WebCore/rendering/RenderListBox.h 2011-06-16 23:45:15 UTC (rev 89086)
+++ branches/safari-534-branch/Source/WebCore/rendering/RenderListBox.h 2011-06-16 23:49:06 UTC (rev 89087)
@@ -117,6 +117,7 @@
virtual int visibleWidth() const;
virtual IntPoint currentMousePosition() const;
virtual bool shouldSuspendScrollAnimations() const;
+ virtual bool isOnActivePage() const;
virtual void disconnectFromPage() { m_page = 0; }
