Changeset 234954 in webkit

Timestamp:

Aug 16, 2018 1:52:30 PM (6 years ago)

Author:

achristensen@apple.com

Message:

Consolidate data/string API loading paths
​https://bugs.webkit.org/show_bug.cgi?id=188417

Reviewed by Michael Catanzaro.

loadHTMLString and loadData are basically duplicate code.
loadPlainTextString was also basically the same except it didn't set up a navigation, which
was almost certainly a bug, but nobody uses it in all of Apple and Debian. We should probably deprecate
and remove it, but for now I make it use the same data loading path.

• UIProcess/API/C/WKPage.cpp:

(WKPageLoadData):
(WKPageLoadDataWithUserData):
(loadString):
(WKPageLoadHTMLString):
(WKPageLoadHTMLStringWithUserData):
(WKPageLoadPlainTextString):
(WKPageLoadPlainTextStringWithUserData):

• UIProcess/API/Cocoa/WKBrowsingContextController.mm:

(-[WKBrowsingContextController loadHTMLString:baseURL:userData:]):
(-[WKBrowsingContextController loadData:MIMEType:textEncodingName:baseURL:userData:]):

• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView loadData:MIMEType:characterEncodingName:baseURL:]):
(-[WKWebView _loadData:MIMEType:characterEncodingName:baseURL:userData:]):

• UIProcess/API/glib/WebKitWebView.cpp:

(webkit_web_view_load_html):
(webkit_web_view_load_plain_text):

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::loadData):
(WebKit::WebPageProxy::loadHTMLString): Deleted.
(WebKit::WebPageProxy::loadPlainTextString): Deleted.

• UIProcess/WebPageProxy.h:
• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::loadData):
(WebKit::WebPage::loadString): Deleted.

• WebProcess/WebPage/WebPage.h:
• WebProcess/WebPage/WebPage.messages.in:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/C/WKPage.cpp (modified) (2 diffs)
• UIProcess/API/Cocoa/WKBrowsingContextController.mm (modified) (2 diffs)
• UIProcess/API/Cocoa/WKWebView.mm (modified) (2 diffs)
• UIProcess/API/glib/WebKitWebView.cpp (modified) (4 diffs)
• UIProcess/WebPageProxy.cpp (modified) (4 diffs)
• UIProcess/WebPageProxy.h (modified) (1 diff)
• WebProcess/WebPage/WebPage.cpp (modified) (3 diffs)
• WebProcess/WebPage/WebPage.h (modified) (2 diffs)
• WebProcess/WebPage/WebPage.messages.in (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-08-16  Alex Christensen  <achristensen@webkit.org>
+
+        Consolidate data/string API loading paths
+        https://bugs.webkit.org/show_bug.cgi?id=188417
+
+        Reviewed by Michael Catanzaro.
+
+        loadHTMLString and loadData are basically duplicate code.
+        loadPlainTextString was also basically the same except it didn't set up a navigation, which
+        was almost certainly a bug, but nobody uses it in all of Apple and Debian.  We should probably deprecate
+        and remove it, but for now I make it use the same data loading path.
+
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageLoadData):
+        (WKPageLoadDataWithUserData):
+        (loadString):
+        (WKPageLoadHTMLString):
+        (WKPageLoadHTMLStringWithUserData):
+        (WKPageLoadPlainTextString):
+        (WKPageLoadPlainTextStringWithUserData):
+        * UIProcess/API/Cocoa/WKBrowsingContextController.mm:
+        (-[WKBrowsingContextController loadHTMLString:baseURL:userData:]):
+        (-[WKBrowsingContextController loadData:MIMEType:textEncodingName:baseURL:userData:]):
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView loadData:MIMEType:characterEncodingName:baseURL:]):
+        (-[WKWebView _loadData:MIMEType:characterEncodingName:baseURL:userData:]):
+        * UIProcess/API/glib/WebKitWebView.cpp:
+        (webkit_web_view_load_html):
+        (webkit_web_view_load_plain_text):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::loadData):
+        (WebKit::WebPageProxy::loadHTMLString): Deleted.
+        (WebKit::WebPageProxy::loadPlainTextString): Deleted.
+        * UIProcess/WebPageProxy.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::loadData):
+        (WebKit::WebPage::loadString): Deleted.
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
 2018-08-16  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp

