Bug 1450199 - Cookie is not synced across tabs r=Ehsan
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 19 Apr 2018 13:18:50 +0200
changeset 414472 5f3b781af6a705f77ea41b1f0dde160eb3968b38
parent 414471 319ba32110edb2551581a86f299c1e9cf003947b
child 414473 9003972112e3a5611f5ca5ea2c4ba32f79da38c6
push id33870
push usercsabou@mozilla.com
push dateThu, 19 Apr 2018 22:28:45 +0000
treeherdermozilla-central@5c983ad5a421 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1450199, 1425031
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1450199 - Cookie is not synced across tabs r=Ehsan This patch reverts parts of changeset e87e706def11 (bug 1425031). The problem in bug 1425031 was that when the content process set a cookie a notification was sent to the parent process. This notification was then forwarded to all the content processes, including the one it originated from. The solution was to not forward cookies that originated from a content process, but this causes the current bug. The correct fix is to forward the cookie changes to all content processes except the one they originated from. The test for bug 1425031 remains, and should keep passing. MozReview-Commit-ID: 1P6JwHQDy93
dom/ipc/ContentParent.cpp
netwerk/cookie/CookieServiceParent.cpp
netwerk/cookie/CookieServiceParent.h
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -587,17 +587,18 @@ 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",
-  "non-js-cookie-changed",
+  "cookie-changed",
+  "private-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 =
@@ -2935,29 +2936,34 @@ 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, "non-js-cookie-changed")) {
+  else if (!strcmp(aTopic, "cookie-changed") ||
+           !strcmp(aTopic, "private-cookie-changed")) {
     if (!aData) {
       return NS_ERROR_UNEXPECTED;
     }
     PNeckoParent *neckoParent = LoneManagedOrNullAsserts(ManagedPNeckoParent());
     if (!neckoParent) {
       return NS_OK;
     }
     PCookieServiceParent *csParent = LoneManagedOrNullAsserts(neckoParent->ManagedPCookieServiceParent());
     if (!csParent) {
       return NS_OK;
     }
     auto *cs = static_cast<CookieServiceParent*>(csParent);
+    // Do not push these cookie updates to the same process they originated from.
+    if (cs->ProcessingCookie()) {
+      return NS_OK;
+    }
     if (!nsCRT::strcmp(aData, u"batch-deleted")) {
       nsCOMPtr<nsIArray> cookieList = do_QueryInterface(aSubject);
       NS_ASSERTION(cookieList, "couldn't get cookie list");
       cs->RemoveBatchDeletedCookies(cookieList);
       return NS_OK;
     }
 
     if (!nsCRT::strcmp(aData, u"cleared")) {
--- a/netwerk/cookie/CookieServiceParent.cpp
+++ b/netwerk/cookie/CookieServiceParent.cpp
@@ -62,16 +62,17 @@ CookieServiceParent::CookieServiceParent
 {
   // Instantiate the cookieservice via the service manager, so it sticks around
   // until shutdown.
   nsCOMPtr<nsICookieService> cs = do_GetService(NS_COOKIESERVICE_CONTRACTID);
 
   // Get the nsCookieService instance directly, so we can call internal methods.
   mCookieService = nsCookieService::GetSingleton();
   NS_ASSERTION(mCookieService, "couldn't get nsICookieService");
+  mProcessingCookie = false;
 }
 
 CookieServiceParent::~CookieServiceParent()
 {
 }
 
 void
 GetInfoFromCookie(nsCookie         *aCookie,
@@ -265,17 +266,22 @@ CookieServiceParent::RecvSetCookieString
   // to use the channel to inspect it.
   nsCOMPtr<nsIChannel> dummyChannel;
   CreateDummyChannel(hostURI, channelURI,
                      const_cast<OriginAttributes&>(aAttrs),
                      getter_AddRefs(dummyChannel));
 
   // NB: dummyChannel could be null if something failed in CreateDummyChannel.
   nsDependentCString cookieString(aCookieString, 0);
+
+  // We set this to true while processing this cookie update, to make sure
+  // we don't send it back to the same content process.
+  mProcessingCookie = true;
   mCookieService->SetCookieStringInternal(hostURI, aIsForeign, cookieString,
-                                          aServerTime, aFromHttp, true, aAttrs,
+                                          aServerTime, aFromHttp, aAttrs,
                                           dummyChannel);
+  mProcessingCookie = false;
   return IPC_OK();
 }
 
 } // namespace net
 } // namespace mozilla
 
--- a/netwerk/cookie/CookieServiceParent.h
+++ b/netwerk/cookie/CookieServiceParent.h
@@ -27,16 +27,21 @@ public:
   void RemoveBatchDeletedCookies(nsIArray *aCookieList);
 
   void RemoveAll();
 
   void RemoveCookie(nsICookie *aCookie);
 
   void AddCookie(nsICookie *aCookie);
 
+  // This will return true if the CookieServiceParent is currently processing
+  // an update from the content process. This is used in ContentParent to make
+  // sure that we are only forwarding those cookie updates to other content
+  // processes, not the one they originated from.
+  bool ProcessingCookie() { return mProcessingCookie; }
 protected:
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   virtual mozilla::ipc::IPCResult RecvGetCookieString(const URIParams& aHost,
                                                       const bool& aIsForeign,
                                                       const bool& aIsSafeTopLevelNav,
                                                       const bool& aIsSameSiteForeign,
                                                       const OriginAttributes& aAttrs,
@@ -57,15 +62,16 @@ protected:
                                                 const OriginAttributes &aAttrs) override;
 
   void
   SerialializeCookieList(const nsTArray<nsCookie*> &aFoundCookieList,
                          nsTArray<CookieStruct> &aCookiesList,
                          nsIURI *aHostURI);
 
   RefPtr<nsCookieService> mCookieService;
+  bool mProcessingCookie;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // mozilla_net_CookieServiceParent_h
 
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -2133,27 +2133,26 @@ nsCookieService::SetCookieStringCommon(n
   OriginAttributes attrs;
   if (aChannel) {
     NS_GetOriginAttributes(aChannel, attrs);
   }
 
   nsDependentCString cookieString(aCookieHeader);
   nsDependentCString serverTime(aServerTime ? aServerTime : "");
   SetCookieStringInternal(aHostURI, isForeign, cookieString,
-                          serverTime, aFromHttp, false, attrs, aChannel);
+                          serverTime, aFromHttp, 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;
@@ -2211,17 +2210,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, aFromChild, aChannel)) {
+                           aCookieHeader, serverTime, aFromHttp, 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
@@ -2294,31 +2293,27 @@ nsCookieService::NotifyThirdParty(nsIURI
 // "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 aFromHttp,
-                               bool aFromChild)
+                               bool aFromHttp)
 {
   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) ||
