[Webkit-unassigned] [Bug 29910] Load state can get out of sync in a client callback, leading to a crash.

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Yael <yael.aharon at nokia.com>  2009-09-30 09:31:03 PDT ---

Thank you for your review. 

> 3 - We *NEED* to start enforcing layouttests for loader changes like these.

I agree with you that some additional testing should be added. If you could
help me understand how to create a test for this complex use case, and I would
be happy to do that.


> Elaborations:
> 1 - Before we jump the gun on this change, I'd like some more details of what
> the problem actually is.  Your description of the bug sounds theoretical -
> could you post a crash log and steps to reproduce?

This crash happened in S60 environment and I could not attach crash log, but I
did attach a callstack leading to the crash. Some explanation of the use case
might help:

This crash was observed when I opened the Browser and the first url I tried to
load was a non-existing html file from the file system. Inside the progress
callback of the first load, the browser sets a new user stylesheet, which leads
to a new load request.

The actual crash happens when we select an active document loader to load the
user stylesheet.

DocumentLoader* FrameLoader::activeDocumentLoader() const
{
    if (m_state == FrameStateProvisional) 
        return m_provisionalDocumentLoader.get();
    return m_documentLoader.get();
}

The state is still FrameStateProvisional, but m_provisionalDocumentLoader was
already set to NULL. This returns a NULL DocumentLoader, leading to a crash.


> 2 - This pattern of setting FrameStateComplete after the progressCompleted()
> callback occurs more than once, and appears to have been a deliberate decision.
>  While I have no direct evidence it's the wrong change, my Spidey Sense is
> tingling.

Could you refer me to where is this pattern used? In FrameLoader.cpp,
ProgressComplete() is called from 2 places. The second place is 
checkLoadCompleteForThisFrame(), and it is called after markLoadComplete() is
called.

> 3 - If we clear up #1 and #2, I'm sure I can help find a way to layouttest
> this.

I appreciate your help here.

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