@@ -192 +192 @@
 void WKPageLoadData(WKPageRef pageRef, WKDataRef dataRef, WKStringRef MIMETypeRef, WKStringRef encodingRef, WKURLRef baseURLRef)
 {
-    toImpl(pageRef)->loadData(toImpl(dataRef), toWTFString(MIMETypeRef), toWTFString(encodingRef), toWTFString(baseURLRef));
+    toImpl(pageRef)->loadData(toImpl(dataRef)->dataReference(), toWTFString(MIMETypeRef), toWTFString(encodingRef), toWTFString(baseURLRef));
 }
 
 void WKPageLoadDataWithUserData(WKPageRef pageRef, WKDataRef dataRef, WKStringRef MIMETypeRef, WKStringRef encodingRef, WKURLRef baseURLRef, WKTypeRef userDataRef)
 {
-    toImpl(pageRef)->loadData(toImpl(dataRef), toWTFString(MIMETypeRef), toWTFString(encodingRef), toWTFString(baseURLRef), toImpl(userDataRef));
+    toImpl(pageRef)->loadData(toImpl(dataRef)->dataReference(), toWTFString(MIMETypeRef), toWTFString(encodingRef), toWTFString(baseURLRef), toImpl(userDataRef));
+}
+
+static void loadString(WKPageRef pageRef, WKStringRef stringRef, const String& mimeType, const String& baseURL, WKTypeRef userDataRef)
+{
+    String string = toWTFString(stringRef);
+    String encoding;
+    IPC::DataReference data;
+    if (string.isNull() || string.is8Bit()) {
+        encoding = "latin1"_s;
+        data = { reinterpret_cast<const uint8_t*>(string.characters8()), string.length() * sizeof(LChar) };
+    } else {
+        encoding = "utf-16"_s;
+        data = { reinterpret_cast<const uint8_t*>(string.characters16()), string.length() * sizeof(UChar) };
+    }
+    toImpl(pageRef)->loadData(data, mimeType, encoding, baseURL, toImpl(userDataRef));
 }
 
 void WKPageLoadHTMLString(WKPageRef pageRef, WKStringRef htmlStringRef, WKURLRef baseURLRef)
 {
-    toImpl(pageRef)->loadHTMLString(toWTFString(htmlStringRef), toWTFString(baseURLRef));
+    WKPageLoadHTMLStringWithUserData(pageRef, htmlStringRef, baseURLRef, nullptr);
 }
 
 void WKPageLoadHTMLStringWithUserData(WKPageRef pageRef, WKStringRef htmlStringRef, WKURLRef baseURLRef, WKTypeRef userDataRef)
 {
-    toImpl(pageRef)->loadHTMLString(toWTFString(htmlStringRef), toWTFString(baseURLRef), toImpl(userDataRef));
+    loadString(pageRef, htmlStringRef, "text/html"_s, toWTFString(baseURLRef), userDataRef);
 }
 
@@ -222 +237 @@
 void WKPageLoadPlainTextString(WKPageRef pageRef, WKStringRef plainTextStringRef)
 {
-    toImpl(pageRef)->loadPlainTextString(toWTFString(plainTextStringRef));    
+    WKPageLoadPlainTextStringWithUserData(pageRef, plainTextStringRef, nullptr);
 }
 
 void WKPageLoadPlainTextStringWithUserData(WKPageRef pageRef, WKStringRef plainTextStringRef, WKTypeRef userDataRef)
 {
-    toImpl(pageRef)->loadPlainTextString(toWTFString(plainTextStringRef), toImpl(userDataRef));    
+    loadString(pageRef, plainTextStringRef, "text/plain"_s, blankURL().string(), userDataRef);
 }
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKBrowsingContextController.mm

@@ -154 +154 @@
         wkUserData = ObjCObjectGraph::create(userData);
 
-    _page->loadHTMLString(HTMLString, [baseURL _web_originalDataAsWTFString], wkUserData.get());
+    NSData *data = [HTMLString dataUsingEncoding:NSUTF8StringEncoding];
+    _page->loadData({ static_cast<const uint8_t*>(data.bytes), data.length }, "text/html"_s, "UTF-8"_s, [baseURL _web_originalDataAsWTFString], wkUserData.get());
 }
 