@@ -3505,17 +3500,16 @@ 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,
@@ -3563,33 +3557,32 @@ 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, aFromChild);
+              aFromHttp);
   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               aFromChild)
+                             bool               aFromHttp)
 {
   MOZ_ASSERT(mInitializedDBStates);
   MOZ_ASSERT(mInitializedDBConn);
 
   int64_t currentTime = aCurrentTimeInUsec / PR_USEC_PER_SEC;
 
   nsListIter exactIter;
   bool foundCookie = false;
@@ -3697,17 +3690,17 @@ nsCookieService::AddInternal(const nsCoo
       // Remove the old cookie.
       RemoveCookieFromList(exactIter);
 
       // If the new cookie has expired -- i.e. the intent was simply to delete
       // the old cookie -- then we're done.
       if (aCookie->Expiry() <= currentTime) {
         COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
           "previously stored cookie was deleted");
-        NotifyChanged(oldCookie, u"deleted", oldCookieIsSession, aFromHttp, aFromChild);
+        NotifyChanged(oldCookie, u"deleted", oldCookieIsSession, aFromHttp);
         return;
       }
 
       // Preserve creation time of cookie for ordering purposes.
       aCookie->SetCreationTime(oldCookie->CreationTime());
     }
 
   } else {
@@ -3771,17 +3764,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, aFromHttp, aFromChild);
+  NotifyChanged(aCookie, foundCookie ? u"changed" : u"added", oldCookieIsSession, aFromHttp);
 }
 
 /******************************************************************************
  * 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
@@ -294,19 +294,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 aIsSafeTopLevelNav, bool aIsTopLevelForeign, 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, 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                          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                          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);
@@ -315,17 +315,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, bool aFromHttp = false, bool aFromChild=false);
+    void                          NotifyChanged(nsISupports *aSubject, const char16_t *aData, bool aOldCookieIsSession = false, bool aFromHttp = 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);
 
     /**