[Webkit-unassigned] [Bug 54380] [Qt]tst_QWebPage::modified() asserts

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54380


Aparna Nandyal <aparna.nand at wipro.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83098|                            |review?, commit-queue?
               Flag|                            |




--- Comment #2 from Aparna Nandyal <aparna.nand at wipro.com>  2011-02-20 10:50:18 PST ---
Created an attachment (id=83098)
 --> (https://bugs.webkit.org/attachment.cgi?id=83098&action=review)
Patch for review

Reason for assert:

1. When FrameView's destructor is called, it asserts when m_enqueueEvents is not 0.
2. This variable is incremented in FrameView::pauseScheduledEvents and decremented in FrameView::resumeScheduledEvents. 
3. In normal circumstances very time QWebFrame::setUrl is called, pauseScheduledEvents is called 3 times (once in d->frame->loader()->activeDocumentLoader()->writer()->begin(absolute) and twice in d->frame->loader()->activeDocumentLoader()->writer()->end())
3. But if back/forward navigation has been done before current QWebFrame::setUrl (as in the case of test case), FrameView::pauseScheduledEvents is called 5 times and FrameView::resumeScheduledEvents is called 4 times, leaving the m_enqueueEvents=1 which is causing the assert.
4. Extra calls to FrameView::pauseScheduledEvents is made due to the wrong code path taken in 
FrameLoader::didFirstLayout which is called from d->frame->loader()->activeDocumentLoader()->writer()->end()

void FrameLoader::didFirstLayout()
{
    if (m_frame->page() && isBackForwardLoadType(m_loadType))
        history()->restoreScrollPositionAndViewState();

    .......
}
The function isBackForwardLoadType() returns true. But it is the previous operation's m_loadType that is used and the new m_loadType is set only in FrameLoader::load (which apparently has a "// FIXME: is this the right place to reset loadType? Perhaps this should be done after loading is finished or aborted."). But in QWebFrame::setUrl, the damage has been done before the call to FrameLoader::load as there is a wrong m_loadType.

Ways to fix this:
1. Make sure that m_loadType is set before DocumentWriter's begin and end is called. Infact as the FIXME suggests, the value should be reset after loading is completed or aborted. But any changes to those functions seemed risky as they are called in different scenarios and there wasnt a single ending function for the operation (several paths were there). Hence put the fix is relatively safer haven QWebFrame::setUrl. 
2. To fix the test case, a wait for loadFinished signal will do the magic. This would actually reset the m_enqueueEvents to 0 immaterial of its current value in FrameView::~FrameView. This fix would Camouflage the root cause. So not a good idea!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.