Changeset 155529 in webkit

Timestamp:

Sep 11, 2013 8:24:45 AM (11 years ago)

Author:

akling@apple.com

Message:

Page::backForward() should return a reference.
<​https://webkit.org/b/121151>

Reviewed by Anders Carlsson.

There is always a BackForwardController, so make backForward() return a reference.
Also made it store a Page& internally since it's tied to the lifetime of Page.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/history/BackForwardController.cpp (modified) (3 diffs)
• WebCore/history/BackForwardController.h (modified) (2 diffs)
• WebCore/history/PageCache.cpp (modified) (2 diffs)
• WebCore/loader/FrameLoader.cpp (modified) (5 diffs)
• WebCore/loader/HistoryController.cpp (modified) (3 diffs)
• WebCore/loader/NavigationScheduler.cpp (modified) (2 diffs)
• WebCore/page/ContextMenuController.cpp (modified) (3 diffs)
• WebCore/page/DOMWindow.cpp (modified) (1 diff)
• WebCore/page/History.cpp (modified) (1 diff)
• WebCore/page/Page.cpp (modified) (7 diffs)
• WebCore/page/Page.h (modified) (2 diffs)
• WebCore/page/Settings.cpp (modified) (1 diff)
• WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm (modified) (1 diff)
• WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp (modified) (1 diff)
• WebKit2/WebProcess/InjectedBundle/InjectedBundleBackForwardList.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-09-11  Andreas Kling  <akling@apple.com>
+
+        Page::backForward() should return a reference.
+        <https://webkit.org/b/121151>
+
+        Reviewed by Anders Carlsson.
+
+        There is always a BackForwardController, so make backForward() return a reference.
+        Also made it store a Page& internally since it's tied to the lifetime of Page.
+
 2013-09-11  Mikhail Pozdnyakov  <mikhail.pozdnyakov@intel.com>
 

trunk/Source/WebCore/history/BackForwardController.cpp

@@ -33 +33 @@
 namespace WebCore {
 
-BackForwardController::BackForwardController(Page* page, PassRefPtr<BackForwardClient> client)
+BackForwardController::BackForwardController(Page& page, PassRefPtr<BackForwardClient> client)
     : m_page(page)
     , m_client(client)
 {
     if (!m_client)
-        m_client = BackForwardList::create(page);
+        m_client = BackForwardList::create(&page);
 }
 
@@ -45 +45 @@
 }
 
-PassOwnPtr<BackForwardController> BackForwardController::create(Page* page, PassRefPtr<BackForwardClient> client)
-{
-    return adoptPtr(new BackForwardController(page, client));
-}
-
 bool BackForwardController::canGoBackOrForward(int distance) const
 {
-    return m_page->canGoBackOrForward(distance);
+    return m_page.canGoBackOrForward(distance);
 }
 
 void BackForwardController::goBackOrForward(int distance)
 {
-    m_page->goBackOrForward(distance);
+    m_page.goBackOrForward(distance);
 }
 
 bool BackForwardController::goBack()
 {
-    return m_page->goBack();
+    return m_page.goBack();
 }
 
 bool BackForwardController::goForward()
 {
-    return m_page->goForward();
+    return m_page.goForward();
 }
 
@@ -82 +77 @@
 int BackForwardController::count() const
 {
-    return m_page->getHistoryLength();
+    return m_page.getHistoryLength();
 }
 

trunk/Source/WebCore/history/BackForwardController.h

@@ -40 +40 @@
     WTF_MAKE_NONCOPYABLE(BackForwardController); WTF_MAKE_FAST_ALLOCATED;
 public:
+    BackForwardController(Page&, PassRefPtr<BackForwardClient>);
     ~BackForwardController();
-
-    static PassOwnPtr<BackForwardController> create(Page*, PassRefPtr<BackForwardClient>);
 
     BackForwardClient* client() const { return m_client.get(); }
@@ -70 +69 @@
 
 private:
-    BackForwardController(Page*, PassRefPtr<BackForwardClient>);
-
-    Page* m_page;
+    Page& m_page;
     RefPtr<BackForwardClient> m_client;
 };

trunk/Source/WebCore/history/PageCache.cpp

