Changeset 21454 in webkit

Timestamp:

May 14, 2007, 6:30:41 AM (18 years ago)

Author:

darin

Message:

Reviewed by Antti.

• fix <rdar://problem/3109180> VIP: after go back, page jumps after loading even if I scrolled while loading

• history/CachedPage.cpp: (WebCore::CachedPage::clear): Call clearFrame rather than clearPart -- the "part" term was leftover prehistoric terminology.

• loader/FrameLoader.cpp: (WebCore::FrameLoader::open): Call setWasScrolledByUser(false) on the page, so that we can detect any user scrolling that happens during the "go back" process. (WebCore::FrameLoader::restoreScrollPositionAndViewState): Don't scroll if the view's wasScrolledByUser flag is set. (WebCore::FrameLoader::loadItem): Call setWasScrolledByUser(false) false when we do a new "load" (actually just a scroll) using the same frame. Also moved the line of code that sets m_currentHistoryItem to make this code path mroe similar to recursiveGoToItem. (WebCore::FrameLoader::recursiveGoToItem): Ditto.

• page/Frame.cpp: (WebCore::Frame::~Frame): Call clearFrame rather than stting m_frame directly. (WebCore::Frame::sendScrollEvent): Call setWasScrolledByUser(true) on FrameView. The FrameView itself knows to ignore this if the scrolling is being done programmatically instead of by the user.

• page/FrameView.h: Added wasScrolledByUser and setWasScrolledByUser. Also removed unneeded include and class forward declarations, renamed clearPart to clearFrame, removed uneeded function haveDelayedLayoutScheduled, and removed unneeded friend declarations.
• page/FrameView.cpp: (WebCore::FrameViewPrivate::FrameViewPrivate): Added m_inProgrammaticScroll and m_wasScrolledByUser. (WebCore::FrameViewPrivate::reset): Set m_wasScrolledByUser to false. (WebCore::FrameView::clearFrame): Renamed from clearPart. (WebCore::FrameView::scrollRectIntoViewRecursively): Added code to set the m_inProgrammaticScroll flag. (WebCore::FrameView::setContentsPos): Ditto. (WebCore::FrameView::wasScrolledByUser): Added. (WebCore::FrameView::setWasScrolledByUser): Added. Does nothing if called when m_inProgrammaticScroll is true.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• history/CachedPage.cpp (modified) (1 diff)
• loader/FrameLoader.cpp (modified) (8 diffs)
• page/Frame.cpp (modified) (2 diffs)
• page/FrameView.cpp (modified) (7 diffs)
• page/FrameView.h (modified) (12 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-05-14  Darin Adler  <darin@apple.com>
+
+        Reviewed by Antti.
+
+        - fix <rdar://problem/3109180> VIP: after go back, page jumps after loading even if I scrolled while loading
+
+        * history/CachedPage.cpp: (WebCore::CachedPage::clear): Call clearFrame rather than
+        clearPart -- the "part" term was leftover prehistoric terminology.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::open): Call setWasScrolledByUser(false) on the page, so that we
+        can detect any user scrolling that happens during the "go back" process.
+        (WebCore::FrameLoader::restoreScrollPositionAndViewState): Don't scroll if the view's
+        wasScrolledByUser flag is set.
+        (WebCore::FrameLoader::loadItem): Call setWasScrolledByUser(false) false when we do
+        a new "load" (actually just a scroll) using the same frame. Also moved the line of code that
+        sets m_currentHistoryItem to make this code path mroe similar to recursiveGoToItem.
+        (WebCore::FrameLoader::recursiveGoToItem): Ditto.
+
+        * page/Frame.cpp:
+        (WebCore::Frame::~Frame): Call clearFrame rather than stting m_frame directly.
+        (WebCore::Frame::sendScrollEvent): Call setWasScrolledByUser(true) on FrameView. The FrameView
+        itself knows to ignore this if the scrolling is being done programmatically instead of by the user.
+
+        * page/FrameView.h: Added wasScrolledByUser and setWasScrolledByUser. Also removed unneeded
+        include and class forward declarations, renamed clearPart to clearFrame, removed uneeded function
+        haveDelayedLayoutScheduled, and removed unneeded friend declarations.
+        * page/FrameView.cpp:
+        (WebCore::FrameViewPrivate::FrameViewPrivate): Added m_inProgrammaticScroll and m_wasScrolledByUser.
+        (WebCore::FrameViewPrivate::reset): Set m_wasScrolledByUser to false.
+        (WebCore::FrameView::clearFrame): Renamed from clearPart.
+        (WebCore::FrameView::scrollRectIntoViewRecursively): Added code to set the m_inProgrammaticScroll flag.
+        (WebCore::FrameView::setContentsPos): Ditto.
+        (WebCore::FrameView::wasScrolledByUser): Added.
+        (WebCore::FrameView::setWasScrolledByUser): Added. Does nothing if called when m_inProgrammaticScroll
+        is true.
+
 2007-05-14  Darin Adler  <darin@apple.com>
 

