Bug 1638248 - EventSource lastEventId is not consistent with spec. r=smaug
authorFarooq AR <farooqbckk@gmail.com>
Tue, 02 Jun 2020 12:05:38 +0000
changeset 597950 d6b75801e9043c8d79ca760f707a419cbf3d0e4d
parent 597949 dd65196892ffa181e0a68b78d64cf672c0d2dd44
child 597951 d0c45557821bf26012eef0f77cbcf5833c88ce55
push id13310
push userffxbld-merge
push dateMon, 29 Jun 2020 14:50:06 +0000
treeherdermozilla-beta@15a59a0afa5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1638248
milestone79.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 1638248 - EventSource lastEventId is not consistent with spec. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D75537
dom/base/EventSource.cpp
--- a/dom/base/EventSource.cpp
+++ b/dom/base/EventSource.cpp
@@ -243,17 +243,22 @@ class EventSourceImpl final : public nsI
   uint32_t mReconnectionTime;  // in ms
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsString mOrigin;
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsIHttpChannel> mHttpChannel;
 
   struct Message {
     nsString mEventName;
-    nsString mLastEventID;
+    // We need to be able to distinguish between different states of id field:
+    // 1) is not given at all
+    // 2) is given but is empty
+    // 3) is given and has a value
+    // We can't check for the 1st state with a simple nsString.
+    Maybe<nsString> mLastEventID;
     nsString mData;
   };
 
   // Message related data members. May be set / initialized when initializing
   // EventSourceImpl on target thread but should only be used on target thread.
   nsString mLastEventID;
   UniquePtr<Message> mCurrentMessage;
   nsDeque mMessagesToDispatch;
@@ -1368,22 +1373,16 @@ nsresult EventSourceImpl::DispatchCurren
   MOZ_ASSERT(message->mData.CharAt(message->mData.Length() - 1) == LF_CHAR,
              "Invalid trailing character! LF was expected instead.");
   message->mData.SetLength(message->mData.Length() - 1);
 
   if (message->mEventName.IsEmpty()) {
     message->mEventName.AssignLiteral("message");
   }
 
-  if (message->mLastEventID.IsEmpty() && !mLastEventID.IsEmpty()) {
-    message->mLastEventID.Assign(mLastEventID);
-  }
-
-  mServiceNotifier->EventReceived(message->mEventName, message->mLastEventID,
-                                  message->mData, mReconnectionTime, PR_Now());
   mMessagesToDispatch.Push(message.release());
 
   if (!mGoingToDispatchAllMessages) {
     nsCOMPtr<nsIRunnable> event =
         NewRunnableMethod("dom::EventSourceImpl::DispatchAllMessageEvents",
                           this, &EventSourceImpl::DispatchAllMessageEvents);
     NS_ENSURE_STATE(event);
 
@@ -1413,16 +1412,29 @@ void EventSourceImpl::DispatchAllMessage
     return;
   }
 
   JSContext* cx = jsapi.cx();
 
   while (mMessagesToDispatch.GetSize() > 0) {
     UniquePtr<Message> message(
         static_cast<Message*>(mMessagesToDispatch.PopFront()));
+
+    if (message->mLastEventID.isSome()) {
+      mLastEventID.Assign(message->mLastEventID.value());
+    }
+
+    if (message->mLastEventID.isNothing() && !mLastEventID.IsEmpty()) {
+      message->mLastEventID = Some(mLastEventID);
+    }
+
+    mServiceNotifier->EventReceived(message->mEventName, mLastEventID,
+                                    message->mData, mReconnectionTime,
+                                    PR_Now());
+
     // Now we can turn our string into a jsval
     JS::Rooted<JS::Value> jsData(cx);
     {
       JSString* jsString;
       jsString = JS_NewUCStringCopyN(cx, message->mData.get(),
                                      message->mData.Length());
       NS_ENSURE_TRUE_VOID(jsString);
 
@@ -1431,29 +1443,27 @@ void EventSourceImpl::DispatchAllMessage
 
     // create an event that uses the MessageEvent interface,
     // which does not bubble, is not cancelable, and has no default action
 
     RefPtr<MessageEvent> event =
         new MessageEvent(mEventSource, nullptr, nullptr);
 
     event->InitMessageEvent(nullptr, message->mEventName, CanBubble::eNo,
-                            Cancelable::eNo, jsData, mOrigin,
-                            message->mLastEventID, nullptr,
-                            Sequence<OwningNonNull<MessagePort>>());
+                            Cancelable::eNo, jsData, mOrigin, mLastEventID,
+                            nullptr, Sequence<OwningNonNull<MessagePort>>());
     event->SetTrusted(true);
 
     IgnoredErrorResult err;
     mEventSource->DispatchEvent(*event, err);
     if (err.Failed()) {
       NS_WARNING("Failed to dispatch the message event!!!");
       return;
     }
 
-    mLastEventID.Assign(message->mLastEventID);
     if (IsClosed() || IsFrozen()) {
       return;
     }
   }
 }
 
 void EventSourceImpl::ClearFields() {
   AssertIsOnTargetThread();
@@ -1490,17 +1500,17 @@ nsresult EventSourceImpl::SetFieldAndCle
     case char16_t('e'):
       if (mLastFieldName.EqualsLiteral("event")) {
         mCurrentMessage->mEventName.Assign(mLastFieldValue);
       }
       break;
 
     case char16_t('i'):
       if (mLastFieldName.EqualsLiteral("id")) {
-        mCurrentMessage->mLastEventID.Assign(mLastFieldValue);
+        mCurrentMessage->mLastEventID = Some(mLastFieldValue);
       }
       break;
 
     case char16_t('r'):
       if (mLastFieldName.EqualsLiteral("retry")) {
         uint32_t newValue = 0;
         uint32_t i = 0;  // we must ensure that there are only digits
         bool assign = true;