@@ -169 +170 @@
 - (void)loadData:(NSData *)data MIMEType:(NSString *)MIMEType textEncodingName:(NSString *)encodingName baseURL:(NSURL *)baseURL userData:(id)userData
 {
-    RefPtr<API::Data> apiData;
-    if (data) {
-        // FIXME: This should copy the data.
-        apiData = API::Data::createWithoutCopying(data);
-    }
-
     RefPtr<ObjCObjectGraph> wkUserData;
     if (userData)
         wkUserData = ObjCObjectGraph::create(userData);
 
-    _page->loadData(apiData.get(), MIMEType, encodingName, [baseURL _web_originalDataAsWTFString], wkUserData.get());
+    _page->loadData({ static_cast<const uint8_t*>(data.bytes), data.length }, MIMEType, encodingName, [baseURL _web_originalDataAsWTFString], wkUserData.get());
 }
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -889 +889 @@
 - (WKNavigation *)loadData:(NSData *)data MIMEType:(NSString *)MIMEType characterEncodingName:(NSString *)characterEncodingName baseURL:(NSURL *)baseURL
 {
-    auto navigation = _page->loadData(API::Data::createWithoutCopying(data).ptr(), MIMEType, characterEncodingName, baseURL.absoluteString);
+    auto navigation = _page->loadData({ static_cast<const uint8_t*>(data.bytes), data.length }, MIMEType, characterEncodingName, baseURL.absoluteString);
     if (!navigation)
         return nil;
@@ -4223 +4223 @@
 - (WKNavigation *)_loadData:(NSData *)data MIMEType:(NSString *)MIMEType characterEncodingName:(NSString *)characterEncodingName baseURL:(NSURL *)baseURL userData:(id)userData
 {
-    auto navigation = _page->loadData(API::Data::createWithoutCopying(data).ptr(), MIMEType, characterEncodingName, baseURL.absoluteString, WebKit::ObjCObjectGraph::create(userData).ptr());
+    auto navigation = _page->loadData({ static_cast<const uint8_t*>(data.bytes), data.length }, MIMEType, characterEncodingName, baseURL.absoluteString, WebKit::ObjCObjectGraph::create(userData).ptr());
     if (!navigation)
         return nil;

trunk/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp

@@ -27 +27 @@
 #include "APINavigation.h"
 #include "APISerializedScriptValue.h"
+#include "DataReference.h"
 #include "ImageOptions.h"
 #include "WebCertificateInfo.h"
@@ -2584 +2585 @@
     g_return_if_fail(content);
 
-    getPage(webView).loadHTMLString(String::fromUTF8(content), String::fromUTF8(baseURI));
+    getPage(webView).loadData({ reinterpret_cast<const uint8_t*>(content), content ? strlen(content) : 0 }, "text/html"_s, "UTF-8"_s, String::fromUTF8(baseURI));
 }
 
@@ -2623 +2624 @@
     g_return_if_fail(plainText);
 
-    getPage(webView).loadPlainTextString(String::fromUTF8(plainText));
-}
-
-static void releaseGBytes(unsigned char*, const void* bytes)
-{
-    // Balanced by g_bytes_ref in webkit_web_view_load_bytes().
-    g_bytes_unref(static_cast<GBytes*>(const_cast<void*>(bytes)));
+    getPage(webView).loadData({ reinterpret_cast<const uint8_t*>(plainText), plainText ? strlen(plainText) : 0 }, "text/plain"_s, "UTF-8"_s, blankURL().string());
 }
 
@@ -2657 +2652 @@
     g_return_if_fail(bytesDataSize);
 
-    // Balanced by g_bytes_unref in releaseGBytes.
-    g_bytes_ref(bytes);
-
-    Ref<API::Data> data = API::Data::createWithoutCopying(static_cast<const unsigned char*>(bytesData), bytesDataSize, releaseGBytes, bytes);
-    getPage(webView).loadData(data.ptr(), mimeType ? String::fromUTF8(mimeType) : String::fromUTF8("text/html"),
+    getPage(webView).loadData({ reinterpret_cast<const uint8_t*>(bytesData), bytesDataSize }, mimeType ? String::fromUTF8(mimeType) : String::fromUTF8("text/html"),
         encoding ? String::fromUTF8(encoding) : String::fromUTF8("UTF-8"), String::fromUTF8(baseURI));
 }

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -1050 +1050 @@
 }
 