trunk/WebCore/history/CachedPage.cpp

@@ -145 +145 @@
         }
 
-        m_view->clearPart();
+        m_view->clearFrame();
     }
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -2559 +2559 @@
     m_wasLoadEventEmitted = false;
     m_outgoingReferrer = URL.url();
-    
-    m_frame->setView(cachedPage.view());
+
+    FrameView* view = cachedPage.view();
+    if (view)
+        view->setWasScrolledByUser(false);
+    m_frame->setView(view);
     
     m_frame->setDocument(document);
@@ -3589 +3592 @@
     }
 
-    LOG (History, "WebCoreHistory: Creating item for %s", url.url().ascii());
+    LOG(History, "WebCoreHistory: Creating item for %s", url.url().ascii());
     
     // Frames that have never successfully loaded any content
@@ -3680 +3683 @@
         return;
     
-    // FIXME: It would be great to work out a way to put this code in WebCore instead of calling through to the client.
+    // FIXME: It would be great to work out a way to put this code in WebCore instead of calling
+    // through to the client. It's currently used only for the PDF view on Mac.
     m_client->restoreViewState();
     
-    if (m_frame->view()) {
-        const IntPoint& scrollPoint = m_currentHistoryItem->scrollPoint();
-        m_frame->view()->setContentsPos(scrollPoint.x(), scrollPoint.y());
-    }
+    if (FrameView* view = m_frame->view())
+        if (!view->wasScrolledByUser()) {
+            const IntPoint& scrollPoint = m_currentHistoryItem->scrollPoint();
+            view->setContentsPos(scrollPoint.x(), scrollPoint.y());
+        }
 }
 
@@ -3785 +3790 @@
     // We also do not do anchor-style navigation if we're posting a form.
     