@@ -218 +218 @@
         rejectReasons |= 1 << FrameCannotBeInPageCache;
     
-    if (!page->backForward()->isActive()) {
+    if (!page->backForward().isActive()) {
         PCLOG("   -The back/forward list is disabled or has 0 capacity");
         rejectReasons |= 1 << DisabledBackForwardList;
@@ -356 +356 @@
     return m_capacity > 0
         && canCachePageContainingThisFrame(&page->mainFrame())
-        && page->backForward()->isActive()
+        && page->backForward().isActive()
         && page->settings().usesPageCache()
 #if ENABLE(DEVICE_ORIENTATION)

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -1124 +1124 @@
         currentItem->setLastVisitWasFailure(true);
         history().setCurrentItem(currentItem.get());
-        m_frame.page()->backForward()->setCurrentItem(currentItem.get());
+        m_frame.page()->backForward().setCurrentItem(currentItem.get());
 
         ASSERT(stateMachine()->isDisplayingInitialEmptyDocument());
@@ -2150 +2150 @@
             if (shouldReset && item)
                 if (Page* page = m_frame.page()) {
-                    page->backForward()->setCurrentItem(item.get());
+                    page->backForward().setCurrentItem(item.get());
                     m_frame.loader().client().updateGlobalHistoryItemForPage();
                 }
@@ -2237 +2237 @@
 
     KURL originalURL;
-    int backCount = page->backForward()->backCount();
+    int backCount = page->backForward().backCount();
     for (int backIndex = 0; backIndex <= backCount; backIndex++) {
         // FIXME: At one point we had code here to check a "was user gesture" flag.
         // Do we need to restore that logic?
-        HistoryItem* historyItem = page->backForward()->itemAtIndex(-backIndex);
+        HistoryItem* historyItem = page->backForward().itemAtIndex(-backIndex);
         if (!historyItem)
             continue;
@@ -2844 +2844 @@
             if (Page* page = m_frame.page()) {
                 if (HistoryItem* resetItem = page->mainFrame().loader().history().currentItem()) {
-                    page->backForward()->setCurrentItem(resetItem);
+                    page->backForward().setCurrentItem(resetItem);
                     m_frame.loader().client().updateGlobalHistoryItemForPage();
                 }
@@ -3061 +3061 @@
         return;
 
-    if (!m_didPerformFirstNavigation && page->backForward()->currentItem() && !page->backForward()->backItem() && !page->backForward()->forwardItem()) {
+    if (!m_didPerformFirstNavigation && page->backForward().currentItem() && !page->backForward().backItem() && !page->backForward().forwardItem()) {
         m_didPerformFirstNavigation = true;
         m_client.didPerformFirstNavigation();

trunk/Source/WebCore/loader/HistoryController.cpp

@@ -278 +278 @@
     // - plus, it only makes sense for the top level of the operation through the frametree,
     // as opposed to happening for some/one of the page commits that might happen soon
-    RefPtr<HistoryItem> currentItem = page->backForward()->currentItem();
-    page->backForward()->setCurrentItem(targetItem);
+    RefPtr<HistoryItem> currentItem = page->backForward().currentItem();
+    page->backForward().setCurrentItem(targetItem);
     m_frame.loader().client().updateGlobalHistoryItemForPage();
 
@@ -803 +803 @@
     RefPtr<HistoryItem> topItem = frameLoader.history().createItemTree(m_frame, doClip);
     LOG(BackForward, "WebCoreBackForward - Adding backforward item %p for frame %s", topItem.get(), m_frame.loader().documentLoader()->url().string().ascii().data());
-    page->backForward()->addItem(topItem.release());
+    page->backForward().addItem(topItem.release());
 }
 
@@ -848 +848 @@
     m_currentItem->setURLString(urlString);
 
-    page->backForward()->addItem(topItem.release());
+    page->backForward().addItem(topItem.release());
 
     if (m_frame.settings().privateBrowsingEnabled())

trunk/Source/WebCore/loader/NavigationScheduler.cpp

@@ -206 +206 @@
         // go(i!=0) from a frame navigates into the history of the frame only,
         // in both IE and NS (but not in Mozilla). We can't easily do that.
-        frame->page()->backForward()->goBackOrForward(m_historySteps);
+        frame->page()->backForward().goBackOrForward(m_historySteps);
     }
 
@@ -403 +403 @@
     // Invalid history navigations (such as history.forward() during a new load) have the side effect of cancelling any scheduled
     // redirects. We also avoid the possibility of cancelling the current load by avoiding the scheduled redirection altogether.
-    BackForwardController* backForward = m_frame->page()->backForward();
-    if (steps > backForward->forwardCount() || -steps > backForward->backCount()) {
+    BackForwardController& backForward = m_frame->page()->backForward();
+    if (steps > backForward.forwardCount() || -steps > backForward.backCount()) {
         cancel();
         return;

trunk/Source/WebCore/page/ContextMenuController.cpp

@@ -293 +293 @@
     case ContextMenuItemTagGoBack:
         if (Page* page = frame->page())
-            page->backForward()->goBackOrForward(-1);
+            page->backForward().goBackOrForward(-1);
         break;
     case ContextMenuItemTagGoForward:
         if (Page* page = frame->page())
-            page->backForward()->goBackOrForward(1);
+            page->backForward().goBackOrForward(1);
         break;
     case ContextMenuItemTagStop:
@@ -913 +913 @@
                 appendItem(ReloadItem, m_contextMenu.get());
 #else
-                if (frame->page() && frame->page()->backForward()->canGoBackOrForward(-1))
+                if (frame->page() && frame->page()->backForward().canGoBackOrForward(-1))
                     appendItem(BackItem, m_contextMenu.get());
 
-                if (frame->page() && frame->page()->backForward()->canGoBackOrForward(1))
+                if (frame->page() && frame->page()->backForward().canGoBackOrForward(1))
                     appendItem(ForwardItem, m_contextMenu.get());
 
@@ -1294 +1294 @@
 #if PLATFORM(GTK)
         case ContextMenuItemTagGoBack:
-            shouldEnable = frame->page() && frame->page()->backForward()->canGoBackOrForward(-1);
+            shouldEnable = frame->page() && frame->page()->backForward().canGoBackOrForward(-1);
             break;
         case ContextMenuItemTagGoForward:
-            shouldEnable = frame->page() && frame->page()->backForward()->canGoBackOrForward(1);
+            shouldEnable = frame->page() && frame->page()->backForward().canGoBackOrForward(1);
             break;
         case ContextMenuItemTagStop:

trunk/Source/WebCore/page/DOMWindow.cpp

@@ -973 +973 @@
     bool allowScriptsToCloseWindows = m_frame->settings().allowScriptsToCloseWindows();
 
-    if (!(page->openedByDOM() || page->backForward()->count() <= 1 || allowScriptsToCloseWindows)) {
+    if (!(page->openedByDOM() || page->backForward().count() <= 1 || allowScriptsToCloseWindows)) {
         pageConsole()->addMessage(JSMessageSource, WarningMessageLevel, ASCIILiteral("Can't close the window since it was not opened by JavaScript"));
         return;

trunk/Source/WebCore/page/History.cpp

@@ -54 +54 @@
     if (!m_frame->page())
         return 0;
-    return m_frame->page()->backForward()->count();
+    return m_frame->page()->backForward().count();
 }
 

trunk/Source/WebCore/page/Page.cpp

@@ -142 +142 @@
     , m_settings(Settings::create(this))
     , m_progress(ProgressTracker::create())
-    , m_backForwardController(BackForwardController::create(this, pageClients.backForwardClient))
+    , m_backForwardController(createOwned<BackForwardController>(*this, pageClients.backForwardClient))
     , m_mainFrame(Frame::create(this, 0, pageClients.loaderClientForMainFrame))
     , m_theme(RenderTheme::themeForPage(this))
@@ -231 +231 @@
         m_scrollingCoordinator->pageDestroyed();
 
-    backForward()->close();
+    backForward().close();
 
 #ifndef NDEBUG
@@ -357 +357 @@
 BackForwardClient* Page::backForwardClient() const
 {
-    return m_backForwardController->client();
+    return backForward().client();
 }
 
 bool Page::goBack()
 {
-    HistoryItem* item = backForward()->backItem();
+    HistoryItem* item = backForward().backItem();
     
     if (item) {
@@ -373 +373 @@
 bool Page::goForward()
 {
-    HistoryItem* item = backForward()->forwardItem();
+    HistoryItem* item = backForward().forwardItem();
     
     if (item) {
@@ -386 +386 @@
     if (distance == 0)
         return true;
-    if (distance > 0 && distance <= backForward()->forwardCount())
+    if (distance > 0 && distance <= backForward().forwardCount())
         return true;
-    if (distance < 0 && -distance <= backForward()->backCount())
+    if (distance < 0 && -distance <= backForward().backCount())
         return true;
     return false;
@@ -398 +398 @@
         return;
 
-    HistoryItem* item = backForward()->itemAtIndex(distance);
+    HistoryItem* item = backForward().itemAtIndex(distance);
     if (!item) {
         if (distance > 0) {
-            if (int forwardCount = backForward()->forwardCount()) 
-                item = backForward()->itemAtIndex(forwardCount);
+            if (int forwardCount = backForward().forwardCount())
+                item = backForward().itemAtIndex(forwardCount);
         } else {
-            if (int backCount = backForward()->backCount())
-                item = backForward()->itemAtIndex(-backCount);
+            if (int backCount = backForward().backCount())
+                item = backForward().itemAtIndex(-backCount);
         }
     }
@@ -429 +429 @@
 int Page::getHistoryLength()
 {
-    return backForward()->backCount() + 1 + backForward()->forwardCount();
+    return backForward().backCount() + 1 + backForward().forwardCount();
 }
 

trunk/Source/WebCore/page/Page.h

@@ -213 +213 @@
     Settings& settings() const { return *m_settings; }
     ProgressTracker& progress() const { return *m_progress; }
-    BackForwardController* backForward() const { return m_backForwardController.get(); }
+    BackForwardController& backForward() const { return *m_backForwardController; }
 
     FeatureObserver* featureObserver() { return &m_featureObserver; }
@@ -464 +464 @@
     const OwnPtr<ProgressTracker> m_progress;
 
-    OwnPtr<BackForwardController> m_backForwardController;
+    const OwnPtr<BackForwardController> m_backForwardController;
     const RefPtr<Frame> m_mainFrame;
 

trunk/Source/WebCore/page/Settings.cpp

@@ -469 +469 @@
     m_usesPageCache = usesPageCache;
     if (!m_usesPageCache) {
-        int first = -m_page->backForward()->backCount();
-        int last = m_page->backForward()->forwardCount();
+        int first = -m_page->backForward().backCount();
+        int last = m_page->backForward().forwardCount();
         for (int i = first; i <= last; i++)
-            pageCache()->remove(m_page->backForward()->itemAtIndex(i));
+            pageCache()->remove(m_page->backForward().itemAtIndex(i));
     }
 }

trunk/Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm

@@ -978 +978 @@
     if (Page* page = core(m_webFrame.get())->page()) {
         if (!page->settings().privateBrowsingEnabled())
-            historyItem = page->backForward()->currentItem();
+            historyItem = page->backForward().currentItem();
     }
 

trunk/Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp

@@ -826 +826 @@
     if (Page* page = webView->page()) {
         if (!page->settings().privateBrowsingEnabled())
-            historyItem = page->backForward()->currentItem();
+            historyItem = page->backForward().currentItem();
     }
 

trunk/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleBackForwardList.cpp

@@ -44 +44 @@
     if (!page)
         return 0;
-    return InjectedBundleBackForwardListItem::create(page->backForward()->itemAtIndex(index));
+    return InjectedBundleBackForwardListItem::create(page->backForward().itemAtIndex(index));
 }
 
@@ -54 +54 @@
     if (!page)
         return 0;
-    return page->backForward()->backCount();
+    return page->backForward().backCount();
 }
 
@@ -64 +64 @@
     if (!page)
         return 0;
-    return page->backForward()->forwardCount();
+    return page->backForward().forwardCount();
 }
 
@@ -74 +74 @@
     if (!page)
         return;
-    static_cast<WebBackForwardListProxy*>(page->backForward()->client())->clear();
+    static_cast<WebBackForwardListProxy*>(page->backForward().client())->clear();
 }