-RefPtr<API::Navigation> WebPageProxy::loadData(API::Data* data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData)
+RefPtr<API::Navigation> WebPageProxy::loadData(const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData)
 {
     if (m_isClosed)
@@ -1066 +1066 @@
     LoadParameters loadParameters;
     loadParameters.navigationID = navigation->navigationID();
-    loadParameters.data = data->dataReference();
+    loadParameters.data = data;
     loadParameters.MIMEType = MIMEType;
     loadParameters.encodingName = encoding;
@@ -1075 +1075 @@
     m_process->assumeReadAccessToBaseURL(baseURL);
     m_process->send(Messages::WebPage::LoadData(loadParameters), m_pageID);
-    m_process->responsivenessTimer().start();
-
-    return WTFMove(navigation);
-}
-
-// FIXME: Get rid of loadHTMLString and just use loadData instead.
-RefPtr<API::Navigation> WebPageProxy::loadHTMLString(const String& htmlString, const String& baseURL, API::Object* userData)
-{
-    if (m_isClosed)
-        return nullptr;
-
-    auto navigation = m_navigationState->createLoadDataNavigation();
-
-    auto transaction = m_pageLoadState.transaction();
-
-    m_pageLoadState.setPendingAPIRequestURL(transaction, !baseURL.isEmpty() ? baseURL : blankURL().string());
-
-    if (!isValid())
-        reattachToWebProcess();
-
-    LoadParameters loadParameters;
-    loadParameters.navigationID = navigation->navigationID();
-    loadParameters.string = htmlString;
-    loadParameters.MIMEType = "text/html"_s;
-    loadParameters.baseURLString = baseURL;
-    loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
-    addPlatformLoadParameters(loadParameters);
-
-    m_process->assumeReadAccessToBaseURL(baseURL);
-    m_process->send(Messages::WebPage::LoadString(loadParameters), m_pageID);
     m_process->responsivenessTimer().start();
 
@@ -1144 +1114 @@
     m_process->assumeReadAccessToBaseURL(unreachableURL);
     m_process->send(Messages::WebPage::LoadAlternateHTMLString(loadParameters), m_pageID);
-    m_process->responsivenessTimer().start();
-}
-
-void WebPageProxy::loadPlainTextString(const String& string, API::Object* userData)
-{
-    if (m_isClosed)
-        return;
-
-    if (!isValid())
-        reattachToWebProcess();
-
-    auto transaction = m_pageLoadState.transaction();
-    m_pageLoadState.setPendingAPIRequestURL(transaction, blankURL().string());
-
-    LoadParameters loadParameters;
-    loadParameters.navigationID = 0;
-    loadParameters.string = string;
-    loadParameters.MIMEType = "text/plain"_s;
-    loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
-    addPlatformLoadParameters(loadParameters);
-
-    m_process->send(Messages::WebPage::LoadString(loadParameters), m_pageID);
     m_process->responsivenessTimer().start();
 }

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -457 +457 @@
     RefPtr<API::Navigation> loadRequest(WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy = WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, API::Object* userData = nullptr);
     RefPtr<API::Navigation> loadFile(const String& fileURL, const String& resourceDirectoryURL, API::Object* userData = nullptr);
-    RefPtr<API::Navigation> loadData(API::Data*, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData = nullptr);
-    RefPtr<API::Navigation> loadHTMLString(const String& htmlString, const String& baseURL, API::Object* userData = nullptr);
+    RefPtr<API::Navigation> loadData(const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData = nullptr);
     void loadAlternateHTMLString(const String& htmlString, const WebCore::URL& baseURL, const WebCore::URL& unreachableURL, API::Object* userData = nullptr);
-    void loadPlainTextString(const String&, API::Object* userData = nullptr);
     void loadWebArchiveData(API::Data*, API::Object* userData = nullptr);
     void navigateToPDFLinkWithSimulatedClick(const String& url, WebCore::IntPoint documentPoint, WebCore::IntPoint screenPoint);

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -1304 +1304 @@
 }
 
