Diff
Modified: branches/safari-600.3-branch/LayoutTests/ChangeLog (175275 => 175276)
--- branches/safari-600.3-branch/LayoutTests/ChangeLog 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/LayoutTests/ChangeLog 2014-10-28 22:36:36 UTC (rev 175276)
@@ -1,5 +1,22 @@
2014-10-28 Dana Burkart <[email protected]>
+ Merge r174190. <rdar://problem/18640846>
+
+ 2014-10-01 Chris Dumez <[email protected]>
+
+ Add basic caching for Document.cookie API
+ https://bugs.webkit.org/show_bug.cgi?id=137225
+
+ Reviewed by Alexey Proskuryakov.
+
+ Add a layout test to make sure that document.cookie returns updated
+ results after cookies are set via a sync XHR.
+
+ * http/tests/cookies/sync-xhr-set-cookie-invalidates-cache-expected.txt: Added.
+ * http/tests/cookies/sync-xhr-set-cookie-invalidates-cache.html: Added.
+
+2014-10-28 Dana Burkart <[email protected]>
+
Merge r173184. <rdar://problem/18428699>
2014-09-02 Simon Fraser <[email protected]>
Modified: branches/safari-600.3-branch/LayoutTests/http/tests/cookies/resources/cookies-test-pre.js (175275 => 175276)
--- branches/safari-600.3-branch/LayoutTests/http/tests/cookies/resources/cookies-test-pre.js 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/LayoutTests/http/tests/cookies/resources/cookies-test-pre.js 2014-10-28 22:36:36 UTC (rev 175276)
@@ -23,6 +23,11 @@
}
}
+function registerCookieForCleanup(cookie)
+{
+ cookies.push(cookie);
+}
+
// Normalize a cookie string
function normalizeCookie(cookie)
{
Copied: branches/safari-600.3-branch/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache-expected.txt (from rev 174190, trunk/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache-expected.txt) (0 => 175276)
--- branches/safari-600.3-branch/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache-expected.txt (rev 0)
+++ branches/safari-600.3-branch/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache-expected.txt 2014-10-28 22:36:36 UTC (rev 175276)
@@ -0,0 +1,12 @@
+Tests that document.cookie returns the right value after a sync XHR
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS normalizeCookie(document.cookie) is "testKey=testValue"
+PASS xhr.status is 200
+PASS normalizeCookie(document.cookie) is "testKey=testValue; xhrKey=xhrValue"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Copied: branches/safari-600.3-branch/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache.html (from rev 174190, trunk/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache.html) (0 => 175276)
--- branches/safari-600.3-branch/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache.html (rev 0)
+++ branches/safari-600.3-branch/LayoutTests/http/tests/cookies/sync-xhr-set-cookie-invalidates-cache.html 2014-10-28 22:36:36 UTC (rev 175276)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<script src=''></script>
+
+<script>
+description('Tests that document.cookie returns the right value after a sync XHR');
+
+document.cookie = "testKey=testValue";
+shouldBeEqualToString('normalizeCookie(document.cookie)', 'testKey=testValue');
+var xhr = new XMLHttpRequest();
+xhr.open('GET', 'resources/setCookies.cgi', false);
+var cookie = 'xhrKey=xhrValue; path=/';
+xhr.setRequestHeader('SET-COOKIE', cookie);
+xhr.send();
+
+// This is so the cookie gets removed at the end of the test.
+registerCookieForCleanup(cookie);
+
+shouldBe('xhr.status', '200');
+shouldBeEqualToString('normalizeCookie(document.cookie)', 'testKey=testValue; xhrKey=xhrValue');
+
+</script>
+<script src=''></script>
Modified: branches/safari-600.3-branch/Source/WebCore/ChangeLog (175275 => 175276)
--- branches/safari-600.3-branch/Source/WebCore/ChangeLog 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/Source/WebCore/ChangeLog 2014-10-28 22:36:36 UTC (rev 175276)
@@ -1,5 +1,65 @@
2014-10-28 Dana Burkart <[email protected]>
+ Merge r174190. <rdar://problem/18640846>
+
+ 2014-10-01 Chris Dumez <[email protected]>
+
+ Add basic caching for Document.cookie API
+ https://bugs.webkit.org/show_bug.cgi?id=137225
+
+ Reviewed by Alexey Proskuryakov.
+
+ While profiling the load of nytimes.com, I noticed that the site is
+ accessing ~250 times document.cookie, just during page load. Accessing
+ document.cookie is currently slow because we:
+ - Call WebPlatformStrategies::cookiesForDOM() virtual function
+ - Send a sync IPC message to the Network process to retrieve the
+ cookies
+ - The Network process gets the list of cookies from CFNetwork then
+ serializes the result to send it back to the WebProcess
+ - We unserialize the cookies into an NSList of cookies
+ - We filter-out the cookies that are 'httpOnly' and construct a new
+ NSList of cookies
+ - We create a WTF String out of the cookies NSList
+
+ In the case of nytimes.com, it turns out that up to 100 calls to
+ document.cookie() are made in the same event loop iteration. This patch
+ thus caches / freezes the cookies until we return to the event
+ loop so that consecutive calls to document.cookie() are extremely fast.
+ Doing so seems to be sufficient to achieve a ~87% cache hit for
+ nytimes.com page load.
+
+ The cookies cache is invalidated whenever:
+ - document.cookie is set
+ - we return to the event loop
+ - a network resource is loaded synchronously as it may cause cookies to
+ be set before we return to the event loop
+
+ Test: http/tests/cookies/sync-xhr-set-cookie-invalidates-cache.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::Document):
+ (WebCore::Document::open):
+ (WebCore::Document::cookie):
+ (WebCore::Document::setCookie):
+ (WebCore::Document::setCookieURL):
+ (WebCore::Document::initSecurityContext):
+ (WebCore::Document::setDOMCookieCache):
+ (WebCore::Document::invalidateDOMCookieCache):
+ (WebCore::Document::domCookieCacheExpiryTimerFired):
+ (WebCore::Document::didLoadResourceSynchronously):
+ * dom/Document.h:
+ (WebCore::Document::domCookieCache):
+ (WebCore::Document::isDOMCookieCacheValid):
+ (WebCore::Document::setCookieURL): Deleted.
+ * dom/ScriptExecutionContext.cpp:
+ (WebCore::ScriptExecutionContext::didLoadResourceSynchronously):
+ * dom/ScriptExecutionContext.h:
+ * loader/ThreadableLoader.cpp:
+ (WebCore::ThreadableLoader::loadResourceSynchronously):
+
+2014-10-28 Dana Burkart <[email protected]>
+
Merge r173184. <rdar://problem/18428699>
2014-09-02 Simon Fraser <[email protected]>
Modified: branches/safari-600.3-branch/Source/WebCore/dom/Document.cpp (175275 => 175276)
--- branches/safari-600.3-branch/Source/WebCore/dom/Document.cpp 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/Source/WebCore/dom/Document.cpp 2014-10-28 22:36:36 UTC (rev 175276)
@@ -513,6 +513,7 @@
, m_inputCursor(EmptyInputCursor::create())
#endif
, m_didAssociateFormControlsTimer(this, &Document::didAssociateFormControlsTimerFired)
+ , m_cookieCacheExpiryTimer(this, &Document::domCookieCacheExpiryTimerFired)
, m_disabledFieldsetElementsCount(0)
, m_hasInjectedPlugInsScript(false)
, m_renderTreeBeingDestroyed(false)
@@ -2212,7 +2213,7 @@
{
if (ownerDocument) {
setURL(ownerDocument->url());
- m_cookieURL = ownerDocument->cookieURL();
+ setCookieURL(ownerDocument->cookieURL());
setSecurityOrigin(ownerDocument->securityOrigin());
}
@@ -3796,7 +3797,7 @@
return frame()->ownerElement();
}
-String Document::cookie(ExceptionCode& ec) const
+String Document::cookie(ExceptionCode& ec)
{
if (page() && !page()->settings().cookieEnabled())
return String();
@@ -3814,7 +3815,10 @@
if (cookieURL.isEmpty())
return String();
- return cookies(this, cookieURL);
+ if (!isDOMCookieCacheValid())
+ setCachedDOMCookies(cookies(this, cookieURL));
+
+ return cachedDOMCookies();
}
void Document::setCookie(const String& value, ExceptionCode& ec)
@@ -3835,6 +3839,7 @@
if (cookieURL.isEmpty())
return;
+ invalidateDOMCookieCache();
setCookies(this, cookieURL, value);
}
@@ -3938,6 +3943,14 @@
return String::format("%02d/%02d/%04d %02d:%02d:%02d", date.month() + 1, date.monthDay(), date.fullYear(), date.hour(), date.minute(), date.second());
}
+void Document::setCookieURL(const URL& url)
+{
+ if (m_cookieURL == url)
+ return;
+ m_cookieURL = url;
+ invalidateDOMCookieCache();
+}
+
static bool isValidNameNonASCII(const LChar* characters, unsigned length)
{
if (!isValidNameStart(characters[0]))
@@ -4645,7 +4658,7 @@
if (!m_frame) {
// No source for a security context.
// This can occur via document.implementation.createDocument().
- m_cookieURL = URL(ParsedURLString, emptyString());
+ setCookieURL(URL(ParsedURLString, emptyString()));
setSecurityOrigin(SecurityOrigin::createUnique());
setContentSecurityPolicy(std::make_unique<ContentSecurityPolicy>(this));
return;
@@ -4653,7 +4666,7 @@
// In the common case, create the security context from the currently
// loading URL with a fresh content security policy.
- m_cookieURL = m_url;
+ setCookieURL(m_url);
enforceSandboxFlags(m_frame->loader().effectiveSandboxFlags());
#if PLATFORM(IOS)
@@ -4719,7 +4732,7 @@
return;
}
- m_cookieURL = ownerFrame->document()->cookieURL();
+ setCookieURL(ownerFrame->document()->cookieURL());
// We alias the SecurityOrigins to match Firefox, see Bug 15313
// https://bugs.webkit.org/show_bug.cgi?id=15313
setSecurityOrigin(ownerFrame->document()->securityOrigin());
@@ -6137,6 +6150,32 @@
m_associatedFormControls.clear();
}
+void Document::setCachedDOMCookies(const String& cookies)
+{
+ ASSERT(!isDOMCookieCacheValid());
+ m_cachedDOMCookies = cookies;
+ // The cookie cache is valid at most until we go back to the event loop.
+ m_cookieCacheExpiryTimer.startOneShot(0);
+}
+
+void Document::invalidateDOMCookieCache()
+{
+ m_cookieCacheExpiryTimer.stop();
+ m_cachedDOMCookies = String();
+}
+
+void Document::domCookieCacheExpiryTimerFired(Timer<Document>&)
+{
+ invalidateDOMCookieCache();
+}
+
+void Document::didLoadResourceSynchronously(const ResourceRequest&)
+{
+ // Synchronous resources loading can set cookies so we invalidate the cookies cache
+ // in this case, to be safe.
+ invalidateDOMCookieCache();
+}
+
void Document::ensurePlugInsInjectedScript(DOMWrapperWorld& world)
{
if (m_hasInjectedPlugInsScript)
Modified: branches/safari-600.3-branch/Source/WebCore/dom/Document.h (175275 => 175276)
--- branches/safari-600.3-branch/Source/WebCore/dom/Document.h 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/Source/WebCore/dom/Document.h 2014-10-28 22:36:36 UTC (rev 175276)
@@ -875,7 +875,7 @@
void setTitleElement(const StringWithDirection&, Element* titleElement);
void removeTitle(Element* titleElement);
- String cookie(ExceptionCode&) const;
+ String cookie(ExceptionCode&);
void setCookie(const String&, ExceptionCode&);
String referrer() const;
@@ -898,7 +898,7 @@
// inherits its cookieURL but not its URL.
//
const URL& cookieURL() const { return m_cookieURL; }
- void setCookieURL(const URL& url) { m_cookieURL = url; }
+ void setCookieURL(const URL&);
// The firstPartyForCookies is used to compute whether this document
// appears in a "third-party" context for the purpose of third-party
@@ -1358,6 +1358,14 @@
void didAssociateFormControlsTimerFired(Timer<Document>&);
+ // DOM Cookies caching.
+ const String& cachedDOMCookies() const { return m_cachedDOMCookies; }
+ void setCachedDOMCookies(const String&);
+ bool isDOMCookieCacheValid() const { return m_cookieCacheExpiryTimer.isActive(); }
+ void invalidateDOMCookieCache();
+ void domCookieCacheExpiryTimerFired(Timer<Document>&);
+ void didLoadResourceSynchronously(const ResourceRequest&) override final;
+
unsigned m_referencingNodeCount;
std::unique_ptr<StyleResolver> m_styleResolver;
@@ -1690,6 +1698,8 @@
#endif
Timer<Document> m_didAssociateFormControlsTimer;
+ Timer<Document> m_cookieCacheExpiryTimer;
+ String m_cachedDOMCookies;
HashSet<RefPtr<Element>> m_associatedFormControls;
unsigned m_disabledFieldsetElementsCount;
Modified: branches/safari-600.3-branch/Source/WebCore/dom/ScriptExecutionContext.cpp (175275 => 175276)
--- branches/safari-600.3-branch/Source/WebCore/dom/ScriptExecutionContext.cpp 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/Source/WebCore/dom/ScriptExecutionContext.cpp 2014-10-28 22:36:36 UTC (rev 175276)
@@ -172,6 +172,10 @@
m_messagePorts.remove(&messagePort);
}
+void ScriptExecutionContext::didLoadResourceSynchronously(const ResourceRequest&)
+{
+}
+
bool ScriptExecutionContext::canSuspendActiveDOMObjects()
{
checkConsistency();
Modified: branches/safari-600.3-branch/Source/WebCore/dom/ScriptExecutionContext.h (175275 => 175276)
--- branches/safari-600.3-branch/Source/WebCore/dom/ScriptExecutionContext.h 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/Source/WebCore/dom/ScriptExecutionContext.h 2014-10-28 22:36:36 UTC (rev 175276)
@@ -30,6 +30,7 @@
#include "ActiveDOMObject.h"
#include "DOMTimer.h"
+#include "ResourceRequest.h"
#include "ScheduledAction.h"
#include "SecurityContext.h"
#include "Supplementable.h"
@@ -110,6 +111,8 @@
void createdMessagePort(MessagePort&);
void destroyedMessagePort(MessagePort&);
+ virtual void didLoadResourceSynchronously(const ResourceRequest&);
+
void ref() { refScriptExecutionContext(); }
void deref() { derefScriptExecutionContext(); }
Modified: branches/safari-600.3-branch/Source/WebCore/loader/ThreadableLoader.cpp (175275 => 175276)
--- branches/safari-600.3-branch/Source/WebCore/loader/ThreadableLoader.cpp 2014-10-28 22:15:16 UTC (rev 175275)
+++ branches/safari-600.3-branch/Source/WebCore/loader/ThreadableLoader.cpp 2014-10-28 22:36:36 UTC (rev 175276)
@@ -66,12 +66,12 @@
{
ASSERT(context);
- if (context->isWorkerGlobalScope()) {
+ if (context->isWorkerGlobalScope())
WorkerThreadableLoader::loadResourceSynchronously(toWorkerGlobalScope(context), request, client, options);
- return;
- }
- DocumentThreadableLoader::loadResourceSynchronously(*toDocument(context), request, client, options);
+ else
+ DocumentThreadableLoader::loadResourceSynchronously(*toDocument(context), request, client, options);
+ context->didLoadResourceSynchronously(request);
}
} // namespace WebCore
