- Revision
- 218896
- Author
- beid...@apple.com
- Date
- 2017-06-28 15:40:10 -0700 (Wed, 28 Jun 2017)
Log Message
DocumentLoader should always notify the client if there are pending icon loads when the load is stopped.
https://bugs.webkit.org/show_bug.cgi?id=173874
Reviewed by Alex Christensen.
Source/WebCore:
Covered by API tests.
Patch started by Carlos Garcia Campos, finished by me.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::stopLoading): Make all of the callbacks for cancelled IconLoaders.
(WebCore::DocumentLoader::didGetLoadDecisionForIcon): Make the callback even if there's no IconLoader.
(WebCore::DocumentLoader::finishedLoadingIcon):
(WebCore::DocumentLoader::notifyFinishedLoadingIcon):
* loader/DocumentLoader.h:
Tools:
* TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218895 => 218896)
--- trunk/Source/WebCore/ChangeLog 2017-06-28 22:27:28 UTC (rev 218895)
+++ trunk/Source/WebCore/ChangeLog 2017-06-28 22:40:10 UTC (rev 218896)
@@ -1,3 +1,21 @@
+2017-06-28 Brady Eidson <beid...@apple.com>
+
+ DocumentLoader should always notify the client if there are pending icon loads when the load is stopped.
+ https://bugs.webkit.org/show_bug.cgi?id=173874
+
+ Reviewed by Alex Christensen.
+
+ Covered by API tests.
+
+ Patch started by Carlos Garcia Campos, finished by me.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::stopLoading): Make all of the callbacks for cancelled IconLoaders.
+ (WebCore::DocumentLoader::didGetLoadDecisionForIcon): Make the callback even if there's no IconLoader.
+ (WebCore::DocumentLoader::finishedLoadingIcon):
+ (WebCore::DocumentLoader::notifyFinishedLoadingIcon):
+ * loader/DocumentLoader.h:
+
2017-06-28 Antoine Quint <grao...@apple.com>
Volume controls should be hidden when AirPlay is active
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (218895 => 218896)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2017-06-28 22:27:28 UTC (rev 218895)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2017-06-28 22:40:10 UTC (rev 218896)
@@ -282,8 +282,10 @@
m_frame->loader().stopLoading(UnloadEventPolicyNone);
}
+ for (auto callbackIdentifier : m_iconLoaders.values())
+ notifyFinishedLoadingIcon(callbackIdentifier, nullptr);
+ m_iconLoaders.clear();
m_iconsPendingLoadDecision.clear();
- m_iconLoaders.clear();
// Always cancel multipart loaders
cancelAll(m_multipartSubresourceLoaders);
@@ -1680,9 +1682,19 @@
void DocumentLoader::didGetLoadDecisionForIcon(bool decision, uint64_t loadIdentifier, uint64_t newCallbackID)
{
auto icon = m_iconsPendingLoadDecision.take(loadIdentifier);
- if (!decision || icon.url.isEmpty() || !m_frame)
+
+ // If the LinkIcon we just took is empty, then the DocumentLoader had all of its loaders stopped
+ // while this icon load decision was pending.
+ // In this case we need to notify the client that the icon finished loading with empty data.
+ if (icon.url.isEmpty()) {
+ notifyFinishedLoadingIcon(newCallbackID, nullptr);
return;
+ }
+ // If the decision was not to load or this DocumentLoader is already detached, there is no load to perform.
+ if (!decision || !m_frame)
+ return;
+
auto iconLoader = std::make_unique<IconLoader>(*this, icon.url);
auto* rawIconLoader = iconLoader.get();
m_iconLoaders.set(WTFMove(iconLoader), newCallbackID);
@@ -1698,9 +1710,15 @@
auto callbackIdentifier = m_iconLoaders.take(&loader);
RELEASE_ASSERT(callbackIdentifier);
- m_frame->loader().client().finishedLoadingIcon(callbackIdentifier, buffer);
+ notifyFinishedLoadingIcon(callbackIdentifier, buffer);
}
+void DocumentLoader::notifyFinishedLoadingIcon(uint64_t callbackIdentifier, SharedBuffer* buffer)
+{
+ if (m_frame)
+ m_frame->loader().client().finishedLoadingIcon(callbackIdentifier, buffer);
+}
+
void DocumentLoader::dispatchOnloadEvents()
{
m_wasOnloadDispatched = true;
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (218895 => 218896)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2017-06-28 22:27:28 UTC (rev 218895)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2017-06-28 22:40:10 UTC (rev 218896)
@@ -368,6 +368,8 @@
void cancelPolicyCheckIfNeeded();
void becomeMainResourceClient();
+ void notifyFinishedLoadingIcon(uint64_t callbackIdentifier, SharedBuffer*);
+
Frame* m_frame { nullptr };
Ref<CachedResourceLoader> m_cachedResourceLoader;
Modified: trunk/Tools/ChangeLog (218895 => 218896)
--- trunk/Tools/ChangeLog 2017-06-28 22:27:28 UTC (rev 218895)
+++ trunk/Tools/ChangeLog 2017-06-28 22:40:10 UTC (rev 218896)
@@ -1,3 +1,12 @@
+2017-06-28 Brady Eidson <beid...@apple.com>
+
+ DocumentLoader should always notify the client if there are pending icon loads when the load is stopped.
+ https://bugs.webkit.org/show_bug.cgi?id=173874
+
+ Reviewed by Alex Christensen.
+
+ * TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm:
+
2017-06-28 Don Olmstead <don.olmst...@sony.com>
Unreviewed, adding Don Olmstead to contributors.json
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm (218895 => 218896)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm 2017-06-28 22:27:28 UTC (rev 218895)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/IconLoadingDelegate.mm 2017-06-28 22:40:10 UTC (rev 218896)
@@ -39,11 +39,16 @@
#if WK_API_ENABLED
static bool doneWithIcons;
-static bool receivedFaviconDataCallback;
static bool alreadyProvidedIconData;
-static RetainPtr<NSData> receivedFaviconData;
-@interface IconLoadingDelegate : NSObject <_WKIconLoadingDelegate>
+@interface IconLoadingDelegate : NSObject <_WKIconLoadingDelegate> {
+ @public
+ RetainPtr<NSData> receivedFaviconData;
+ bool receivedFaviconDataCallback;
+ bool shouldSaveCallback;
+ bool didSaveCallback;
+ void (^savedCallback)(void (^)(NSData*));
+}
@end
@implementation IconLoadingDelegate {
@@ -70,7 +75,13 @@
doneWithIcons = true;
if (parameters.iconType == WKLinkIconTypeFavicon) {
- completionHandler([](NSData *iconData) {
+ if (shouldSaveCallback) {
+ savedCallback = [completionHandler retain];
+ didSaveCallback = true;
+ return;
+ }
+
+ completionHandler([self](NSData *iconData) {
receivedFaviconData = iconData;
receivedFaviconDataCallback = true;
});
@@ -80,7 +91,12 @@
@end
-@interface IconLoadingSchemeHandler : NSObject <WKURLSchemeHandler>
+@interface IconLoadingSchemeHandler : NSObject <WKURLSchemeHandler> {
+ @public
+ bool shouldIgnoreFaviconTask;
+ bool receivedFaviconTask;
+ bool faviconTaskStopped;
+}
- (instancetype)initWithData:(NSData *)data;
- (void)setFaviconData:(NSData *)data;
@end
@@ -113,6 +129,10 @@
if ([[task.request.URL absoluteString] isEqual:@"testing:///favicon.ico"]) {
EXPECT_FALSE(alreadyProvidedIconData);
+ if (shouldIgnoreFaviconTask) {
+ receivedFaviconTask = true;
+ return;
+ }
response = adoptNS([[NSURLResponse alloc] initWithURL:task.request.URL MIMEType:@"image/png" expectedContentLength:1 textEncodingName:nil]);
data = ""
alreadyProvidedIconData = true;
@@ -128,6 +148,8 @@
- (void)webView:(WKWebView *)webView stopURLSchemeTask:(id <WKURLSchemeTask>)task
{
+ if ([[task.request.URL absoluteString] isEqual:@"testing:///favicon.ico"])
+ faviconTaskStopped = true;
}
@end
@@ -154,6 +176,7 @@
[webView loadRequest:request];
TestWebKitAPI::Util::run(&doneWithIcons);
+ TestWebKitAPI::Util::run(&iconDelegate.get()->receivedFaviconDataCallback);
}
static const char mainBytes2[] =
@@ -180,20 +203,86 @@
NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"testing:///main"]];
[webView loadRequest:request];
- TestWebKitAPI::Util::run(&receivedFaviconDataCallback);
+ TestWebKitAPI::Util::run(&iconDelegate.get()->receivedFaviconDataCallback);
- EXPECT_TRUE([iconDataFromDisk.get() isEqual:receivedFaviconData.get()]);
+ EXPECT_TRUE([iconDataFromDisk.get() isEqual:iconDelegate.get()->receivedFaviconData.get()]);
- receivedFaviconDataCallback = false;
- receivedFaviconData = nil;
+ iconDelegate.get()->receivedFaviconDataCallback = false;
+ iconDelegate.get()->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);
+ TestWebKitAPI::Util::run(&iconDelegate.get()->receivedFaviconDataCallback);
- EXPECT_TRUE([iconDataFromDisk.get() isEqual:receivedFaviconData.get()]);
+ EXPECT_TRUE([iconDataFromDisk.get() isEqual:iconDelegate.get()->receivedFaviconData.get()]);
}
+TEST(IconLoading, IconLoadCancelledCallback)
+{
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+ NSData *mainData = [NSData dataWithBytesNoCopy:(void*)mainBytes length:sizeof(mainBytes) freeWhenDone:NO];
+ RetainPtr<IconLoadingSchemeHandler> handler = adoptNS([[IconLoadingSchemeHandler alloc] initWithData:mainData]);
+ handler.get()->shouldIgnoreFaviconTask = true;
+ [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(&handler.get()->receivedFaviconTask);
+
+ // Our scheme handler never replies to the favicon task, so our icon delegate load callback is still pending.
+ // Stop the documentloader's loading and verify the icon delegate callback is called.
+ [webView stopLoading];
+
+ // Wait until the data callback is called, *and* the task is stopped
+ TestWebKitAPI::Util::run(&handler.get()->faviconTaskStopped);
+ TestWebKitAPI::Util::run(&iconDelegate.get()->receivedFaviconDataCallback);
+
+ EXPECT_EQ(iconDelegate.get()->receivedFaviconData.get().length, (unsigned long)0);
+}
+
+TEST(IconLoading, IconLoadCancelledCallback2)
+{
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+
+ NSData *mainData = [NSData dataWithBytesNoCopy:(void*)mainBytes length:sizeof(mainBytes) freeWhenDone:NO];
+ RetainPtr<IconLoadingSchemeHandler> handler = adoptNS([[IconLoadingSchemeHandler alloc] initWithData:mainData]);
+ [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]);
+ iconDelegate.get()->shouldSaveCallback = true;
+ webView.get()._iconLoadingDelegate = iconDelegate.get();
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"testing:///main"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&iconDelegate.get()->didSaveCallback);
+
+ // Our scheme handler never replies to the favicon task, so our icon delegate load callback is still pending.
+ // Stop the documentloader's loading and verify the icon delegate callback is called.
+ [webView stopLoading];
+
+
+ // Even though loading has already been stopped (and therefore IconLoaders were cancelled),
+ // we should still get the callback.
+ static bool iconCallbackCalled;
+ iconDelegate.get()->savedCallback([iconCallbackCalled = &iconCallbackCalled](NSData *data) {
+ EXPECT_EQ(data.length, (unsigned long)0);
+
+ *iconCallbackCalled = true;
+ });
+
+ TestWebKitAPI::Util::run(&iconCallbackCalled);
+}
+
#endif