-    // FIXME: These checks don't match the ones in _loadURL:referrer:loadType:target:triggeringEvent:isFormSubmission:
-    // Perhaps they should.
     if (!formData && !shouldReload(itemURL, currentURL) && urlsMatchItem(item)) {
-#if 0
-        // FIXME:  We need to normalize the code paths for anchor navigation.  Something
-        // like the following line of code should be done, but also accounting for correct
-        // updates to the back/forward list and scroll position.
-        // rjw 4/9/03 See 3223929.
-        [self _loadURL:itemURL referrer:[[[self dataSource] request] HTTPReferrer] loadType:loadType target:nil triggeringEvent:nil form:nil formValues:nil];
-#endif
-
         // Must do this maintenance here, since we don't go through a real page reload
         saveScrollPositionAndViewStateToItem(m_currentHistoryItem.get());
 
-        // FIXME: form state might want to be saved here too
+        if (FrameView* view = m_frame->view())
+            view->setWasScrolledByUser(false);
+
+        m_currentHistoryItem = item;
+
+        // FIXME: Form state might need to be saved here too.
 
         // We always call scrollToAnchor here, even if the URL doesn't have an
@@ -3806 +3806 @@
     
         // must do this maintenance here, since we don't go through a real page reload
-        m_currentHistoryItem = item;
         restoreScrollPositionAndViewState();
         
@@ -3830 +3829 @@
             double interval = currentTime() - cachedPage->timeStamp();
             
-            // FIXME: 1800 is the current backforwardcache expiration time, but we actually store as a pref -
-            // previously, this code was -
-            //if (interval <= [[getWebView(self) preferences] _backForwardCacheExpirationInterval]) {
+            // FIXME: 1800 should not be hardcoded, it should come from
+            // WebKitBackForwardCacheExpirationIntervalKey in WebKit.
+            // Or we should remove WebKitBackForwardCacheExpirationIntervalKey.
             if (interval <= 1800) {
                 load(cachedPage->documentLoader(), loadType, 0);   
                 inPageCache = true;
             } else {
-                LOG (PageCache, "Not restoring page for %s from back/forward cache because cache entry has expired", m_provisionalHistoryItem->url().url().ascii());
+                LOG(PageCache, "Not restoring page for %s from back/forward cache because cache entry has expired", m_provisionalHistoryItem->url().url().ascii());
                 item->setCachedPage(0);
             }
@@ -3971 +3970 @@
         saveDocumentState();
         saveScrollPositionAndViewStateToItem(m_currentHistoryItem.get());
+
+        if (FrameView* view = m_frame->view())
+            view->setWasScrolledByUser(false);
+
         m_currentHistoryItem = item;
                 
@@ -3976 +3979 @@
         restoreDocumentState();
         
-        // Restore the scroll position (taken in favor of going back to the anchor)
+        // Restore the scroll position (we choose to do this rather than going back to the anchor point)
         restoreScrollPositionAndViewState();
         

trunk/WebCore/page/Frame.cpp

@@ -243 +243 @@
     if (d->m_view) {
         d->m_view->hide();
-        d->m_view->m_frame = 0;
+        d->m_view->clearFrame();
     }
   
@@ -1397 +1397 @@
 void Frame::sendScrollEvent()
 {
-    FrameView *v = d->m_view.get();
-    if (v) {
-        Document *doc = document();
-        if (!doc)
-            return;
-        doc->dispatchHTMLEvent(scrollEvent, true, false);
-    }
+    FrameView* v = d->m_view.get();
+    if (!v)
+        return;
+    v->setWasScrolledByUser(true);
+    Document* doc = document();
+    if (!doc)
+        return;
+    doc->dispatchHTMLEvent(scrollEvent, true, false);
 }
 

trunk/WebCore/page/FrameView.cpp

@@ -61 +61 @@
         , m_overflowStatusDirty(true)
         , m_viewportRenderer(0)
+        , m_wasScrolledByUser(false)
+        , m_inProgrammaticScroll(false)
     {
         isTransparent = false;
@@ -81 +83 @@
         firstLayout = true;
         repaintRects.clear();
+        m_wasScrolledByUser = false;
     }
 
@@ -117 +120 @@
     bool m_verticalOverflow;    
     RenderObject* m_viewportRenderer;
+
+    bool m_wasScrolledByUser;
+    bool m_inProgrammaticScroll;
 };
 
@@ -151 +157 @@
 }
 
-void FrameView::clearPart()
+void FrameView::clearFrame()
 {
     m_frame = 0;
@@ -600 +606 @@
     if (frame()->prohibitsScrolling())
         return;
+    bool wasInProgrammaticScroll = d->m_inProgrammaticScroll;
+    d->m_inProgrammaticScroll = true;
     ScrollView::scrollRectIntoViewRecursively(r);
+    d->m_inProgrammaticScroll = wasInProgrammaticScroll;
 }
 
 void FrameView::setContentsPos(int x, int y)
 {
+    ASSERT(!d->m_inProgrammaticScroll);
     if (frame()->prohibitsScrolling())
         return;
+    bool wasInProgrammaticScroll = d->m_inProgrammaticScroll;
+    d->m_inProgrammaticScroll = true;
     ScrollView::setContentsPos(x, y);
+    d->m_inProgrammaticScroll = wasInProgrammaticScroll;
 }
 
