Bug 1425031 - Don't broadcast to content processes cookie updates that initiated in content processes. r=jdm
☠☠ backed out by 8c52ad85c722 ☠ ☠
authorAmy Chung <amchung>
Fri, 12 Jan 2018 12:53:00 -0500
changeset 748682 1a64ce266ba5dcfe54d5d7d04b78cac100083b05
parent 748681 4114e1e168ea61deb48d00b89e4ad42f11ca74e3
child 748683 a117f844cf6584759e36f235c37aaeb5fd65b1bb
push id97228
push usersfraser@mozilla.com
push dateTue, 30 Jan 2018 10:21:04 +0000
reviewersjdm
bugs1425031
milestone60.0a1
Bug 1425031 - Don't broadcast to content processes cookie updates that initiated in content processes. r=jdm
dom/ipc/ContentParent.cpp
netwerk/cookie/CookieServiceParent.cpp
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
netwerk/test/mochitests/file_1331680.js
netwerk/test/mochitests/mochitest.ini
netwerk/test/mochitests/test_1425031.html
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -585,18 +585,17 @@ static const char* sObserverTopics[] = {
   "last-pb-context-exited",
   "file-watcher-update",
 #ifdef ACCESSIBILITY
   "a11y-init-or-shutdown",
 #endif
   "cacheservice:empty-cache",
   "intl:app-locales-changed",
   "intl:requested-locales-changed",
-  "cookie-changed",
-  "private-cookie-changed",
+  "non-js-cookie-changed",
 };
 
 // PreallocateProcess is called by the PreallocatedProcessManager.
 // ContentParent then takes this process back within GetNewOrUsedBrowserProcess.
 /*static*/ already_AddRefed<ContentParent>
 ContentParent::PreallocateProcess()
 {
   RefPtr<ContentParent> process =
@@ -2940,18 +2939,17 @@ ContentParent::Observe(nsISupports* aSub
     LocaleService::GetInstance()->GetAppLocalesAsLangTags(appLocales);
     Unused << SendUpdateAppLocales(appLocales);
   }
   else if (!strcmp(aTopic, "intl:requested-locales-changed")) {
     nsTArray<nsCString> requestedLocales;
     LocaleService::GetInstance()->GetRequestedLocales(requestedLocales);
     Unused << SendUpdateRequestedLocales(requestedLocales);
   }
-  else if (!strcmp(aTopic, "cookie-changed") ||
-           !strcmp(aTopic, "private-cookie-changed")) {
+  else if (!strcmp(aTopic, "non-js-cookie-changed")) {
     if (!aData) {
       return NS_ERROR_UNEXPECTED;
     }
     PNeckoParent *neckoParent = LoneManagedOrNullAsserts(ManagedPNeckoParent());
     if (!neckoParent) {
       return NS_OK;
     }
     PCookieServiceParent *csParent = LoneManagedOrNullAsserts(neckoParent->ManagedPCookieServiceParent());
--- a/netwerk/cookie/CookieServiceParent.cpp
+++ b/netwerk/cookie/CookieServiceParent.cpp
@@ -259,16 +259,16 @@ CookieServiceParent::RecvSetCookieString
   // to use the channel to inspect it.
   nsCOMPtr<nsIChannel> dummyChannel;
   CreateDummyChannel(hostURI, const_cast<OriginAttributes&>(aAttrs),
                      getter_AddRefs(dummyChannel));
 
   // NB: dummyChannel could be null if something failed in CreateDummyChannel.
   nsDependentCString cookieString(aCookieString, 0);
   mCookieService->SetCookieStringInternal(hostURI, aIsForeign, cookieString,
-                                          aServerTime, aFromHttp, aAttrs,
+                                          aServerTime, aFromHttp, true, aAttrs,
                                           dummyChannel);
   return IPC_OK();
 }
 
 } // namespace net
 } // namespace mozilla
 
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -2129,26 +2129,27 @@ nsCookieService::SetCookieStringCommon(n
   OriginAttributes attrs;
   if (aChannel) {
     NS_GetOriginAttributes(aChannel, attrs);
   }
 
   nsDependentCString cookieString(aCookieHeader);
   nsDependentCString serverTime(aServerTime ? aServerTime : "");
   SetCookieStringInternal(aHostURI, isForeign, cookieString,
-                          serverTime, aFromHttp, attrs, aChannel);
+                          serverTime, aFromHttp, false, attrs, aChannel);
   return NS_OK;
 }
 
 void
 nsCookieService::SetCookieStringInternal(nsIURI                 *aHostURI,
                                          bool                    aIsForeign,
                                          nsDependentCString     &aCookieHeader,
                                          const nsCString        &aServerTime,
                                          bool                    aFromHttp,
+                                         bool                    aFromChild,
                                          const OriginAttributes &aOriginAttrs,
                                          nsIChannel             *aChannel)
 {
   NS_ASSERTION(aHostURI, "null host!");
 
   if (!mDBState) {
     NS_WARNING("No DBState! Profile already closed?");
     return;
@@ -2206,17 +2207,17 @@ nsCookieService::SetCookieStringInternal
   default:
     break;
   }
 
   int64_t serverTime = ParseServerTime(aServerTime);
 
   // process each cookie in the header
   while (SetCookieInternal(aHostURI, key, requireHostMatch, cookieStatus,
-                           aCookieHeader, serverTime, aFromHttp, aChannel)) {
+                           aCookieHeader, serverTime, aFromHttp, aFromChild, aChannel)) {
     // document.cookie can only set one cookie at a time
     if (!aFromHttp)
       break;
   }
 }
 
 // notify observers that a cookie was rejected due to the users' prefs.
 void
