Bug 88453 - [GTK] [WebKit2] CanHandleRequest API test fails
Summary: [GTK] [WebKit2] CanHandleRequest API test fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anton Obzhirov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 14:04 PDT by Martin Robinson
Modified: 2013-10-31 03:32 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.01 KB, patch)
2013-10-24 09:29 PDT, Anton Obzhirov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2012-06-06 14:04:07 PDT
The CanHandleRequest API test fails with the following output on GTK+:

[ RUN      ] WebKit2.CanHandleRequest
../../Tools/TestWebKitAPI/Tests/WebKit2/CanHandleRequest.cpp:68: Failure
Value of: canHandleRequest
  Actual: false
Expected: true
Comment 1 Anton Obzhirov 2013-10-24 09:24:45 PDT
After investigation:
The issue is WebPage::platformCanHandleRequest(const ResourceRequest&) for GTK port always returns true. This doesn't look right since it should return false by default for all unknown schemes. For example, currently the test checks
canHandleURL("about:blank") && canHandleURL("emptyscheme://") && !canHandleURL("notascheme://");.
First two are handled by WebCore in SchemeRegistry::shouldLoadURLSchemeAsEmptyDocument since "about:blank" is default and "emptyscheme:" was registered. But "notascheme://" is not in the list of known schemes and the call should return false.
Comment 2 Anton Obzhirov 2013-10-24 09:29:05 PDT
Created attachment 215072 [details]
Patch
Comment 3 Carlos Garcia Campos 2013-10-25 00:49:56 PDT
Comment on attachment 215072 [details]
Patch

Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
Comment 4 Carlos Garcia Campos 2013-10-25 00:52:42 PDT
Comment on attachment 215072 [details]
Patch

