Attachment 346797 Details for Bug 188417 – Patch

[patch] Patch

bug-188417-20180808143231.patch (text/plain), 18.98 KB, created by Alex Christensen on 2018-08-08 14:32:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-08-08 14:32:33 PDT

Size: 18.98 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 234710)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,54 @@
>+2018-08-08  Alex Christensen  <achristensen@webkit.org>
>+
>+        Consolidate data/string API loading paths
>+        https://bugs.webkit.org/show_bug.cgi?id=188417
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-08  Alex Christensen  <achristensen@webkit.org>
>+
>+        Fix possible null dereference in WebBackForwardList::restoreFromState
>+        https://bugs.webkit.org/show_bug.cgi?id=188418
>+        <rdar://problem/42531726>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/WebBackForwardList.cpp:
>+        (WebKit::WebBackForwardList::restoreFromState):
>+
> 2018-08-08  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r234314, r234320, and r234321.
>Index: Source/WebKit/UIProcess/WebPageProxy.cpp
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.cpp	(revision 234663)
>+++ Source/WebKit/UIProcess/WebPageProxy.cpp	(working copy)
>@@ -1048,7 +1048,7 @@ RefPtr<API::Navigation> WebPageProxy::lo
>     return WTFMove(navigation);
> }
> 
>-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)
>         return nullptr;
>@@ -1064,7 +1064,7 @@ RefPtr<API::Navigation> WebPageProxy::lo
> 
>     LoadParameters loadParameters;
>     loadParameters.navigationID = navigation->navigationID();
>-    loadParameters.data = data->dataReference();
>+    loadParameters.data = data;
>     loadParameters.MIMEType = MIMEType;
>     loadParameters.encodingName = encoding;
>     loadParameters.baseURLString = baseURL;
>@@ -1078,36 +1078,6 @@ RefPtr<API::Navigation> WebPageProxy::lo
>     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();
>-
>-    return WTFMove(navigation);
>-}
>-
> void WebPageProxy::loadAlternateHTMLString(const String& htmlString, const WebCore::URL& baseURL, const WebCore::URL& unreachableURL, API::Object* userData)
> {
>     // When the UIProcess is in the process of handling a failing provisional load, do not attempt to
>@@ -1145,28 +1115,6 @@ void WebPageProxy::loadAlternateHTMLStri
>     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();
>-}
>-
> void WebPageProxy::loadWebArchiveData(API::Data* webArchiveData, API::Object* userData)
> {
>     if (m_isClosed)
>Index: Source/WebKit/UIProcess/WebPageProxy.h
>===================================================================
>--- Source/WebKit/UIProcess/WebPageProxy.h	(revision 234663)
>+++ Source/WebKit/UIProcess/WebPageProxy.h	(working copy)
>@@ -456,10 +456,8 @@ public:
>     void addPlatformLoadParameters(LoadParameters&);
>     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);
> 
>Index: Source/WebKit/UIProcess/API/C/WKPage.cpp
>===================================================================
>--- Source/WebKit/UIProcess/API/C/WKPage.cpp	(revision 234663)
>+++ Source/WebKit/UIProcess/API/C/WKPage.cpp	(working copy)
>@@ -191,22 +191,37 @@ void WKPageLoadFileWithUserData(WKPageRe
> 
> 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);
> }
> 
> void WKPageLoadAlternateHTMLString(WKPageRef pageRef, WKStringRef htmlStringRef, WKURLRef baseURLRef, WKURLRef unreachableURLRef)
>@@ -221,12 +236,12 @@ void WKPageLoadAlternateHTMLStringWithUs
> 
> 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);
> }
> 
> void WKPageLoadWebArchiveData(WKPageRef pageRef, WKDataRef webArchiveDataRef)
>Index: Source/WebKit/UIProcess/API/Cocoa/WKBrowsingContextController.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKBrowsingContextController.mm	(revision 234663)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKBrowsingContextController.mm	(working copy)
>@@ -153,7 +153,8 @@ - (void)loadHTMLString:(NSString *)HTMLS
>     if (userData)
>         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());
> }
> 
> - (void)loadAlternateHTMLString:(NSString *)string baseURL:(NSURL *)baseURL forUnreachableURL:(NSURL *)unreachableURL
>@@ -168,17 +169,11 @@ - (void)loadData:(NSData *)data MIMEType
> 
> - (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());
> }
> 
> - (void)stopLoading
>Index: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	(revision 234663)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	(working copy)
>@@ -903,7 +903,7 @@ - (WKNavigation *)loadHTMLString:(NSStri
> 
> - (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;
> 
>@@ -4237,7 +4237,7 @@ - (void)_loadAlternateHTMLString:(NSStri
> 
> - (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;
> 
>Index: Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp
>===================================================================
>--- Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp	(revision 234663)
>+++ Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp	(working copy)
>@@ -26,6 +26,7 @@
> #include "APIData.h"
> #include "APINavigation.h"
> #include "APISerializedScriptValue.h"
>+#include "DataReference.h"
> #include "ImageOptions.h"
> #include "WebCertificateInfo.h"
> #include "WebContextMenuItem.h"
>@@ -2583,7 +2584,7 @@ void webkit_web_view_load_html(WebKitWeb
>     g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));
>     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));
> }
> 
> /**
>@@ -2622,13 +2623,7 @@ void webkit_web_view_load_plain_text(Web
>     g_return_if_fail(WEBKIT_IS_WEB_VIEW(webView));
>     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());
> }
> 
> /**
>@@ -2656,11 +2651,7 @@ void webkit_web_view_load_bytes(WebKitWe
>     gconstpointer bytesData = g_bytes_get_data(bytes, &bytesDataSize);
>     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));
> }
> 
>Index: Source/WebKit/WebProcess/WebPage/WebPage.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.cpp	(revision 234663)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.cpp	(working copy)
>@@ -1318,7 +1318,7 @@ void WebPage::loadStringImpl(uint64_t na
>     }
> }
> 
>-void WebPage::loadData(const LoadParameters& loadParameters)
>+void WebPage::loadData(LoadParameters&& loadParameters)
> {
>     platformDidReceiveLoadParameters(loadParameters);
> 
>@@ -1327,14 +1327,6 @@ void WebPage::loadData(const LoadParamet
>     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);
>-}
>-
> void WebPage::loadAlternateHTMLString(const LoadParameters& loadParameters)
> {
>     platformDidReceiveLoadParameters(loadParameters);
>Index: Source/WebKit/WebProcess/WebPage/WebPage.h
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.h	(revision 234663)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.h	(working copy)
>@@ -1161,8 +1161,7 @@ private:
>     void tryClose();
>     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);
>     void reload(uint64_t navigationID, uint32_t reloadOptions, SandboxExtension::Handle&&);
>Index: Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>===================================================================
>--- Source/WebKit/WebProcess/WebPage/WebPage.messages.in	(revision 234663)
>+++ Source/WebKit/WebProcess/WebPage/WebPage.messages.in	(working copy)
>@@ -146,7 +146,6 @@ messages -> WebPage LegacyReceiver {
>     LoadURLInFrame(WebCore::URL url, uint64_t frameID)
>     LoadRequest(struct WebKit::LoadParameters loadParameters)
>     LoadData(struct WebKit::LoadParameters loadParameters)
>-    LoadString(struct WebKit::LoadParameters loadParameters)
>     LoadAlternateHTMLString(struct WebKit::LoadParameters loadParameters)
> 
>     NavigateToPDFLinkWithSimulatedClick(String url, WebCore::IntPoint documentPoint, WebCore::IntPoint screenPoint)

Actions: View | Formatted Diff | Diff

Attachments on bug 188417: 346791 | 346792 | 346796 | 346797 | 346800 | 347187