-void WebPage::loadStringImpl(uint64_t navigationID, const String& htmlString, const String& MIMEType, const URL& baseURL, const URL& unreachableURL, const UserData& userData)
-{
-    if (!htmlString.isNull() && htmlString.is8Bit()) {
-        auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(htmlString.characters8()), htmlString.length() * sizeof(LChar));
-        loadDataImpl(navigationID, WTFMove(sharedBuffer), MIMEType, "latin1"_s, baseURL, unreachableURL, userData);
-    } else {
-        auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(htmlString.characters16()), htmlString.length() * sizeof(UChar));
-        loadDataImpl(navigationID, WTFMove(sharedBuffer), MIMEType, "utf-16"_s, baseURL, unreachableURL, userData);
-    }
-}
-
-void WebPage::loadData(const LoadParameters& loadParameters)
+void WebPage::loadData(LoadParameters&& loadParameters)
 {
     platformDidReceiveLoadParameters(loadParameters);
@@ -1322 +1311 @@
     URL baseURL = loadParameters.baseURLString.isEmpty() ? blankURL() : URL(URL(), loadParameters.baseURLString);
     loadDataImpl(loadParameters.navigationID, WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, URL(), loadParameters.userData);
-}
-
-void WebPage::loadString(const LoadParameters& loadParameters)
-{
-    platformDidReceiveLoadParameters(loadParameters);
-
-    URL baseURL = loadParameters.baseURLString.isEmpty() ? blankURL() : URL(URL(), loadParameters.baseURLString);
-    loadStringImpl(loadParameters.navigationID, loadParameters.string, loadParameters.MIMEType, baseURL, URL(), loadParameters.userData);
 }
 
@@ -1340 +1321 @@
     URL provisionalLoadErrorURL = loadParameters.provisionalLoadErrorURLString.isEmpty() ? URL() : URL(URL(), loadParameters.provisionalLoadErrorURLString);
     m_mainFrame->coreFrame()->loader().setProvisionalLoadErrorBeingHandledURL(provisionalLoadErrorURL);
-    loadStringImpl(0, loadParameters.string, "text/html"_s, baseURL, unreachableURL, loadParameters.userData);
+    
+    const String& htmlString = loadParameters.string;
+    if (!htmlString.isNull() && htmlString.is8Bit()) {
+        auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(htmlString.characters8()), htmlString.length() * sizeof(LChar));
+        loadDataImpl(0, WTFMove(sharedBuffer), "text/html"_s, "latin1"_s, baseURL, unreachableURL, loadParameters.userData);
+    } else {
+        auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(htmlString.characters16()), htmlString.length() * sizeof(UChar));
+        loadDataImpl(0, WTFMove(sharedBuffer), "text/html"_s, "utf-16"_s, baseURL, unreachableURL, loadParameters.userData);
+    }
     m_mainFrame->coreFrame()->loader().setProvisionalLoadErrorBeingHandledURL({ });
 }

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -1156 +1156 @@
 
     void loadDataImpl(uint64_t navigationID, Ref<WebCore::SharedBuffer>&&, const String& MIMEType, const String& encodingName, const WebCore::URL& baseURL, const WebCore::URL& failingURL, const UserData&);
-    void loadStringImpl(uint64_t navigationID, const String&, const String& MIMEType, const WebCore::URL& baseURL, const WebCore::URL& failingURL, const UserData&);
 
     // Actions
@@ -1162 +1161 @@
     void platformDidReceiveLoadParameters(const LoadParameters&);
     void loadRequest(LoadParameters&&);
-    void loadData(const LoadParameters&);
-    void loadString(const LoadParameters&);
+    void loadData(LoadParameters&&);
     void loadAlternateHTMLString(const LoadParameters&);
     void navigateToPDFLinkWithSimulatedClick(const String& url, WebCore::IntPoint documentPoint, WebCore::IntPoint screenPoint);

trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

@@ -147 +147 @@
     LoadRequest(struct WebKit::LoadParameters loadParameters)
     LoadData(struct WebKit::LoadParameters loadParameters)
-    LoadString(struct WebKit::LoadParameters loadParameters)
     LoadAlternateHTMLString(struct WebKit::LoadParameters loadParameters)