ProgressTracker never completes if iframe detached during parsing
authorjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2012 02:58:37 +0000 (02:58 +0000)
committerjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Aug 2012 02:58:37 +0000 (02:58 +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:
(FrameLoader::FrameProgressTracker):
(WebCore::FrameLoader::FrameProgressTracker::create):
(WebCore::FrameLoader::FrameProgressTracker::~FrameProgressTracker):
(WebCore::FrameLoader::FrameProgressTracker::progressStarted):
(WebCore::FrameLoader::FrameProgressTracker::progressCompleted):
(WebCore::FrameLoader::FrameProgressTracker::FrameProgressTracker):
(WebCore):
(WebCore::FrameLoader::init):
(WebCore::FrameLoader::prepareForLoadStart):
(WebCore::FrameLoader::clearProvisionalLoad):
(WebCore::FrameLoader::checkLoadCompleteForThisFrame):
(WebCore::FrameLoader::detachFromParent):
* loader/FrameLoader.h:
(FrameLoader):

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

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

index 9f8b0f1..eedd909 100644 (file)
@@ -1,3 +1,31 @@
+2012-08-29  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:
+        (FrameLoader::FrameProgressTracker):
+        (WebCore::FrameLoader::FrameProgressTracker::create):
+        (WebCore::FrameLoader::FrameProgressTracker::~FrameProgressTracker):
+        (WebCore::FrameLoader::FrameProgressTracker::progressStarted):
+        (WebCore::FrameLoader::FrameProgressTracker::progressCompleted):
+        (WebCore::FrameLoader::FrameProgressTracker::FrameProgressTracker):
+        (WebCore):
+        (WebCore::FrameLoader::init):
+        (WebCore::FrameLoader::prepareForLoadStart):
+        (WebCore::FrameLoader::clearProvisionalLoad):
+        (WebCore::FrameLoader::checkLoadCompleteForThisFrame):
+        (WebCore::FrameLoader::detachFromParent):
+        * loader/FrameLoader.h:
+        (FrameLoader):
+
 2012-08-29  Yoshifumi Inoue  <yosin@chromium.org>
 
         [Forms] Rename DateTimeFieldElement::FieldEventHandler to FieldOwner
index ca9ff09..c242bf7 100644 (file)
@@ -165,6 +165,43 @@ 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_inProgress || m_frame->page());
+        if (m_inProgress)
+            m_frame->page()->progress()->progressCompleted(m_frame);
+    }
+
+    void progressStarted()
+    {
+        ASSERT(m_frame->page());
+        if (!m_inProgress)
+            m_frame->page()->progress()->progressStarted(m_frame);
+        m_inProgress = true;
+    }
+
+    void progressCompleted()
+    {
+        ASSERT(m_inProgress);
+        ASSERT(m_frame->page());
+        m_inProgress = false;
+        m_frame->page()->progress()->progressCompleted(m_frame);
+    }
+
+private:
+    FrameProgressTracker(Frame* frame)
+        : m_frame(frame)
+        , m_inProgress(false)
+    {
+    }
+
+    Frame* m_frame;
+    bool m_inProgress;
+};
+
 FrameLoader::FrameLoader(Frame* frame, FrameLoaderClient* client)
     : m_frame(frame)
     , m_client(client)
@@ -228,6 +265,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 +1125,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 +1683,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 +2159,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 +2178,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 +2403,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;