Changeset 174889 in webkit

Timestamp:

Oct 20, 2014 10:17:32 AM (10 years ago)

Author:

Antti Koivisto

Message:

Avoid unnecessary NSURLResponse construction for QuickLook on iOS
​https://bugs.webkit.org/show_bug.cgi?id=137876

Reviewed by Andy Estes.

Source/WebCore:

QuickLook specific code path creates NSURLResponse in the web process for every response.
It is rarely needed so this is unnecessary work.

• WebCore.exp.in:
• platform/network/ios/QuickLook.h:
• platform/network/ios/QuickLook.mm:

(WebCore::isMainResourceLoader):
(WebCore::QuickLookHandle::shouldCreateForMIMEType):

Separate function for testing if QuickLookHandle is needed.

(WebCore::QuickLookHandle::create):

Switch the interface to take ResourceResponse.

Source/WebKit2:

• WebProcess/Network/WebResourceLoader.cpp:

(WebKit::WebResourceLoader::didReceiveResponse):

Switch to the modified QuickLookHandle interface that does not need NSURLResponse.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/WebCore.exp.in (modified) (1 diff)
• WebCore/platform/network/ios/QuickLook.h (modified) (2 diffs)
• WebCore/platform/network/ios/QuickLook.mm (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/Network/WebResourceLoader.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-10-19  Antti Koivisto  <antti@apple.com>
+
+        Avoid unnecessary NSURLResponse construction for QuickLook on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=137876
+
+        Reviewed by Andy Estes.
+
+        QuickLook specific code path creates NSURLResponse in the web process for every response.
+        It is rarely needed so this is unnecessary work.
+
+        * WebCore.exp.in:
+        * platform/network/ios/QuickLook.h:
+        * platform/network/ios/QuickLook.mm:
+        (WebCore::isMainResourceLoader):
+        (WebCore::QuickLookHandle::shouldCreateForMIMEType):
+
+            Separate function for testing if QuickLookHandle is needed.
+
+        (WebCore::QuickLookHandle::create):
+
+            Switch the interface to take ResourceResponse.
+
 2014-10-20  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -3335 +3335 @@
 __ZN7WebCore15QuickLookHandle14didReceiveDataEPK8__CFData
 __ZN7WebCore15QuickLookHandle16didFinishLoadingEv
-__ZN7WebCore15QuickLookHandle6createEPNS_14ResourceLoaderEP13NSURLResponse
+__ZN7WebCore15QuickLookHandle23shouldCreateForMIMETypeERKN3WTF6StringE
+__ZN7WebCore15QuickLookHandle6createERNS_14ResourceLoaderERKNS_16ResourceResponseE
 __ZN7WebCore15QuickLookHandle7didFailEv
 __ZN7WebCore15QuickLookHandleD1Ev

trunk/Source/WebCore/platform/network/ios/QuickLook.h

@@ -55 +55 @@
 class ResourceHandle;
 class ResourceLoader;
+class ResourceResponse;
 class SynchronousResourceHandleCFURLConnectionDelegate;
 
@@ -83 +84 @@
     WTF_MAKE_NONCOPYABLE(QuickLookHandle);
 public:
+    WEBCORE_EXPORT static bool shouldCreateForMIMEType(const String&);
+
     static std::unique_ptr<QuickLookHandle> create(ResourceHandle*, NSURLConnection *, NSURLResponse *, id delegate);
 #if USE(CFNETWORK)
     static std::unique_ptr<QuickLookHandle> create(ResourceHandle*, SynchronousResourceHandleCFURLConnectionDelegate*, CFURLResponseRef);
 #endif
-    WEBCORE_EXPORT static std::unique_ptr<QuickLookHandle> create(ResourceLoader*, NSURLResponse *);
+    // FIXME: Use of ResourceLoader here is a platform violation.
+    WEBCORE_EXPORT static std::unique_ptr<QuickLookHandle> create(ResourceLoader&, const ResourceResponse&);
+
     WEBCORE_EXPORT ~QuickLookHandle();
 

trunk/Source/WebCore/platform/network/ios/QuickLook.mm

@@ -489 +489 @@
 #endif
 
-static inline bool isMainResourceLoader(ResourceLoader* loader)
-{
-    return loader->documentLoader()->mainResourceLoader() == loader;
-}
-
-std::unique_ptr<QuickLookHandle> QuickLookHandle::create(ResourceLoader* loader, NSURLResponse *response)
-{
-    ASSERT_ARG(loader, loader);
-    if (!isMainResourceLoader(loader) || ![WebCore::QLPreviewGetSupportedMIMETypesSet() containsObject:[response MIMEType]])
-        return nullptr;
-
-    RetainPtr<WebResourceLoaderQuickLookDelegate> delegate = adoptNS([[WebResourceLoaderQuickLookDelegate alloc] initWithResourceLoader:loader]);
-    std::unique_ptr<QuickLookHandle> quickLookHandle(new QuickLookHandle([loader->originalRequest().nsURLRequest(DoNotUpdateHTTPBody) URL], nil, response, delegate.get()));
+bool QuickLookHandle::shouldCreateForMIMEType(const String& mimeType)
+{
+    return [WebCore::QLPreviewGetSupportedMIMETypesSet() containsObject:mimeType];
+}
+
+std::unique_ptr<QuickLookHandle> QuickLookHandle::create(ResourceLoader& loader, const ResourceResponse& response)
+{
+    ASSERT(shouldCreateForMIMEType(response.mimeType()));
+
+    RetainPtr<WebResourceLoaderQuickLookDelegate> delegate = adoptNS([[WebResourceLoaderQuickLookDelegate alloc] initWithResourceLoader:&loader]);
+    std::unique_ptr<QuickLookHandle> quickLookHandle(new QuickLookHandle([loader.originalRequest().nsURLRequest(DoNotUpdateHTTPBody) URL], nil, response.nsURLResponse(), delegate.get()));
     [delegate setQuickLookHandle:quickLookHandle.get()];
-    loader->didCreateQuickLookHandle(*quickLookHandle);
+    loader.didCreateQuickLookHandle(*quickLookHandle);
     return WTF::move(quickLookHandle);
 }

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-10-19  Antti Koivisto  <antti@apple.com>
+
+        Avoid unnecessary NSURLResponse construction for QuickLook on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=137876
+
+        Reviewed by Andy Estes.
+
+        * WebProcess/Network/WebResourceLoader.cpp:
+        (WebKit::WebResourceLoader::didReceiveResponse):
+
+            Switch to the modified QuickLookHandle interface that does not need NSURLResponse.
+
 2014-10-16  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp

@@ -113 +113 @@
         return;
 
+    bool shoudCallCoreLoaderDidReceiveResponse = true;
 #if USE(QUICK_LOOK)
     // Refrain from calling didReceiveResponse if QuickLook will convert this response, since the MIME type of the
     // converted resource isn't yet known. WebResourceLoaderQuickLookDelegate will later call didReceiveResponse upon
     // receiving the converted data.
-    m_coreLoader->documentLoader()->setQuickLookHandle(QuickLookHandle::create(resourceLoader(), response.nsURLResponse()));
-    if (!m_coreLoader->documentLoader()->quickLookHandle())
-#endif
+    bool isMainLoad = m_coreLoader->documentLoader()->mainResourceLoader() == m_coreLoader;
+    if (isMainLoad && QuickLookHandle::shouldCreateForMIMEType(response.mimeType())) {
+        m_coreLoader->documentLoader()->setQuickLookHandle(QuickLookHandle::create(*m_coreLoader, response));
+        shoudCallCoreLoaderDidReceiveResponse = false;
+    }
+#endif
+    if (shoudCallCoreLoaderDidReceiveResponse)
         m_coreLoader->didReceiveResponse(response);