REGRESSION (r218015) IconLoaders for already-cached resources expect to be asynchrono...
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Jun 2017 20:34:47 +0000 (20:34 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Jun 2017 20:34:47 +0000 (20:34 +0000)
<rdar://problem/32817519> and https://bugs.webkit.org/show_bug.cgi?id=173478

Reviewed by Daniel Bates.

Source/WebCore:

Covered by API test.

Being synchronous is actually better as it's resolved another issue or two.
But only if we can actually deliver the data without crashing first.
So let's do that.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::didGetLoadDecisionForIcon): Put the IconLoader in the set of active icon loaders
  before actually starting the icon loading.

Tools:

* TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@218409 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm

index fc5faf5..7b93630 100644 (file)
@@ -1,3 +1,20 @@
+2017-06-16  Brady Eidson  <beidson@apple.com>
+
+        REGRESSION (r218015) IconLoaders for already-cached resources expect to be asynchronous, no longer are.
+        <rdar://problem/32817519> and https://bugs.webkit.org/show_bug.cgi?id=173478
+
+        Reviewed by Daniel Bates.
+
+        Covered by API test.
+
+        Being synchronous is actually better as it's resolved another issue or two.
+        But only if we can actually deliver the data without crashing first.
+        So let's do that.
+        
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::didGetLoadDecisionForIcon): Put the IconLoader in the set of active icon loaders
+          before actually starting the icon loading.
+
 2017-06-16  Jeremy Jones  <jeremyj@apple.com>
 
         Don't use WebCore Timer from code that runs in the UI process.
index 92d8eea..9babcef 100644 (file)
@@ -1681,8 +1681,10 @@ void DocumentLoader::didGetLoadDecisionForIcon(bool decision, uint64_t loadIdent
         return;
 
     auto iconLoader = std::make_unique<IconLoader>(*this, icon.url);
-    iconLoader->startLoading();
+    auto* rawIconLoader = iconLoader.get();
     m_iconLoaders.set(WTFMove(iconLoader), newCallbackID);
+
+    rawIconLoader->startLoading();
 }
 
 void DocumentLoader::finishedLoadingIcon(IconLoader& loader, SharedBuffer* buffer)
index 7f10124..d60fb2f 100644 (file)
@@ -1,3 +1,12 @@
+2017-06-16  Brady Eidson  <beidson@apple.com>
+
+        REGRESSION (r218015) IconLoaders for already-cached resources expect to be asynchronous, no longer are.
+        <rdar://problem/32817519> and https://bugs.webkit.org/show_bug.cgi?id=173478
+
+        Reviewed by Daniel Bates.
+
+        * TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm:
+
 2017-06-16  Chris Dumez  <cdumez@apple.com>
 
         DRT fails to reset page visibility between tests
index 7f496ea..8bfade1 100644 (file)
@@ -39,6 +39,9 @@
 #if WK_API_ENABLED
 
 static bool doneWithIcons;
+static bool receivedFaviconDataCallback;
+static bool alreadyProvidedIconData;
+static RetainPtr<NSData> receivedFaviconData;
 
 @interface IconLoadingDelegate : NSObject <_WKIconLoadingDelegate>
 @end
@@ -66,37 +69,60 @@ static bool doneWithIcons;
     if (favicon && touch && touchPrecomposed)
         doneWithIcons = true;
 
-    completionHandler(nil);
+    if (parameters.iconType == WKLinkIconTypeFavicon) {
+        completionHandler([](NSData *iconData) {
+            receivedFaviconData = iconData;
+            receivedFaviconDataCallback = true;
+        });
+    } else
+        completionHandler(nil);
 }
 
 @end
 
 @interface IconLoadingSchemeHandler : NSObject <WKURLSchemeHandler>
-- (instancetype)initWithData:(NSData *)data mimeType:(NSString *)inMIMEType;
+- (instancetype)initWithData:(NSData *)data;
+- (void)setFaviconData:(NSData *)data;
 @end
 
 @implementation IconLoadingSchemeHandler {
-    RetainPtr<NSData> resourceData;
-    RetainPtr<NSString> mimeType;
+    RetainPtr<NSData> mainResourceData;
+    RetainPtr<NSData> faviconData;
 }
 