In WebKit1 we have always returned true.
Comment 5 Anton Obzhirov 2013-10-25 01:47:35 PDT
(In reply to comment #4)
> (From update of attachment 215072 [details])
> In WebKit1 we have always returned true.
Is not it unsafe
(In reply to comment #3)
> (From update of attachment 215072 [details])
> Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
Are not custom schemes supposed to be registered before loading?
Comment 6 Anton Obzhirov 2013-10-25 01:50:10 PDT
(In reply to comment #4)
> (From update of attachment 215072 [details])
> In WebKit1 we have always returned true.
Was it made to support the custom schemes? Please also see my question above.
Comment 7 Carlos Garcia Campos 2013-10-25 04:30:18 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 215072 [details] [details])
> > In WebKit1 we have always returned true.
> Is not it unsafe
> (In reply to comment #3)
> > (From update of attachment 215072 [details] [details])
> > Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
> Are not custom schemes supposed to be registered before loading?

Ok, I was confused, because the policy checker calls canHandleRequest in the frame loader client, but that one in WebKit2 also returns always true.

bool WebFrameLoaderClient::canHandleRequest(const ResourceRequest&) const
{
    notImplemented();
    return true;
}

The one in WebPage is not called by WebCore while loading resources, but only by the injected bundle API, so it shouldn't affect custom uri schemes nor any other thing, except tests using WKBundlePageCanHandleRequest. I wonder why this is handled differently, and also whether we should properly implement it in GTK by checking the schemes registered in the soup session, instead of returning false unconditionally. I think it's not yet possible to register custom uri schemes with the C API in GTK+, so it's probably safe to always return false for now to make the test pass.
Comment 8 WebKit Commit Bot 2013-10-25 04:55:24 PDT
Comment on attachment 215072 [details]
Patch

Clearing flags on attachment: 215072

Committed r158002: <http://trac.webkit.org/changeset/158002>
Comment 9 WebKit Commit Bot 2013-10-25 04:55:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Anton Obzhirov 2013-10-25 05:52:01 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 215072 [details] [details] [details])
> > > In WebKit1 we have always returned true.
> > Is not it unsafe
> > (In reply to comment #3)
> > > (From update of attachment 215072 [details] [details] [details])
> > > Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
> > Are not custom schemes supposed to be registered before loading?
> 
> Ok, I was confused, because the policy checker calls canHandleRequest in the frame loader client, but that one in WebKit2 also returns always true.
> 
> bool WebFrameLoaderClient::canHandleRequest(const ResourceRequest&) const
> {
>     notImplemented();
>     return true;
> }
> 
> The one in WebPage is not called by WebCore while loading resources, but only by the injected bundle API, so it shouldn't affect custom uri schemes nor any other thing, except tests using WKBundlePageCanHandleRequest. I wonder why this is handled differently, and also whether we should properly implement it in GTK by checking the schemes registered in the soup session, instead of returning false unconditionally. I think it's not yet possible to register custom uri schemes with the C API in GTK+, so it's probably safe to always return false for now to make the test pass.

Hm, good point about two version of canHandleRequest APIs. Is WebFrameLoaderClient created for each frame in the document?

Regarding registering custom uri schemes for the page - are we planning to have such API in the future? For this and for checking lib soup session schemes I could create a new bug and start investigating options.
Comment 11 Carlos Garcia Campos 2013-10-31 02:41:32 PDT
(In reply to comment #10)
> (In reply to comment #7)
> > (In reply to comment #5)
> > > (In reply to comment #4)
> > > > (From update of attachment 215072 [details] [details] [details] [details])
> > > > In WebKit1 we have always returned true.
> > > Is not it unsafe
> > > (In reply to comment #3)
> > > > (From update of attachment 215072 [details] [details] [details] [details])
> > > > Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
> > > Are not custom schemes supposed to be registered before loading?
> > 
> > Ok, I was confused, because the policy checker calls canHandleRequest in the frame loader client, but that one in WebKit2 also returns always true.
> > 
> > bool WebFrameLoaderClient::canHandleRequest(const ResourceRequest&) const
> > {
> >     notImplemented();
> >     return true;
> > }
> > 
> > The one in WebPage is not called by WebCore while loading resources, but only by the injected bundle API, so it shouldn't affect custom uri schemes nor any other thing, except tests using WKBundlePageCanHandleRequest. I wonder why this is handled differently, and also whether we should properly implement it in GTK by checking the schemes registered in the soup session, instead of returning false unconditionally. I think it's not yet possible to register custom uri schemes with the C API in GTK+, so it's probably safe to always return false for now to make the test pass.
> 
> Hm, good point about two version of canHandleRequest APIs. Is WebFrameLoaderClient created for each frame in the document?
> 
> Regarding registering custom uri schemes for the page - are we planning to have such API in the future? For this and for checking lib soup session schemes I could create a new bug and start investigating options.

No, we don't plan to add any new C API unless it's required by the tests, since nobody except WTR and unit tests is using the C API of the gtk port.
Comment 12 Anton Obzhirov 2013-10-31 03:13:48 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #7)
> > > (In reply to comment #5)
> > > > (In reply to comment #4)
> > > > > (From update of attachment 215072 [details] [details] [details] [details] [details])
> > > > > In WebKit1 we have always returned true.
> > > > Is not it unsafe
> > > > (In reply to comment #3)
> > > > > (From update of attachment 215072 [details] [details] [details] [details] [details])
> > > > > Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
> > > > Are not custom schemes supposed to be registered before loading?
> > > 
> > > Ok, I was confused, because the policy checker calls canHandleRequest in the frame loader client, but that one in WebKit2 also returns always true.
> > > 
> > > bool WebFrameLoaderClient::canHandleRequest(const ResourceRequest&) const
> > > {
> > >     notImplemented();
> > >     return true;
> > > }
> > > 
> > > The one in WebPage is not called by WebCore while loading resources, but only by the injected bundle API, so it shouldn't affect custom uri schemes nor any other thing, except tests using WKBundlePageCanHandleRequest. I wonder why this is handled differently, and also whether we should properly implement it in GTK by checking the schemes registered in the soup session, instead of returning false unconditionally. I think it's not yet possible to register custom uri schemes with the C API in GTK+, so it's probably safe to always return false for now to make the test pass.
> > 
> > Hm, good point about two version of canHandleRequest APIs. Is WebFrameLoaderClient created for each frame in the document?
> > 
> > Regarding registering custom uri schemes for the page - are we planning to have such API in the future? For this and for checking lib soup session schemes I could create a new bug and start investigating options.
> 
> No, we don't plan to add any new C API unless it's required by the tests, since nobody except WTR and unit tests is using the C API of the gtk port.
Should we check lib soup session schemes?
Comment 13 Carlos Garcia Campos 2013-10-31 03:32:54 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #7)
> > > > (In reply to comment #5)
> > > > > (In reply to comment #4)
> > > > > > (From update of attachment 215072 [details] [details] [details] [details] [details] [details])
> > > > > > In WebKit1 we have always returned true.
> > > > > Is not it unsafe
> > > > > (In reply to comment #3)
> > > > > > (From update of attachment 215072 [details] [details] [details] [details] [details] [details])
> > > > > > Are you sure this doesn't break loading custom uri schemes? Please, check it before landing.
> > > > > Are not custom schemes supposed to be registered before loading?
> > > > 
> > > > Ok, I was confused, because the policy checker calls canHandleRequest in the frame loader client, but that one in WebKit2 also returns always true.
> > > > 
> > > > bool WebFrameLoaderClient::canHandleRequest(const ResourceRequest&) const
> > > > {
> > > >     notImplemented();
> > > >     return true;
> > > > }
> > > > 
> > > > The one in WebPage is not called by WebCore while loading resources, but only by the injected bundle API, so it shouldn't affect custom uri schemes nor any other thing, except tests using WKBundlePageCanHandleRequest. I wonder why this is handled differently, and also whether we should properly implement it in GTK by checking the schemes registered in the soup session, instead of returning false unconditionally. I think it's not yet possible to register custom uri schemes with the C API in GTK+, so it's probably safe to always return false for now to make the test pass.
> > > 
> > > Hm, good point about two version of canHandleRequest APIs. Is WebFrameLoaderClient created for each frame in the document?
> > > 
> > > Regarding registering custom uri schemes for the page - are we planning to have such API in the future? For this and for checking lib soup session schemes I could create a new bug and start investigating options.
> > 
> > No, we don't plan to add any new C API unless it's required by the tests, since nobody except WTR and unit tests is using the C API of the gtk port.
> Should we check lib soup session schemes?

Well, CanHandleRequest is only used by this test, and there's no other custom scheme registered in the test.