Make URLSearchParams spec-compliant
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2016 20:11:54 +0000 (20:11 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Sep 2016 20:11:54 +0000 (20:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162247

Reviewed by Chris Dumez and Sam Weinig.

LayoutTests/imported/w3c:

* web-platform-tests/url/url-constructor-expected.txt:

Source/WebCore:

Covered by newly-passing web platform tests.

* html/DOMURL.cpp:
(WebCore::DOMURL::~DOMURL):
(WebCore::DOMURL::setHref):
(WebCore::DOMURL::setQuery):
Update any associated URLSearchParams object when the query could change.
(WebCore::DOMURL::searchParams):
The lifetime of the URLSearchParams was wrong.  We were creating a new URLSearchParams each time
URL.searchParams was called, and we should have been creating one the first time and returning the
same instance for subsequent calls.  This means the DOMURL must own the URLSearchParams if it is associated,
but if it is not associated, then a URLSearchParams can live on its own.
* html/DOMURL.h:
* html/URLSearchParams.h:
(WebCore::URLSearchParams::URLDestroyed):
(WebCore::URLSearchParams::setContents):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@206168 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/url/url-constructor-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/html/DOMURL.cpp
Source/WebCore/html/DOMURL.h
Source/WebCore/html/URLSearchParams.cpp
Source/WebCore/html/URLSearchParams.h

index 78037470cdafefd96a9cac6ec359a40c7e54e2f4..da86b11d15781f84c0141cb9dcdcdefe2d6aa37b 100644 (file)
@@ -1,3 +1,12 @@
+2016-09-20  Alex Christensen  <achristensen@webkit.org>
+
+        Make URLSearchParams spec-compliant
+        https://bugs.webkit.org/show_bug.cgi?id=162247
+
+        Reviewed by Chris Dumez and Sam Weinig.
+
+        * web-platform-tests/url/url-constructor-expected.txt:
+
 2016-09-20  Alex Christensen  <achristensen@webkit.org>
 
         Non-special URLs should have an opaque origin
index 11c8f0e632a7703691ab385d182ee9dde5987c6d..38c559a67bcb83459cda9fca6be52fcc9fcf3cd8 100644 (file)
@@ -1,8 +1,8 @@
 
 PASS URL.searchParams getter 
-FAIL URL.searchParams updating, clearing assert_equals: expected "" but got "a=b"
+PASS URL.searchParams updating, clearing 
 PASS URL.searchParams setter, invalid values 
-FAIL URL.searchParams and URL.search setters, update propagation assert_equals: expected "e=f&g=h" but got "a=b&c=d"
+PASS URL.searchParams and URL.search setters, update propagation 
 PASS Loading data‚Ķ 
 PASS Parsing: <http://example  .
 org> against <http://example.org/foo/bar> 
@@ -350,7 +350,7 @@ ry#f        r
 a
 g> against <about:blank> 
 PASS Parsing: <?a=b&c=d> against <http://example.org/foo/bar> 
-FAIL Parsing: <??a=b&c=d> against <http://example.org/foo/bar> assert_equals: searchParams expected "%3Fa=b&c=d" but got "a=b&c=d"
+PASS Parsing: <??a=b&c=d> against <http://example.org/foo/bar> 
 PASS Parsing: <http:> against <http://example.org/foo/bar> 
 FAIL Parsing: <http:> against <https://example.org/foo/bar> assert_throws: function "function () {
           bURL(expected.input, expected.bas..." did not throw
index dabd5311aa1c8eb4b8187892d82bb8c44176eeb6..b2c92bf11680756d380dc92a11996dd2035ebb6b 100644 (file)
@@ -1,3 +1,27 @@
+2016-09-20  Alex Christensen  <achristensen@webkit.org>
+
+        Make URLSearchParams spec-compliant
+        https://bugs.webkit.org/show_bug.cgi?id=162247
+
+        Reviewed by Chris Dumez and Sam Weinig.
+
+        Covered by newly-passing web platform tests.
+
+        * html/DOMURL.cpp:
+        (WebCore::DOMURL::~DOMURL):
+        (WebCore::DOMURL::setHref):
+        (WebCore::DOMURL::setQuery):
+        Update any associated URLSearchParams object when the query could change.
+        (WebCore::DOMURL::searchParams):
+        The lifetime of the URLSearchParams was wrong.  We were creating a new URLSearchParams each time
+        URL.searchParams was called, and we should have been creating one the first time and returning the
+        same instance for subsequent calls.  This means the DOMURL must own the URLSearchParams if it is associated,
+        but if it is not associated, then a URLSearchParams can live on its own.
+        * html/DOMURL.h:
+        * html/URLSearchParams.h:
+        (WebCore::URLSearchParams::URLDestroyed):
+        (WebCore::URLSearchParams::setContents):
+
 2016-09-20  Antti Koivisto  <antti@apple.com>
 
         Remove AuthorStyleSheets::m_hadActiveLoadingStylesheet bit
index 4fb5e274f140093495e585624d0deeefa1f1065f..04996bf6ddc39b16fab047af9888bf2d5f4c6e27 100644 (file)
@@ -78,9 +78,17 @@ inline DOMURL::DOMURL(const String& url, ExceptionCode& ec)
         ec = TypeError;
 }
 
+DOMURL::~DOMURL()
+{
+    if (m_searchParams)
+        m_searchParams->associatedURLDestroyed();
+}
+
 void DOMURL::setHref(const String& url)
 {
     m_url = URL(m_baseURL, url);
+    if (m_searchParams)
+        m_searchParams->updateFromAssociatedURL();
 }
 
 void DOMURL::setQuery(const String& query)
@@ -113,9 +121,11 @@ String DOMURL::createPublicURL(ScriptExecutionContext& scriptExecutionContext, U
     return publicURL.string();
 }
 
-Ref<URLSearchParams> DOMURL::searchParams()
+URLSearchParams& DOMURL::searchParams()
 {
-    return URLSearchParams::create(m_url.query(), this);
+    if (!m_searchParams)
+        m_searchParams = URLSearchParams::create(search(), this);
+    return *m_searchParams;
 }
     
 void DOMURL::revokeObjectURL(ScriptExecutionContext& scriptExecutionContext, const String& urlString)
index c6d7d30938afd04e33fc16bf0451aedbe235eb57..d97bb8e821d1f2252916c524fcb4f73bfe0c803e 100644 (file)
@@ -45,19 +45,19 @@ public:
     static Ref<DOMURL> create(const String& url, const String& base, ExceptionCode&);
     static Ref<DOMURL> create(const String& url, const DOMURL& base, ExceptionCode&);
     static Ref<DOMURL> create(const String& url, ExceptionCode&);
+    ~DOMURL();
 
     URL href() const { return m_url; }
     void setHref(const String& url);
     void setHref(const String&, ExceptionCode&);
     void setQuery(const String&);
 
-    Ref<URLSearchParams> searchParams();
+    URLSearchParams& searchParams();
 
     static String createObjectURL(ScriptExecutionContext&, Blob*);
     static void revokeObjectURL(ScriptExecutionContext&, const String&);
 
     static String createPublicURL(ScriptExecutionContext&, URLRegistrable*);
-
 private:
     DOMURL(const String& url, const String& base, ExceptionCode&);
     DOMURL(const String& url, const DOMURL& base, ExceptionCode&);
@@ -65,6 +65,7 @@ private:
 
     URL m_baseURL;
     URL m_url;
+    RefPtr<URLSearchParams> m_searchParams;
 };
 
 } // namespace WebCore
