Cached Page and Frame don't need to be ref-counted.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Sep 2013 01:41:18 +0000 (01:41 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Sep 2013 01:41:18 +0000 (01:41 +0000)
<https://webkit.org/b/120710>

Reviewed by Anders Carlsson.

- CachedPage is owned by HistoryItem.
- CachedFrame is owned by CachedPage.

Remove the ref counting from these objects to make the code less confusing.

The only place that used this was in FrameLoader::commitProvisionalLoad() which
took a temporary ref on the CachedPage. Switched this to using a raw pointer.

* history/CachedFrame.h:
(WebCore::CachedFrame::create):
* history/CachedPage.cpp:
(WebCore::CachedPage::create):
* history/CachedPage.h:
* history/HistoryItem.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::commitProvisionalLoad):
(WebCore::FrameLoader::transitionToCommitted):
* loader/FrameLoader.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@155086 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/history/CachedFrame.h
Source/WebCore/history/CachedPage.cpp
Source/WebCore/history/CachedPage.h
Source/WebCore/history/HistoryItem.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h

index bd0fc32..4046acc 100644 (file)
@@ -1,3 +1,29 @@
+2013-09-04  Andreas Kling  <akling@apple.com>
+
+        Cached Page and Frame don't need to be ref-counted.
+        <https://webkit.org/b/120710>
+
+        Reviewed by Anders Carlsson.
+
+        - CachedPage is owned by HistoryItem.
+        - CachedFrame is owned by CachedPage.
+
+        Remove the ref counting from these objects to make the code less confusing.
+
+        The only place that used this was in FrameLoader::commitProvisionalLoad() which
+        took a temporary ref on the CachedPage. Switched this to using a raw pointer.
+
+        * history/CachedFrame.h:
+        (WebCore::CachedFrame::create):
+        * history/CachedPage.cpp:
+        (WebCore::CachedPage::create):
+        * history/CachedPage.h:
+        * history/HistoryItem.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::commitProvisionalLoad):
+        (WebCore::FrameLoader::transitionToCommitted):
+        * loader/FrameLoader.h:
+
 2013-09-04  Roger Fong  <roger_fong@apple.com>
 
         Unreviewed. Windows build fix and WebCore project cleanup.
index 463fa7f..9277028 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2009, 2010, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -30,7 +30,6 @@
 #include "KURL.h"
 #include "ScriptCachedFrameData.h"
 #include <wtf/PassOwnPtr.h>
-#include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
 
 namespace WebCore {
@@ -42,8 +41,6 @@ class DocumentLoader;
 class FrameView;
 class Node;
 
-typedef Vector<RefPtr<CachedFrame> > CachedFrameVector;
-
 class CachedFrameBase {
 public:
     void restore();
@@ -69,12 +66,12 @@ protected:
     bool m_isComposited;
 #endif
     
-    CachedFrameVector m_childFrames;
+    Vector<OwnPtr<CachedFrame>> m_childFrames;
 };
 
-class CachedFrame : public RefCounted<CachedFrame>, private CachedFrameBase {
+class CachedFrame : private CachedFrameBase {
 public:
-    static PassRefPtr<CachedFrame> create(Frame* frame) { return adoptRef(new CachedFrame(frame)); }
+    static PassOwnPtr<CachedFrame> create(Frame* frame) { return adoptPtr(new CachedFrame(frame)); }
 
     void open();
     void clear();
index f7d93b4..056bbea 100644 (file)
@@ -45,9 +45,9 @@ namespace WebCore {
 
 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, cachedPageCounter, ("CachedPage"));
 
-PassRefPtr<CachedPage> CachedPage::create(Page* page)
+PassOwnPtr<CachedPage> CachedPage::create(Page* page)
 {
-    return adoptRef(new CachedPage(page));
+    return adoptPtr(new CachedPage(page));
 }
 
 CachedPage::CachedPage(Page* page)
index 0f08aa6..eda704b 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2006, 2007, 2008, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -27,7 +27,6 @@
 #define CachedPage_h
 
 #include "CachedFrame.h"
-#include <wtf/RefCounted.h>
 
 namespace WebCore {
     
@@ -35,9 +34,9 @@ class Document;
 class DocumentLoader;
 class Page;
 
-class CachedPage : public RefCounted<CachedPage> {
+class CachedPage {
 public:
-    static PassRefPtr<CachedPage> create(Page*);
+    static PassOwnPtr<CachedPage> create(Page*);
     ~CachedPage();
 
     void restore(Page*);
@@ -67,7 +66,7 @@ private:
 
     double m_timeStamp;
     double m_expirationTime;
-    RefPtr<CachedFrame> m_cachedMainFrame;
+    OwnPtr<CachedFrame> m_cachedMainFrame;
     bool m_needStyleRecalcForVisitedLinks;
     bool m_needsFullStyleRecalc;
     bool m_needsCaptionPreferencesChanged;
index 7fe4511..573c1b1 100644 (file)
@@ -286,7 +286,7 @@ private:
     // PageCache controls these fields.
     HistoryItem* m_next;
     HistoryItem* m_prev;
-    RefPtr<CachedPage> m_cachedPage;
+    OwnPtr<CachedPage> m_cachedPage;
     
 #if PLATFORM(MAC)
     RetainPtr<id> m_viewState;
index ae050bd..f311527 100644 (file)
@@ -1699,7 +1699,7 @@ void FrameLoader::clearProvisionalLoad()
 
 void FrameLoader::commitProvisionalLoad()
 {
-    RefPtr<CachedPage> cachedPage = m_loadingFromCachedPage ? pageCache()->get(history().provisionalItem()) : 0;
+    CachedPage* cachedPage = m_loadingFromCachedPage ? pageCache()->get(history().provisionalItem()) : 0;
     RefPtr<DocumentLoader> pdl = m_provisionalDocumentLoader;
     Ref<Frame> protect(m_frame);
 
@@ -1743,6 +1743,9 @@ void FrameLoader::commitProvisionalLoad()
         // The page should be removed from the cache immediately after a restoration in order for the PageCache to be consistent.
         pageCache()->remove(history().currentItem());
 
+        // Clear out 'cachedPage' right away since it now points to a deleted object.
+        cachedPage = nullptr;
+
         dispatchDidCommitLoad();
 
         // If we have a title let the WebView know about it. 
@@ -1752,8 +1755,12 @@ void FrameLoader::commitProvisionalLoad()
 
         checkCompleted();
     } else {
-        if (cachedPage)
+        if (cachedPage) {
             pageCache()->remove(history().currentItem());
+
+            // Clear out 'cachedPage' right away since it now points to a deleted object.
+            cachedPage = nullptr;
+        }
         didOpenURL();
     }
 
@@ -1789,7 +1796,7 @@ void FrameLoader::commitProvisionalLoad()
     }
 }
 
-void FrameLoader::transitionToCommitted(PassRefPtr<CachedPage> cachedPage)
+void FrameLoader::transitionToCommitted(CachedPage* cachedPage)
 {
     ASSERT(m_client.hasWebView());
     ASSERT(m_state == FrameStateProvisional);
index 803eac1..fab57a3 100644 (file)
@@ -308,7 +308,7 @@ private:
     void addExtraFieldsToRequest(ResourceRequest&, FrameLoadType, bool isMainResource);
 
     void clearProvisionalLoad();
-    void transitionToCommitted(PassRefPtr<CachedPage>);
+    void transitionToCommitted(CachedPage*);
     void frameLoadCompleted();
 
     SubstituteData defaultSubstituteDataForURL(const KURL&);