Bug 20716 - FrameLoader should delegate NewWindow browser policy decision
Summary: FrameLoader should delegate NewWindow browser policy decision
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Major
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-08 03:52 PDT by Anton Tarasov
Modified: 2008-09-08 04:37 PDT (History)
0 users

See Also:


Attachments
patch (2.93 KB, patch)
2008-09-08 04:08 PDT, Anton Tarasov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anton Tarasov 2008-09-08 03:52:11 PDT
FrameLoader::continueAfterNewWindowPolicy method, unlike
FrameLoader::continueAfterNavigationPolicy method, doesn't delegate
policy decision to the client.

The latter does it as follows:

-----
        case PolicyUse: {
            ResourceRequest request(check.request());
            
            if (!m_client->canHandleRequest(request)) {                               
                handleUnimplementablePolicy(
                      m_client->cannotShowURLError(check.request()));
                check.clearRequest();
                shouldContinue = false;
            }
            break;
        }

        check.call(shouldContinue);
-----

I guess some similar approach should be implemented in 
continueAfterNewWindowPolicy.

Respectively, FrameLoaderClient.h should provide a method like CanOpenNewWindow.
Comment 1 Anton Tarasov 2008-09-08 04:08:02 PDT
Created attachment 23246 [details]
patch

Proposes new methods in FrameLoaderClient:

    virtual bool canOpenNewWindow(const ResourceRequest&) const = 0;
    virtual ResourceError cannotOpenNewWindowError(const ResourceRequest&) = 0;

The methods are called in:

void FrameLoader::continueAfterNewWindowPolicy(PolicyAction policy)
Comment 2 Anton Tarasov 2008-09-08 04:37:59 PDT
Based on the post below, the issue reported is not a bug.

On Sep 3, 2008, at 4:56 AM, Anton V. Tarasov wrote:

> The method FrameLoader::continueAfterNavigationPolicy (the first stack) calls
> m_client->canHandleRequest(request) in its turn in order to request an
> approval from the client.

Yes, there is a client function named canHandleRequest, but that call is not requesting the navigation policy. That's a separate client function and it's not about navigation policy. It's only called if the navigation policy named PolicyUse is specified.

The navigation policy comes from the value passed by the client's m_client->dispatchDecidePolicyForNavigationAction function when it calls the FramePolicyFunction. The code that respects the policy is the switch statement in the continueAfterNavigationPolicy function.

> However the method FrameLoader::continueAfterNewWindowPolicy (the second
> stack) does nothing to get an approval. The class FrameLoaderClient misses a
> method like "canOpenNewWindow" at all.

As in the case of navigation policy, the new window policy comes from the value the client passes back when it calls the FramePolicyFunction passed to m_client->dispatchDecidePolicyForNewWindowAction, which is respected by the switch statement in the continueAfterNewWindowPolicy function.

You may have a legitimate question here or maybe even a bug, but it's incorrect to call the canHandleRequest function the policy check, so I think you need to at least re-word your question to clarify what you're asking.

The client function that performs the new window policy check is dispatchDecidePolicyForNewWindowAction.

    -- Darin