@@ -703 +716 @@
 }
 
-bool FrameView::haveDelayedLayoutScheduled()
-{
-    return d->layoutTimer.isActive() && d->delayedLayout;
-}
-
 void FrameView::unscheduleRelayout()
 {
@@ -902 +910 @@
 }
 
-}
+bool FrameView::wasScrolledByUser() const
+{
+    return d->m_wasScrolledByUser;
+}
+
+void FrameView::setWasScrolledByUser(bool wasScrolledByUser)
+{
+    if (d->m_inProgrammaticScroll)
+        return;
+    d->m_wasScrolledByUser = wasScrolledByUser;
+}
+
+}

trunk/WebCore/page/FrameView.h

@@ -1 @@
-/* This file is part of the KDE project
-
+/*
    Copyright (C) 1997 Martin Jones (mjones@kde.org)
              (C) 1998 Waldo Bastian (bastian@kde.org)
@@ -6 +5 @@
              (C) 1999 Lars Knoll (knoll@kde.org)
              (C) 1999 Antti Koivisto (koivisto@kde.org)
-   Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
+   Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
 
    This library is free software; you can redistribute it and/or
@@ -29 +28 @@
 #include "ScrollView.h"
 #include "IntSize.h"
-#include "PlatformString.h"
+#include <wtf/Forward.h>
 
 namespace WebCore {
 
-class AtomicString;
 class Color;
-class Clipboard;
 class Event;
 class EventTargetNode;
 class Frame;
 class FrameViewPrivate;
-class HTMLFrameSetElement;
-class IntPoint;
 class IntRect;
-class KeyboardEvent;
 class PlatformMouseEvent;
-class MouseEventWithHitTestResults;
 class Node;
 class RenderLayer;
 class RenderObject;
-class PlatformWheelEvent;
+class String;
 
 template <typename T> class Timer;
@@ -59 +52 @@
 
     Frame* frame() const { return m_frame.get(); }
+    void clearFrame();
 
     void ref() { ++m_refCount; }
@@ -72 +66 @@
     virtual void setHScrollbarMode(ScrollbarMode);
     virtual void setScrollbarsMode(ScrollbarMode);
-    
+
     void layout(bool allowSubtree = true);
     bool didFirstLayout() const;
@@ -79 +73 @@
     void scheduleRelayoutOfSubtree(Node*);
     void unscheduleRelayout();
-    bool haveDelayedLayoutScheduled();
     bool layoutPending() const;
 
@@ -96 +89 @@
 
     void clear();
-    void clearPart();
 
     bool isTransparent() const;
@@ -111 +103 @@
     IntRect windowClipRectForLayer(const RenderLayer*, bool clipToLayerContents) const;
 
-    virtual void scrollRectIntoViewRecursively(const IntRect& r);
+    virtual void scrollRectIntoViewRecursively(const IntRect&);
     virtual void setContentsPos(int x, int y);
 
     String mediaType() const;
+    void setMediaType(const String&);
 
     void setUseSlowRepaints();
@@ -126 +119 @@
     void restoreScrollbar();
 
-    void setMediaType(const String&);
-
     virtual bool handleMouseMoveEvent(const PlatformMouseEvent&);
     virtual bool handleMouseReleaseEvent(const PlatformMouseEvent&);
@@ -134 +125 @@
     void pauseScheduledEvents();
     void resumeScheduledEvents();
+
+    bool wasScrolledByUser() const;
+    void setWasScrolledByUser(bool);
 
 private:
@@ -157 +151 @@
     RefPtr<Frame> m_frame;
     FrameViewPrivate* d;
-
-    friend class Frame;
-    friend class FrameMac;
-
 };
 
@@ -166 +156 @@
 
 #endif
-