index 2a55b013bdbb0e91401ffeb5c13bc5e4df7500c0..c18be063dbcc8ba4edf864cb1a568d5bb632c64b 100644 (file)
@@ -117,4 +117,11 @@ void URLSearchParams::updateURL()
         m_associatedURL->setQuery(URLParser::serialize(m_pairs));
 }
 
+void URLSearchParams::updateFromAssociatedURL()
+{
+    ASSERT(m_associatedURL);
+    String search = m_associatedURL->search();
+    m_pairs = search.startsWith('?') ? URLParser::parseURLEncodedForm(StringView(search).substring(1)) : URLParser::parseURLEncodedForm(search);
+}
+    
 }
index 5ae292fe76ef507d28e5e93f13fc78738575a572..43d6d5670f3562acef2cafd0e451c0ff327c20ad 100644 (file)
@@ -35,6 +35,7 @@ public:
     static Ref<URLSearchParams> create(const String& string, DOMURL* associatedURL = nullptr) { return adoptRef(*new URLSearchParams(string, associatedURL)); }
     static Ref<URLSearchParams> create(const Vector<std::pair<String, String>>& pairs) { return adoptRef(*new URLSearchParams(pairs)); }
 
+    void associatedURLDestroyed() { m_associatedURL = nullptr; }
     void append(const String& name, const String& value);
     void remove(const String& name);
     String get(const String& name) const;
@@ -43,13 +44,14 @@ public:
     void set(const String& name, const String& value);
     String toString();
     operator const Vector<std::pair<String, String>>&() { return m_pairs; }
+    void updateFromAssociatedURL();
 
 private:
     URLSearchParams(const String&, DOMURL*);
     explicit URLSearchParams(const Vector<std::pair<String, String>>&);
     void updateURL();
 
-    RefPtr<DOMURL> m_associatedURL;
+    DOMURL* m_associatedURL;
     Vector<std::pair<String, String>> m_pairs;
 };