ProgressTracker never completes if iframe detached during parsing
authorjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 21:26:07 +0000 (21:26 +0000)
committerjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 21:26:07 +0000 (21:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=92272

Reviewed by Adam Barth.

Add a simple helper class to FrameLoader to ensure progressStarted/progressCompleted calls are matched,
and balance the calls when the Frame is detached.

No new tests, as this behavior has only been producing reliably by setting a breakpoint in a specific place.

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::init):
(WebCore::FrameLoader::FrameProgressTracker::progressStarted):
(WebCore::FrameLoader::FrameProgressTracker::progressCompleted):
(WebCore::FrameLoader::FrameProgressTracker::~FrameProgressTracker):
(WebCore::FrameLoader::prepareForLoadStart):
(WebCore::FrameLoader::clearProvisionalLoad):
(WebCore::FrameLoader::checkLoadCompleteForThisFrame):
(WebCore::FrameLoader::detachFromParent):
* loader/FrameLoader.h:
(FrameProgressTracker):
(WebCore::FrameLoader::FrameProgressTracker::create):
(WebCore::FrameLoader::FrameProgressTracker::FrameProgressTracker):

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

Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h

index 4ce9716..5c4ea45 100644 (file)
@@ -1,3 +1,29 @@
+2012-08-23  Nate Chapin  <japhet@chromium.org>
+
+        ProgressTracker never completes if iframe detached during parsing 
+        https://bugs.webkit.org/show_bug.cgi?id=92272
+
+        Reviewed by Adam Barth.
+
+        Add a simple helper class to FrameLoader to ensure progressStarted/progressCompleted calls are matched,
+        and balance the calls when the Frame is detached.
+
+        No new tests, as this behavior has only been producing reliably by setting a breakpoint in a specific place.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::init):
+        (WebCore::FrameLoader::FrameProgressTracker::progressStarted):
+        (WebCore::FrameLoader::FrameProgressTracker::progressCompleted):
+        (WebCore::FrameLoader::FrameProgressTracker::~FrameProgressTracker):
+        (WebCore::FrameLoader::prepareForLoadStart):
+        (WebCore::FrameLoader::clearProvisionalLoad):
+        (WebCore::FrameLoader::checkLoadCompleteForThisFrame):
+        (WebCore::FrameLoader::detachFromParent):
+        * loader/FrameLoader.h:
+        (FrameProgressTracker):
+        (WebCore::FrameLoader::FrameProgressTracker::create):
+        (WebCore::FrameLoader::FrameProgressTracker::FrameProgressTracker):
+
 2012-08-23  Dana Jansens  <danakj@chromium.org>
 
         [chromium] Create sharedQuadState at same time as creating quads and give them to the quadSink
index ca9ff09..384daa9 100644 (file)
@@ -165,6 +165,42 @@ static bool isDocumentSandboxed(Frame* frame, SandboxFlags mask)
     return frame->document() && frame->document()->isSandboxed(mask);
 }
 
+class FrameLoader::FrameProgressTracker {
+public:
+    static PassOwnPtr<FrameProgressTracker> create(Frame* frame) { return adoptPtr(new FrameProgressTracker(frame)); }
+    ~FrameProgressTracker()
+    {
+        ASSERT(!m_inProgressCount || m_frame->page());
+        for (; m_inProgressCount; m_inProgressCount--)
+            m_frame->page()->progress()->progressCompleted(m_frame);
+    }
+
+    void progressStarted()
+    {
+        ASSERT(m_frame->page());
+        m_inProgressCount++;
+        m_frame->page()->progress()->progressStarted(m_frame);
+    }
+
+    void progressCompleted()
+    {
+        ASSERT(m_inProgressCount > 0);
+        ASSERT(m_frame->page());
+        m_inProgressCount--;
+        m_frame->page()->progress()->progressCompleted(m_frame);
+    }
+
+private:
+    FrameProgressTracker(Frame* frame)
+        : m_frame(frame)
+        , m_inProgressCount(0)
+    {
+    }
+
+    Frame* m_frame;
+    int m_inProgressCount;
+};
+
 FrameLoader::FrameLoader(Frame* frame, FrameLoaderClient* client)
     : m_frame(frame)
     , m_client(client)
@@ -228,6 +264,7 @@ void FrameLoader::init()
     m_didCallImplicitClose = true;
 
     m_networkingContext = m_client->createNetworkingContext();
+    m_progressTracker = FrameProgressTracker::create(m_frame);
 }
 
 void FrameLoader::setDefersLoading(bool defers)
@@ -1087,8 +1124,7 @@ void FrameLoader::prepareForHistoryNavigation()
 
 void FrameLoader::prepareForLoadStart()
 {
-    if (Page* page = m_frame->page())
-        page->progress()->progressStarted(m_frame);
+    m_progressTracker->progressStarted();
     m_client->dispatchDidStartProvisionalLoad();
 
     // Notify accessibility.
@@ -1646,8 +1682,7 @@ void FrameLoader::setState(FrameState newState)
 void FrameLoader::clearProvisionalLoad()
 {
     setProvisionalDocumentLoader(0);
-    if (Page* page = m_frame->page())
-        page->progress()->progressCompleted(m_frame);
+    m_progressTracker->progressCompleted();
     setState(FrameStateComplete);
 }
 
@@ -2123,9 +2158,8 @@ void FrameLoader::checkLoadCompleteForThisFrame()
                 return;
 
             if (!settings->needsDidFinishLoadOrderQuirk()) {
+                m_progressTracker->progressCompleted();
                 if (Page* page = m_frame->page()) {
-                    page->progress()->progressCompleted(m_frame);
-
                     if (m_frame == page->mainFrame())
                         page->resetRelevantPaintedObjectCounter();
                 }
@@ -2143,9 +2177,8 @@ void FrameLoader::checkLoadCompleteForThisFrame()
             }
 
             if (settings->needsDidFinishLoadOrderQuirk()) {
+                m_progressTracker->progressCompleted();
                 if (Page* page = m_frame->page()) {
-                    page->progress()->progressCompleted(m_frame);
-
                     if (m_frame == page->mainFrame())
                         page->resetRelevantPaintedObjectCounter();
                 }
@@ -2369,6 +2402,8 @@ void FrameLoader::detachFromParent()
 
     detachViewsAndDocumentLoader();
 
+    m_progressTracker.clear();
+
     if (Frame* parent = m_frame->tree()->parent()) {
         parent->loader()->closeAndRemoveChild(m_frame);
         parent->loader()->scheduleCheckCompleted();
index 4a17ab2..a21926f 100644 (file)
@@ -385,6 +385,9 @@ private:
     mutable FrameLoaderStateMachine m_stateMachine;
     mutable IconController m_icon;
 
+    class FrameProgressTracker;
+    OwnPtr<FrameProgressTracker> m_progressTracker;
+
     FrameState m_state;
     FrameLoadType m_loadType;