Bug 1450199 - Cookie is not synced across tabs
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
--- 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 =
@@ -2989,29 +2990,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
@@ -2132,27 +2132,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;
@@ -2210,17 +2209,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
@@ -2293,31 +2292,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) ||
@@ -3489,17 +3484,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,
@@ -3547,33 +3541,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;
@@ -3681,17 +3674,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 {
@@ -3755,17 +3748,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
@@ -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 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);
@@ -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, 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);
/**