[Webkit-unassigned] [Bug 18288] New: FrameLoader wrongly assumes dispatchDecidePolicyForNavigationAction is synchronous

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=18288

           Summary: FrameLoader wrongly assumes
                    dispatchDecidePolicyForNavigationAction is synchronous
           Product: WebKit
           Version: 526+ (Nightly build)
          Platform: Macintosh
        OS/Version: Mac OS X 10.5
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: New Bugs
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: eric at webkit.org


FrameLoader wrongly assumes dispatchDecidePolicyForNavigationAction is
synchronous

I was browsing around in our frame loader today and saw this:

    m_delegateIsDecidingNavigationPolicy = true;
   
m_client->dispatchDecidePolicyForNavigationAction(&FrameLoader::continueAfterNavigationPolicy,
        action, request);
    m_delegateIsDecidingNavigationPolicy = false;

That looks wrong.  Looking at dispatchDecidePolicyForNavigationAction, which
calls: (up in webkit)

    [[webView _policyDelegateForwarder] webView:webView
                decidePolicyForNavigationAction:actionDictionary(action)
                                        request:request.nsURLRequest()
                                          frame:m_webFrame.get()
                              
decisionListener:setUpPolicyListener(function).get()];

Assumes that the policy delegate makes a synchronous call to the policy
listener from within this decidePolicyForNavigationAction: call.

This seems like a bogus assumption.  At least if your policy delegate needs to
do any work to decide if the navigation should happen.

It looks like the only consequence of this error is that:
bool FrameLoader::shouldReloadToHandleUnreachableURL(DocumentLoader* docLoader)

may return the wrong value, causing:

FrameLoader::load() to screw up the b/f list:

    // When we loading alternate content for an unreachable URL that we're
    // visiting in the b/f list, we treat it as a reload so the b/f list 
    // is appropriately maintained.
    if (shouldReloadToHandleUnreachableURL(newDocumentLoader)) {
        ASSERT(type == FrameLoadTypeStandard);
        type = FrameLoadTypeReload;
    }

Again, I don't have any test case which breaks this.  This is not a bug in
Safari.  This is only ever a bug if some WebKit client were to implement
dispatchDecidePolicyForNavigationAction delegate method asynchronous.  But the
code looked wrong (and further inspection seemed to confirm the wrongness), so
I'm filing this bug.


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