@@ -2288,27 +2289,32 @@ nsCookieService::NotifyThirdParty(nsIURI
 // "added"   means a cookie was added. aSubject is the added cookie.
 // "changed" means a cookie was altered. aSubject is the new cookie.
 // "cleared" means the entire cookie list was cleared. aSubject is null.
 // "batch-deleted" means a set of cookies was purged. aSubject is the list of
 // cookies.
 void
 nsCookieService::NotifyChanged(nsISupports     *aSubject,
                                const char16_t *aData,
-                               bool aOldCookieIsSession)
+                               bool aOldCookieIsSession,
+                               bool aFromHttp,
+                               bool aFromChild)
 {
   const char* topic = mDBState == mPrivateDBState ?
       "private-cookie-changed" : "cookie-changed";
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (!os) {
     return;
   }
   // Notify for topic "private-cookie-changed" or "cookie-changed"
   os->NotifyObservers(aSubject, topic, aData);
 
+  if (!aFromChild) {
+    os->NotifyObservers(aSubject, "non-js-cookie-changed", aData);
+  }
   // Notify for topic "session-cookie-changed" to update the copy of session
   // cookies in session restore component.
   // Ignore private session cookies since they will not be restored.
   if (mDBState == mPrivateDBState) {
     return;
   }
   // Filter out notifications for individual non-session cookies.
   if (NS_LITERAL_STRING("changed").Equals(aData) ||
@@ -3449,16 +3455,17 @@ nsCookieService::CanSetCookie(nsIURI*   
 bool
 nsCookieService::SetCookieInternal(nsIURI                        *aHostURI,
                                    const mozilla::nsCookieKey    &aKey,
                                    bool                           aRequireHostMatch,
                                    CookieStatus                   aStatus,
                                    nsDependentCString            &aCookieHeader,
                                    int64_t                        aServerTime,
                                    bool                           aFromHttp,
+                                   bool                           aFromChild,
                                    nsIChannel                    *aChannel)
 {
   NS_ASSERTION(aHostURI, "null host!");
   bool canSetCookie = false;
   nsDependentCString savedCookieHeader(aCookieHeader);
   nsCookieAttributes cookieAttributes;
   bool newCookie = CanSetCookie(aHostURI, aKey, cookieAttributes, aRequireHostMatch,
                                 aStatus, aCookieHeader, aServerTime, aFromHttp,
@@ -3506,32 +3513,33 @@ nsCookieService::SetCookieInternal(nsIUR
     // update isSession and expiry attributes, in case they changed
     cookie->SetIsSession(cookieAttributes.isSession);
     cookie->SetExpiry(cookieAttributes.expiryTime);
   }
 
   // add the cookie to the list. AddInternal() takes care of logging.
   // we get the current time again here, since it may have changed during prompting
   AddInternal(aKey, cookie, PR_Now(), aHostURI, savedCookieHeader.get(),
-              aFromHttp);
+              aFromHttp, aFromChild);
   return newCookie;
 }
 
 // this is a backend function for adding a cookie to the list, via SetCookie.
 // also used in the cookie manager, for profile migration from IE.
 // it either replaces an existing cookie; or adds the cookie to the hashtable,
 // and deletes a cookie (if maximum number of cookies has been
 // reached). also performs list maintenance by removing expired cookies.
 void
 nsCookieService::AddInternal(const nsCookieKey &aKey,
                              nsCookie          *aCookie,
                              int64_t            aCurrentTimeInUsec,
                              nsIURI            *aHostURI,
                              const char        *aCookieHeader,
-                             bool               aFromHttp)
+                             bool               aFromHttp,
+                             bool               aFromChild)
 {
   MOZ_ASSERT(mInitializedDBStates);
   MOZ_ASSERT(mInitializedDBConn);
 
   int64_t currentTime = aCurrentTimeInUsec / PR_USEC_PER_SEC;
 
   nsListIter exactIter;
   bool foundCookie = false;
@@ -3713,17 +3721,17 @@ nsCookieService::AddInternal(const nsCoo
   COOKIE_LOGSUCCESS(SET_COOKIE, aHostURI, aCookieHeader, aCookie, foundCookie);
 
   // Now that list mutations are complete, notify observers. We do it here
   // because observers may themselves attempt to mutate the list.
   if (purgedList) {
     NotifyChanged(purgedList, u"batch-deleted");
   }
 
-  NotifyChanged(aCookie, foundCookie ? u"changed" : u"added", oldCookieIsSession);
+  NotifyChanged(aCookie, foundCookie ? u"changed" : u"added", oldCookieIsSession, aFromHttp, aFromChild);
 }
 
 /******************************************************************************
  * nsCookieService impl:
  * private cookie header parsing functions
  ******************************************************************************/
 
 // The following comment block elucidates the function of ParseAttributes.
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -291,19 +291,19 @@ class nsCookieService final : public nsI
     void                          RebuildCorruptDB(DBState* aDBState);
     OpenDBResult                  Read();
     mozilla::UniquePtr<ConstCookie> GetCookieFromRow(mozIStorageStatement *aRow, const OriginAttributes &aOriginAttributes);
     void                          EnsureReadComplete(bool aInitDBConn);
     nsresult                      NormalizeHost(nsCString &aHost);
     nsresult                      GetCookieStringCommon(nsIURI *aHostURI, nsIChannel *aChannel, bool aHttpBound, char** aCookie);
     void                          GetCookieStringInternal(nsIURI *aHostURI, bool aIsForeign, bool aHttpBound, const OriginAttributes& aOriginAttrs, nsCString &aCookie);
     nsresult                      SetCookieStringCommon(nsIURI *aHostURI, const char *aCookieHeader, const char *aServerTime, nsIChannel *aChannel, bool aFromHttp);
-    void                          SetCookieStringInternal(nsIURI *aHostURI, bool aIsForeign, nsDependentCString &aCookieHeader, const nsCString &aServerTime, bool aFromHttp, const OriginAttributes &aOriginAttrs, nsIChannel* aChannel);
-    bool                          SetCookieInternal(nsIURI *aHostURI, const nsCookieKey& aKey, bool aRequireHostMatch, CookieStatus aStatus, nsDependentCString &aCookieHeader, int64_t aServerTime, bool aFromHttp, nsIChannel* aChannel);
-    void                          AddInternal(const nsCookieKey& aKey, nsCookie *aCookie, int64_t aCurrentTimeInUsec, nsIURI *aHostURI, const char *aCookieHeader, bool aFromHttp);
+    void                          SetCookieStringInternal(nsIURI *aHostURI, bool aIsForeign, nsDependentCString &aCookieHeader, const nsCString &aServerTime, bool aFromHttp, bool aFromChild, const OriginAttributes &aOriginAttrs, nsIChannel* aChannel);
+    bool                          SetCookieInternal(nsIURI *aHostURI, const nsCookieKey& aKey, bool aRequireHostMatch, CookieStatus aStatus, nsDependentCString &aCookieHeader, int64_t aServerTime, bool aFromHttp, bool aFromChild, nsIChannel* aChannel);
+    void                          AddInternal(const nsCookieKey& aKey, nsCookie *aCookie, int64_t aCurrentTimeInUsec, nsIURI *aHostURI, const char *aCookieHeader, bool aFromHttp, bool aFromChild = false);
     void                          RemoveCookieFromList(const nsListIter &aIter, mozIStorageBindingParamsArray *aParamsArray = nullptr);
     void                          AddCookieToList(const nsCookieKey& aKey, nsCookie *aCookie, DBState *aDBState, mozIStorageBindingParamsArray *aParamsArray, bool aWriteToDB = true);
     void                          UpdateCookieInList(nsCookie *aCookie, int64_t aLastAccessed, mozIStorageBindingParamsArray *aParamsArray);
     static bool                   GetTokenValue(nsACString::const_char_iterator &aIter, nsACString::const_char_iterator &aEndIter, nsDependentCSubstring &aTokenString, nsDependentCSubstring &aTokenValue, bool &aEqualsFound);
     static bool                   ParseAttributes(nsDependentCString &aCookieHeader, nsCookieAttributes &aCookie);
     bool                          RequireThirdPartyCheck();
     static bool                   CheckDomain(nsCookieAttributes &aCookie, nsIURI *aHostURI, const nsCString &aBaseDomain, bool aRequireHostMatch);
     static bool                   CheckPath(nsCookieAttributes &aCookie, nsIURI *aHostURI);
@@ -312,17 +312,17 @@ class nsCookieService final : public nsI
     void                          RemoveAllFromMemory();
     already_AddRefed<nsIArray>    PurgeCookies(int64_t aCurrentTimeInUsec);
     bool                          FindCookie(const nsCookieKey& aKey, const nsCString& aHost, const nsCString& aName, const nsCString& aPath, nsListIter &aIter);
     bool                          FindSecureCookie(const nsCookieKey& aKey, nsCookie* aCookie);
     int64_t                       FindStaleCookie(nsCookieEntry *aEntry, int64_t aCurrentTime, nsIURI* aSource, const mozilla::Maybe<bool> &aIsSecure, nsListIter &aIter);
     void                          TelemetryForEvictingStaleCookie(nsCookie* aEvicted, int64_t oldestCookieTime);
     void                          NotifyRejected(nsIURI *aHostURI);
     void                          NotifyThirdParty(nsIURI *aHostURI, bool aAccepted, nsIChannel *aChannel);
-    void                          NotifyChanged(nsISupports *aSubject, const char16_t *aData, bool aOldCookieIsSession = false);
+    void                          NotifyChanged(nsISupports *aSubject, const char16_t *aData, bool aOldCookieIsSession = false, bool aFromHttp = false, bool aFromChild=false);
     void                          NotifyPurged(nsICookie2* aCookie);
     already_AddRefed<nsIArray>    CreatePurgeList(nsICookie2* aCookie);
     void                          UpdateCookieOldestTime(DBState* aDBState, nsCookie* aCookie);
 
     nsresult                      GetCookiesWithOriginAttributes(const mozilla::OriginAttributesPattern& aPattern, const nsCString& aBaseDomain, nsISimpleEnumerator **aEnumerator);
     nsresult                      RemoveCookiesWithOriginAttributes(const mozilla::OriginAttributesPattern& aPattern, const nsCString& aBaseDomain);
 
     /**
--- a/netwerk/test/mochitests/file_1331680.js
+++ b/netwerk/test/mochitests/file_1331680.js
@@ -3,16 +3,17 @@ let {utils: Cu, classes: Cc, interfaces:
 Cu.import("resource://gre/modules/Services.jsm");
 let cs = Cc["@mozilla.org/cookieService;1"].getService(Ci.nsICookieService);
 
 var observer = {
   observe: function(subject, topic, data) {
     if (topic == "cookie-changed") {
       let cookie = subject.QueryInterface(Ci.nsICookie2);
       sendAsyncMessage("cookieName", cookie.name + "=" + cookie.value);
+      sendAsyncMessage("cookieOperation", data);
     }
   }
 };
 
 addMessageListener("createObserver" , function(e) {
   Services.obs.addObserver(observer, "cookie-changed");
   sendAsyncMessage("createObserver:return");
 });
--- a/netwerk/test/mochitests/mochitest.ini
+++ b/netwerk/test/mochitests/mochitest.ini
@@ -32,9 +32,10 @@ support-files =
 [test_user_agent_updates_reset.html]
 [test_viewsource_unlinkable.html]
 [test_xhr_method_case.html]
 [test_1331680.html]
 [test_1331680_iframe.html]
 [test_1331680_xhr.html]
 [test_1396395.html]
 [test_1421324.html]
+[test_1425031.html]
 [test_origin_header.html]
new file mode 100644
--- /dev/null
+++ b/netwerk/test/mochitests/test_1425031.html
@@ -0,0 +1,67 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1425031
+-->
+<head>
+  <title>Cookies set in content processes update immediately.</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1425031">Mozilla Bug 1425031</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<script type="application/javascript">
+
+// Verify that cookie operations initiated by content processes do not cause
+// asynchronous updates for those operations to be processed later.
+
+SimpleTest.waitForExplicitFinish();
+
+var gScript = SpecialPowers.loadChromeScript(SimpleTest.getTestFileURL('file_1331680.js'));
+
+gScript.addMessageListener("cookieOperation", confirmCookieOperation);
+gScript.addMessageListener("createObserver:return", testSetCookie);
+gScript.sendAsyncMessage('createObserver');
+var testsNum = 0;
+var cookieString = "cookie0=test";
+
+// Confirm the notify which represents the cookie is updating.
+function confirmCookieOperation(op) {
+  testsNum++;
+  switch(testsNum) {
+    case 1:
+      is(op, "added", "Confirm the cookie operation is added.");
+      is(document.cookie, cookieString, "Confirm the cookie string is unaffected by the addition");
+      break;
+    case 2:
+      is(op, "deleted", "Confirm the cookie operation is deleted.");
+      is(document.cookie, cookieString, "Confirm the cookie string is unaffected by the deletion");
+      break;
+    case 3:
+      is(op, "added", "Confirm the cookie operation is added.");
+      is(document.cookie, cookieString, "Confirm the cookie string is unaffected by the second addition.");
+      document.cookie = "cookie0=; expires=Thu, 01-Jan-1970 00:00:01 GMT;";
+      gScript.sendAsyncMessage('removeObserver');
+      SimpleTest.finish();
+      break;
+  }
+}
+
+function testSetCookie() {
+  document.cookie = cookieString;
+  is(document.cookie, cookieString, "Confirm cookie string.");
+  document.cookie = "cookie0=; expires=Thu, 01-Jan-1970 00:00:01 GMT;";
+  is(document.cookie, "", "Removed all cookies.");
+  document.cookie = cookieString;
+  is(document.cookie, cookieString, "Confirm cookie string.");
+}
+
+</script>
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>