Bug 126129

Summary: Implement NetworkResourceLoader::receivedCancellation
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, beidson, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108832    
Attachments:
Description Flags
Patch
none
Updated patch ap: review+

Carlos Garcia Campos
Reported 2013-12-22 03:05:16 PST
$ Programs/WebKit2APITests/TestWebKitWebView -p /webkit2/WebKitWebView/authentication-cancel /webkit2/WebKitWebView/authentication-cancel: OK $ WEBKIT_USE_NETWORK_PROCESS=1 Programs/WebKit2APITests/TestWebKitWebView -p /webkit2/WebKitWebView/authentication-cancel /webkit2/WebKitWebView/authentication-cancel: ** ERROR:../../Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:1331:void testWebViewAuthenticationCancel(AuthenticationTest*, gconstpointer): assertion failed (test->m_loadEvents[1] == LoadTrackingTest::ProvisionalLoadFailed): (3 == 2)
Attachments
Patch (3.71 KB, patch)
2013-12-30 02:27 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Updated patch (3.71 KB, patch)
2013-12-31 00:39 PST, Carlos Garcia Campos 		ap: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2013-12-30 02:23:53 PST
The problem is that NetworkResourceLoader doesn't implement ResourceHandleClient::receivedCancellation() and the load finished normally instead of failing with cancel error like the web process does.
Carlos Garcia Campos
Comment 2 2013-12-30 02:27:07 PST
Created attachment 220104 [details] Patch $ WEBKIT_USE_NETWORK_PROCESS=1 Programs/WebKit2APITests/TestAuthentication /webkit2/WebKitWebView/authentication-request: OK /webkit2/WebKitWebView/authentication-cancel: OK /webkit2/WebKitWebView/authentication-load-cancelled: OK /webkit2/WebKitWebView/authentication-failure: OK /webkit2/WebKitWebView/authentication-no-credential: OK /webkit2/WebKitWebView/authentication-storage: OK /webkit2/WebKitWebView/authentication-success: OK
Carlos Garcia Campos
Comment 3 2013-12-31 00:39:11 PST
Created attachment 220153 [details] Updated patch Just rebased to apply on current git master
Brian Holt
Comment 4 2014-01-08 07:01:53 PST
Comment on attachment 220153 [details] Updated patch Informal review: looks good to me, hope to see it landed soon.
Alexey Proskuryakov
Comment 5 2014-01-09 10:15:21 PST
Comment on attachment 220153 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=220153&action=review > Source/WebKit2/ChangeLog:3 > + [GTK] Test /webkit2/WebKitWebView/authentication-cancel fails with network process enabled This patch is cross-platform, please remove "[GTK]" here and from the bug title. > Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:174 > +void NetworkResourceLoader::cancel() I'm not a fan of this function. It looks like someone tells NetworkResourceLoader to cancel, but this is not what it is - it implements what needs to be done in two places where NetworkResourceLoader decides to cancel the load on its own, based on indirect hints. In other future places where NetworkResourceLoader decides to cancel, it may need to do something else. I would have just put the code inline, not factoring it out into a cancel() function. But at the very least, the function should be private, not public.
Carlos Garcia Campos
Comment 6 2014-01-10 01:07:08 PST
Committed r161625: <http://trac.webkit.org/changeset/161625>
Note You need to log in before you can comment on or make changes to this bug.