-- (instancetype)initWithData:(NSData *)data mimeType:(NSString *)inMIMEType
+- (instancetype)initWithData:(NSData *)data
 {
     self = [super init];
     if (!self)
         return nil;
 
-    resourceData = data;
-    mimeType = inMIMEType;
+    mainResourceData = data;
 
     return self;
 }
 
+- (void)setFaviconData:(NSData *)data
+{
+    faviconData = data;
+}
+
 - (void)webView:(WKWebView *)webView startURLSchemeTask:(id <WKURLSchemeTask>)task
 {
-    RetainPtr<NSURLResponse> response = adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:mimeType.get() expectedContentLength:1 textEncodingName:nil]);
+    RetainPtr<NSURLResponse> response;
+    NSData *data = nil;
+
+    if ([[task.request.URL absoluteString] isEqual:@"testing:///favicon.ico"]) {
+        EXPECT_FALSE(alreadyProvidedIconData);
+        response = adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"image/png" expectedContentLength:1 textEncodingName:nil]);
+        data = faviconData.get();
+        alreadyProvidedIconData = true;
+    } else {
+        response = adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"text/html" expectedContentLength:1 textEncodingName:nil]);
+        data = mainResourceData.get();
+    }
+
     [task didReceiveResponse:response.get()];
-    [task didReceiveData:resourceData.get()];
+    [task didReceiveData:data];
     [task didFinish];
 }
 
@@ -116,7 +142,7 @@ TEST(IconLoading, DefaultFavicon)
 {
     RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
 
-    RetainPtr<IconLoadingSchemeHandler> handler = adoptNS([[IconLoadingSchemeHandler alloc] initWithData:[NSData dataWithBytesNoCopy:(void*)mainBytes length:sizeof(mainBytes)] mimeType:@"text/html"]);
+    RetainPtr<IconLoadingSchemeHandler> handler = adoptNS([[IconLoadingSchemeHandler alloc] initWithData:[NSData dataWithBytesNoCopy:(void*)mainBytes length:sizeof(mainBytes)]]);
     [configuration setURLSchemeHandler:handler.get() forURLScheme:@"testing"];
 
     RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
@@ -130,4 +156,44 @@ TEST(IconLoading, DefaultFavicon)
     TestWebKitAPI::Util::run(&doneWithIcons);
 }
 
+static const char mainBytes2[] =
+"Oh, hello there!";
+
+TEST(IconLoading, AlreadyCachedIcon)
+{
+    RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+    NSData *mainData = [NSData dataWithBytesNoCopy:(void*)mainBytes2 length:sizeof(mainBytes2)];
+    RetainPtr<IconLoadingSchemeHandler> handler = adoptNS([[IconLoadingSchemeHandler alloc] initWithData:mainData]);
+
+    NSURL *url = [[NSBundle mainBundle] URLForResource:@"large-red-square-image" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    RetainPtr<NSData *> iconDataFromDisk = [NSData dataWithContentsOfURL:url];
+    [handler.get() setFaviconData:iconDataFromDisk.get()];
+
+    [configuration setURLSchemeHandler:handler.get() forURLScheme:@"testing"];
+
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+    RetainPtr<IconLoadingDelegate> iconDelegate = adoptNS([[IconLoadingDelegate alloc] init]);
+
+    webView.get()._iconLoadingDelegate = iconDelegate.get();
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"testing:///main"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&receivedFaviconDataCallback);
+
+    EXPECT_TRUE([iconDataFromDisk.get() isEqual:receivedFaviconData.get()]);
+
+    receivedFaviconDataCallback = false;
+    receivedFaviconData = nil;
+
+    // Load another main resource that results in the same icon being loaded (which should come from the memory cache).
+    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"testing:///main2"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&receivedFaviconDataCallback);
+
+    EXPECT_TRUE([iconDataFromDisk.get() isEqual:receivedFaviconData.get()]);
+}
+
 #endif