In favor of a URL constructor receiving a SoupURI. We can also add a method to URL to create a soupURI.
Created attachment 221371 [details] Patch
Comment on attachment 221371 [details] Patch Attachment 221371 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/6191449162907648
(In reply to comment #2) > (From update of attachment 221371 [details]) > Attachment 221371 [details] did not pass efl-wk2-ews (efl-wk2): > Output: http://webkit-queues.appspot.com/results/6191449162907648 Last 500 characters of output: ir/__/__/DerivedSources/WebCore/JSSVGAnimatedEnumeration.cpp.o c++: internal compiler error: Killed (program cc1plus) Please submit a full bug report, with preprocessed source if appropriate. See <file:///usr/share/doc/gcc-4.7/README.Bugs> for instructions. make[2]: *** [Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/JSDOMWindow.cpp.o] Error 4 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [Source/WebCore/CMakeFiles/WebCore.dir/all] Error 2 make: *** [all] Error 2 That doesn't seem to be my fault.
Comment on attachment 221371 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221371&action=review > Source/WebCore/platform/URL.h:169 > + SoupURI* createSoupURI() const; Why can't you make createSoupURI() return a GOwnPtr?
(In reply to comment #4) > (From update of attachment 221371 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221371&action=review > > > Source/WebCore/platform/URL.h:169 > > + SoupURI* createSoupURI() const; > > Why can't you make createSoupURI() return a GOwnPtr? Because our GOwnPtr is non copyable. We don't have PassGOwnPtr.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 221371 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=221371&action=review > > > > > Source/WebCore/platform/URL.h:169 > > > + SoupURI* createSoupURI() const; > > > > Why can't you make createSoupURI() return a GOwnPtr? > > Because our GOwnPtr is non copyable. We don't have PassGOwnPtr. See https://bugs.webkit.org/show_bug.cgi?id=127170
Created attachment 221996 [details] New patch Rebased and changed to return GUniquePtr when a new allocated SoupURI is returned.
Created attachment 222271 [details] Try to fix EFL build Not sure I did it right, I haven't found the way yo add include paths only for the webcore test, so I've added to the global includes (which seems to mix includes from webcore, JavaScriptCore, WebKit2, etc.)
Attachment 222271 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/CMakeLists.txt:18: Alphabetical sorting problem. "JavaScriptCore" should be before "WTF". [list/order] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 222272 [details] Try to fix EFL build This time it was WTR
Attachment 222272 [details] did not pass style-queue: ERROR: Tools/TestWebKitAPI/CMakeLists.txt:18: Alphabetical sorting problem. "JavaScriptCore" should be before "WTF". [list/order] [5] Total errors found: 1 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 222272 [details] Try to fix EFL build View in context: https://bugs.webkit.org/attachment.cgi?id=222272&action=review > Source/WebCore/platform/URL.h:168 > + URL(SoupURI*); Perhaps this should be marked "explicit." > Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:129 > +GUniquePtr<SoupURI> ResourceRequest::createSoupURI() const > { > - // WebKit does not support fragment identifiers in data URLs, but soup does. > - // Before passing the URL to soup, we should make sure to urlencode any '#' > - // characters, so that soup does not interpret them as fragment identifiers. > - // See http://wkbug.com/68089 > - if (m_url.protocolIsData()) { > - String urlString = m_url.string(); > - urlString.replace("#", "%23"); > - return soup_uri_new(urlString.utf8().data()); > - } > - > URL url = m_url; > url.removeFragmentIdentifier(); > - SoupURI* uri = soup_uri_new(url.string().utf8().data()); > - > - // Versions of libsoup prior to 2.42 have a soup_uri_new that will convert empty passwords that are not > - // prefixed by a colon into null. Some parts of soup like the SoupAuthenticationManager will only be active > - // when both the username and password are non-null. When we have credentials, empty usernames and passwords > - // should be empty strings instead of null. > - if (!url.user().isEmpty() || !url.pass().isEmpty()) { > - soup_uri_set_user(uri, url.user().utf8().data()); > - soup_uri_set_password(uri, url.pass().utf8().data()); > - } > - return uri; > + return url.createSoupURI(); > } Can we just get rid of this entirely in favor of url().createSoupURI()?
(In reply to comment #12) > (From update of attachment 222272 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=222272&action=review > > > Source/WebCore/platform/URL.h:168 > > + URL(SoupURI*); > > Perhaps this should be marked "explicit." I thought about it, but other platform constructor don't do that so I followed it :-P > > Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:129 > > +GUniquePtr<SoupURI> ResourceRequest::createSoupURI() const > > { > > - // WebKit does not support fragment identifiers in data URLs, but soup does. > > - // Before passing the URL to soup, we should make sure to urlencode any '#' > > - // characters, so that soup does not interpret them as fragment identifiers. > > - // See http://wkbug.com/68089 > > - if (m_url.protocolIsData()) { > > - String urlString = m_url.string(); > > - urlString.replace("#", "%23"); > > - return soup_uri_new(urlString.utf8().data()); > > - } > > - > > URL url = m_url; > > url.removeFragmentIdentifier(); > > - SoupURI* uri = soup_uri_new(url.string().utf8().data()); > > - > > - // Versions of libsoup prior to 2.42 have a soup_uri_new that will convert empty passwords that are not > > - // prefixed by a colon into null. Some parts of soup like the SoupAuthenticationManager will only be active > > - // when both the username and password are non-null. When we have credentials, empty usernames and passwords > > - // should be empty strings instead of null. > > - if (!url.user().isEmpty() || !url.pass().isEmpty()) { > > - soup_uri_set_user(uri, url.user().utf8().data()); > > - soup_uri_set_password(uri, url.pass().utf8().data()); > > - } > > - return uri; > > + return url.createSoupURI(); > > } > > Can we just get rid of this entirely in favor of url().createSoupURI()? I left this just because of the url.removeFragmentIdentifier(); I assumed that's desired only in the cases where this method is called.
Comment on attachment 222272 [details] Try to fix EFL build Okay. That all sounds reasonable.
Committed r162922: <http://trac.webkit.org/changeset/162922>
It seems that r162922 broke 9 tests in EFL: fast/css/import-style-update.html fast/events/attribute-listener-cloned-from-frameless-doc-context-2.html fast/events/attribute-listener-cloned-from-frameless-doc-context.html fast/events/attribute-listener-extracted-from-frameless-doc-context-2.html fast/events/attribute-listener-extracted-from-frameless-doc-context.html fast/html/link-rel-stylesheet.html fast/loader/data-url-encoding-html.html fast/loader/data-url-encoding-svg.html fast/spatial-navigation/snav-iframe-nested.html According to flakiness dashboard there are failures also in GTK i.e. http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss%2Fimport-style-update.html
(In reply to comment #16) > It seems that r162922 broke 9 tests in EFL: > fast/css/import-style-update.html fast/events/attribute-listener-cloned-from-frameless-doc-context-2.html fast/events/attribute-listener-cloned-from-frameless-doc-context.html fast/events/attribute-listener-extracted-from-frameless-doc-context-2.html fast/events/attribute-listener-extracted-from-frameless-doc-context.html fast/html/link-rel-stylesheet.html fast/loader/data-url-encoding-html.html fast/loader/data-url-encoding-svg.html fast/spatial-navigation/snav-iframe-nested.html > > According to flakiness dashboard there are failures also in GTK i.e. http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss%2Fimport-style-update.html Ok, I'm looking at it.
https://bugs.webkit.org/show_bug.cgi?id=127836