Bug 786419 - Part 11 - Address possible issues with offline notifications in nsGlobalWindow and WorkerPrivate r=bent
authorValentin Gosu <valentin.gosu@gmail.com>
Wed, 27 Aug 2014 05:42:13 +0300
changeset 209178 7e1d2aaeeec1b843ba061552d6e66bd9cc19df85
parent 209177 129db8233683f0ef0c4c134317f7a5eb7b19aa60
child 209179 c00a6d8da4d9f14dba076762ef52bee0d67c6942
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbent
bugs786419
milestone35.0a1
Bug 786419 - Part 11 - Address possible issues with offline notifications in nsGlobalWindow and WorkerPrivate r=bent Part 2 of this bug adds nsGlobalWindow as an observer for the app-offline notification. There are however a few corner cases we haven't handled. For example: If the browser is offline, and an app is made offline, there should be no offline event dispatched. Also, WorkerPrivate should ignore offline events that cause no change in its offline state.
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
dom/workers/RuntimeService.cpp
dom/workers/WorkerPrivate.cpp
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -1090,17 +1090,17 @@ nsGlobalWindow::nsGlobalWindow(nsGlobalW
     mIsFrozen(false),
     mFullScreen(false),
     mIsClosed(false),
     mInClose(false),
     mHavePendingClose(false),
     mHadOriginalOpener(false),
     mIsPopupSpam(false),
     mBlockScriptedClosingFlag(false),
-    mFireOfflineStatusChangeEventOnThaw(false),
+    mWasOffline(false),
     mNotifyIdleObserversIdleOnThaw(false),
     mNotifyIdleObserversActiveOnThaw(false),
     mCreatingInnerWindow(false),
     mIsChrome(false),
     mCleanMessageManager(false),
     mNeedsFocus(true),
     mHasFocus(false),
 #if defined(XP_MACOSX)
@@ -2454,21 +2454,21 @@ nsGlobalWindow::SetNewDocument(nsIDocume
     JS_SetCompartmentPrincipals(compartment,
                                 nsJSPrincipals::get(aDocument->NodePrincipal()));
   } else {
     if (aState) {
       newInnerWindow = wsh->GetInnerWindow();
       newInnerGlobal = newInnerWindow->GetWrapperPreserveColor();
     } else {
       if (thisChrome) {
-        newInnerWindow = new nsGlobalChromeWindow(this);
+        newInnerWindow = nsGlobalChromeWindow::Create(this);
       } else if (mIsModalContentWindow) {
-        newInnerWindow = new nsGlobalModalWindow(this);
+        newInnerWindow = nsGlobalModalWindow::Create(this);
       } else {
-        newInnerWindow = new nsGlobalWindow(this);
+        newInnerWindow = nsGlobalWindow::Create(this);
       }
 
       // Freeze the outer window and null out the inner window so
       // that initializing classes on the new inner doesn't end up
       // reaching into the old inner window for classes etc.
       //
       // [This happens with Object.prototype when XPConnect creates
       // a temporary global while initializing classes; the reason
@@ -10733,22 +10733,32 @@ void
 nsGlobalWindow::GetInterface(JSContext* aCx, nsIJSID* aIID,
                              JS::MutableHandle<JS::Value> aRetval,
                              ErrorResult& aError)
 {
   dom::GetInterface(aCx, this, aIID, aRetval, aError);
 }
 
 void
-nsGlobalWindow::FireOfflineStatusEvent()
+nsGlobalWindow::FireOfflineStatusEventIfChanged()
 {
   if (!IsCurrentInnerWindow())
     return;
+
+  bool isOffline = NS_IsOffline() || NS_IsAppOffline(GetPrincipal());
+
+  // Don't fire an event if the status hasn't changed
+  if (mWasOffline == isOffline) {
+    return;
+  }
+
+  mWasOffline = isOffline;
+
   nsAutoString name;
-  if (NS_IsOffline() || NS_IsAppOffline(GetPrincipal())) {
+  if (isOffline) {
     name.AssignLiteral("offline");
   } else {
     name.AssignLiteral("online");
   }
   // The event is fired at the body element, or if there is no body element,
   // at the document.
   nsCOMPtr<EventTarget> eventTarget = mDoc.get();
   nsHTMLDocument* htmlDoc = mDoc->AsHTMLDocument();
@@ -11296,22 +11306,19 @@ nsGlobalWindow::UnregisterIdleObserver(n
 }
 
 nsresult
 nsGlobalWindow::Observe(nsISupports* aSubject, const char* aTopic,
                         const char16_t* aData)
 {
   if (!nsCRT::strcmp(aTopic, NS_IOSERVICE_OFFLINE_STATUS_TOPIC) ||
       !nsCRT::strcmp(aTopic, NS_IOSERVICE_APP_OFFLINE_STATUS_TOPIC)) {
-    if (IsFrozen()) {
-      // if an even number of notifications arrive while we're frozen,
-      // we don't need to fire.
-      mFireOfflineStatusChangeEventOnThaw = !mFireOfflineStatusChangeEventOnThaw;
-    } else {
-      FireOfflineStatusEvent();
+    if (!IsFrozen()) {
+        // Fires an offline status event if the offline status has changed
+        FireOfflineStatusEventIfChanged();
     }
     return NS_OK;
   }
 
   if (!nsCRT::strcmp(aTopic, OBSERVER_TOPIC_IDLE)) {
     mCurrentlyIdle = true;
     if (IsFrozen()) {
       // need to fire only one idle event while the window is frozen.
@@ -11576,20 +11583,18 @@ nsGlobalWindow::FireDelayedDOMEvents()
   for (uint32_t i = 0, len = mPendingStorageEvents.Length(); i < len; ++i) {
     Observe(mPendingStorageEvents[i], "dom-storage2-changed", nullptr);
   }
 
   if (mApplicationCache) {
     static_cast<nsDOMOfflineResourceList*>(mApplicationCache.get())->FirePendingEvents();
   }
 
-  if (mFireOfflineStatusChangeEventOnThaw) {
-    mFireOfflineStatusChangeEventOnThaw = false;
-    FireOfflineStatusEvent();
-  }
+  // Fires an offline status event if the offline status has changed
+  FireOfflineStatusEventIfChanged();
 
   if (mNotifyIdleObserversIdleOnThaw) {
     mNotifyIdleObserversIdleOnThaw = false;
     HandleIdleActiveEvent();
   }
 
   if (mNotifyIdleObserversActiveOnThaw) {
     mNotifyIdleObserversActiveOnThaw = false;
@@ -13357,16 +13362,24 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 // QueryInterface implementation for nsGlobalChromeWindow
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(nsGlobalChromeWindow)
   NS_INTERFACE_MAP_ENTRY(nsIDOMChromeWindow)
 NS_INTERFACE_MAP_END_INHERITING(nsGlobalWindow)
 
 NS_IMPL_ADDREF_INHERITED(nsGlobalChromeWindow, nsGlobalWindow)
 NS_IMPL_RELEASE_INHERITED(nsGlobalChromeWindow, nsGlobalWindow)
 
+/* static */ already_AddRefed<nsGlobalChromeWindow>
+nsGlobalChromeWindow::Create(nsGlobalWindow *aOuterWindow)
+{
+  nsRefPtr<nsGlobalChromeWindow> window = new nsGlobalChromeWindow(aOuterWindow);
+  window->InitWasOffline();
+  return window.forget();
+}
+
 NS_IMETHODIMP
 nsGlobalChromeWindow::GetWindowState(uint16_t* aWindowState)
 {
   *aWindowState = WindowState();
   return NS_OK;
 }
 
 uint16_t
@@ -13781,27 +13794,49 @@ nsGlobalWindow::GetDialogArguments(JSCon
   // This does an internal origin check, and returns undefined if the subject
   // does not subsumes the origin of the arguments.
   JS::Rooted<JSObject*> wrapper(aCx, GetWrapper());
   JSAutoCompartment ac(aCx, wrapper);
   mDialogArguments->Get(aCx, wrapper, nsContentUtils::SubjectPrincipal(),
                         aRetval, aError);
 }
 
+/* static */ already_AddRefed<nsGlobalModalWindow>
+nsGlobalModalWindow::Create(nsGlobalWindow *aOuterWindow)
+{
+  nsRefPtr<nsGlobalModalWindow> window = new nsGlobalModalWindow(aOuterWindow);
+  window->InitWasOffline();
+  return window.forget();
+}
+
 NS_IMETHODIMP
 nsGlobalModalWindow::GetDialogArguments(nsIVariant **aArguments)
 {
   FORWARD_TO_OUTER_MODAL_CONTENT_WINDOW(GetDialogArguments, (aArguments),
                                         NS_ERROR_NOT_INITIALIZED);
 
   // This does an internal origin check, and returns undefined if the subject
   // does not subsumes the origin of the arguments.
   return mDialogArguments->Get(nsContentUtils::SubjectPrincipal(), aArguments);
 }
 
+/* static */ already_AddRefed<nsGlobalWindow>
+nsGlobalWindow::Create(nsGlobalWindow *aOuterWindow)
+{
+  nsRefPtr<nsGlobalWindow> window = new nsGlobalWindow(aOuterWindow);
+  window->InitWasOffline();
+  return window.forget();
+}
+
+void
+nsGlobalWindow::InitWasOffline()
+{
+  mWasOffline = NS_IsOffline() || NS_IsAppOffline(GetPrincipal());
+}
+
 void
 nsGlobalWindow::GetReturnValue(JSContext* aCx,
                                JS::MutableHandle<JS::Value> aReturnValue,
                                ErrorResult& aError)
 {
   FORWARD_TO_OUTER_OR_THROW(GetReturnValue, (aCx, aReturnValue, aError),
                             aError, );
 
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -485,17 +485,17 @@ public:
   bool DoNewResolve(JSContext* aCx, JS::Handle<JSObject*> aObj,
                     JS::Handle<jsid> aId,
                     JS::MutableHandle<JSPropertyDescriptor> aDesc);
 
   void GetOwnPropertyNames(JSContext* aCx, nsTArray<nsString>& aNames,
                            mozilla::ErrorResult& aRv);
 
   // Object Management
-  explicit nsGlobalWindow(nsGlobalWindow *aOuterWindow);
+  static already_AddRefed<nsGlobalWindow> Create(nsGlobalWindow *aOuterWindow);
 
   static nsGlobalWindow *FromSupports(nsISupports *supports)
   {
     // Make sure this matches the casts we do in QueryInterface().
     return (nsGlobalWindow *)(mozilla::dom::EventTarget *)supports;
   }
   static nsGlobalWindow *FromWrapper(nsIXPConnectWrappedNative *wrapper)
   {
@@ -818,17 +818,20 @@ public:
   uint32_t Length();
   already_AddRefed<nsIDOMWindow> GetTop(mozilla::ErrorResult& aError)
   {
     nsCOMPtr<nsIDOMWindow> top;
     aError = GetScriptableTop(getter_AddRefs(top));
     return top.forget();
   }
 protected:
+  explicit nsGlobalWindow(nsGlobalWindow *aOuterWindow);
   nsIDOMWindow* GetOpenerWindow(mozilla::ErrorResult& aError);
+  // Initializes the mWasOffline member variable
+  void InitWasOffline();
 public:
   void GetOpener(JSContext* aCx, JS::MutableHandle<JS::Value> aRetval,
                  mozilla::ErrorResult& aError);
   void SetOpener(JSContext* aCx, JS::Handle<JS::Value> aOpener,
                  mozilla::ErrorResult& aError);
   using nsIDOMWindow::GetParent;
   already_AddRefed<nsIDOMWindow> GetParent(mozilla::ErrorResult& aError);
   mozilla::dom::Element* GetFrameElement(mozilla::ErrorResult& aError);
@@ -1241,17 +1244,17 @@ public:
   nsresult SecurityCheckURL(const char *aURL);
 
   bool PopupWhitelisted();
   PopupControlState RevisePopupAbuseLevel(PopupControlState);
   void     FireAbuseEvents(bool aBlocked, bool aWindow,
                            const nsAString &aPopupURL,
                            const nsAString &aPopupWindowName,
                            const nsAString &aPopupWindowFeatures);
-  void FireOfflineStatusEvent();
+  void FireOfflineStatusEventIfChanged();
 
   // Inner windows only.
   nsresult ScheduleNextIdleObserverCallback();
   uint32_t GetFuzzTimeMS();
   nsresult ScheduleActiveTimerCallback();
   uint32_t FindInsertionIndex(IdleObserverHolder* aIdleObserver);
   virtual nsresult RegisterIdleObserver(nsIIdleObserver* aIdleObserverPtr);
   nsresult FindIndexOfElementToRemove(nsIIdleObserver* aIdleObserver,
@@ -1444,18 +1447,20 @@ protected:
   // event posted.  If this is set, just ignore window.close() calls.
   bool                          mHavePendingClose : 1;
   bool                          mHadOriginalOpener : 1;
   bool                          mIsPopupSpam : 1;
 
   // Indicates whether scripts are allowed to close this window.
   bool                          mBlockScriptedClosingFlag : 1;
 
+  // Window offline status. Checked to see if we need to fire offline event
+  bool                          mWasOffline : 1;
+
   // Track what sorts of events we need to fire when thawed
-  bool                          mFireOfflineStatusChangeEventOnThaw : 1;
   bool                          mNotifyIdleObserversIdleOnThaw : 1;
   bool                          mNotifyIdleObserversActiveOnThaw : 1;
 
   // Indicates whether we're in the middle of creating an initializing
   // a new inner window object.
   bool                          mCreatingInnerWindow : 1;
 
   // Fast way to tell if this is a chrome window (without having to QI).
@@ -1652,36 +1657,38 @@ class nsGlobalChromeWindow : public nsGl
 {
 public:
   // nsISupports
   NS_DECL_ISUPPORTS_INHERITED
 
   // nsIDOMChromeWindow interface
   NS_DECL_NSIDOMCHROMEWINDOW
 
-  explicit nsGlobalChromeWindow(nsGlobalWindow *aOuterWindow)
-    : nsGlobalWindow(aOuterWindow),
-      mGroupMessageManagers(1)
-  {
-    mIsChrome = true;
-    mCleanMessageManager = true;
-  }
+  static already_AddRefed<nsGlobalChromeWindow> Create(nsGlobalWindow *aOuterWindow);
 
   static PLDHashOperator
   DisconnectGroupMessageManager(const nsAString& aKey,
                                 nsIMessageBroadcaster* aMM,
                                 void* aUserArg)
   {
     if (aMM) {
       static_cast<nsFrameMessageManager*>(aMM)->Disconnect();
     }
     return PL_DHASH_NEXT;
   }
 
 protected:
+  explicit nsGlobalChromeWindow(nsGlobalWindow *aOuterWindow)
+    : nsGlobalWindow(aOuterWindow),
+      mGroupMessageManagers(1)
+  {
+    mIsChrome = true;
+    mCleanMessageManager = true;
+  }
+
   ~nsGlobalChromeWindow()
   {
     NS_ABORT_IF_FALSE(mCleanMessageManager,
                       "chrome windows may always disconnect the msg manager");
 
     mGroupMessageManagers.EnumerateRead(DisconnectGroupMessageManager, nullptr);
     mGroupMessageManagers.Clear();
 
@@ -1719,39 +1726,41 @@ public:
  * nsGlobalModalWindow inherits from nsGlobalWindow. It is the global
  * object created for a modal content windows only (i.e. not modal
  * chrome dialogs).
  */
 class nsGlobalModalWindow : public nsGlobalWindow,
                             public nsIDOMModalContentWindow
 {
 public:
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_NSIDOMMODALCONTENTWINDOW
+
+  static already_AddRefed<nsGlobalModalWindow> Create(nsGlobalWindow *aOuterWindow);
+
+protected:
   explicit nsGlobalModalWindow(nsGlobalWindow *aOuterWindow)
     : nsGlobalWindow(aOuterWindow)
   {
     mIsModalContentWindow = true;
   }
 
-  NS_DECL_ISUPPORTS_INHERITED
-  NS_DECL_NSIDOMMODALCONTENTWINDOW
-
-protected:
   ~nsGlobalModalWindow() {}
 };
 
 /* factory function */
 inline already_AddRefed<nsGlobalWindow>
 NS_NewScriptGlobalObject(bool aIsChrome, bool aIsModalContentWindow)
 {
   nsRefPtr<nsGlobalWindow> global;
 
   if (aIsChrome) {
-    global = new nsGlobalChromeWindow(nullptr);
+    global = nsGlobalChromeWindow::Create(nullptr);
   } else if (aIsModalContentWindow) {
-    global = new nsGlobalModalWindow(nullptr);
+    global = nsGlobalModalWindow::Create(nullptr);
   } else {
-    global = new nsGlobalWindow(nullptr);
+    global = nsGlobalWindow::Create(nullptr);
   }
 
   return global.forget();
 }
 
 #endif /* nsGlobalWindow_h___ */
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -2596,36 +2596,20 @@ RuntimeService::Observe(nsISupports* aSu
     CycleCollectAllWorkers();
     return NS_OK;
   }
   if (!strcmp(aTopic, NS_IOSERVICE_OFFLINE_STATUS_TOPIC)) {
     SendOfflineStatusChangeEventToAllWorkers(NS_IsOffline());
     return NS_OK;
   }
   if (!strcmp(aTopic, NS_IOSERVICE_APP_OFFLINE_STATUS_TOPIC)) {
-    nsCOMPtr<nsIAppOfflineInfo> info(do_QueryInterface(aSubject));
-    if (!info) {
-      return NS_OK;
-    }
-    nsIPrincipal * principal = GetPrincipalForAsmJSCacheOp();
-    if (!principal) {
-      return NS_OK;
-    }
-
-    uint32_t appId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-    principal->GetAppId(&appId);
-
-    uint32_t notificationAppId = nsIScriptSecurityManager::UNKNOWN_APP_ID;
-    info->GetAppId(&notificationAppId);
-
-    if (appId != notificationAppId) {
-      return NS_OK;
-    }
-
-    SendOfflineStatusChangeEventToAllWorkers(NS_IsOffline() || NS_IsAppOffline(appId));
+    BROADCAST_ALL_WORKERS(OfflineStatusChangeEvent,
+                          NS_IsOffline() ||
+                          NS_IsAppOffline(workers[index]->GetPrincipal()));
+    return NS_OK;
   }
 
   NS_NOTREACHED("Unknown observer topic!");
   return NS_OK;
 }
 
 /* static */ void
 RuntimeService::WorkerPrefChanged(const char* aPrefName, void* aClosure)
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3023,16 +3023,21 @@ WorkerPrivateParent<Derived>::OfflineSta
   }
 }
 
 void
 WorkerPrivate::OfflineStatusChangeEventInternal(JSContext* aCx, bool aIsOffline)
 {
   AssertIsOnWorkerThread();
 
+  // The worker is already in this state. No need to dispatch an event.
+  if (mOnLine == !aIsOffline) {
+    return;
+  }
+
   for (uint32_t index = 0; index < mChildWorkers.Length(); ++index) {
     mChildWorkers[index]->OfflineStatusChangeEvent(aCx, aIsOffline);
   }
 
   mOnLine = !aIsOffline;
   WorkerGlobalScope* globalScope = GlobalScope();
   nsRefPtr<WorkerNavigator> nav = globalScope->GetExistingNavigator();
   if (nav) {