Backed out changeset ad1892e1bb82

docshell/base/nsDocShell.cpp

@@ -7964,7 +7964,7 @@ nsDocShell::InternalLoad(nsIURI * aURI,
                     do_QueryInterface(mScriptGlobal);
 
                 if (window)
-                    window->DispatchSyncHashchange();
+                    window->DispatchAsyncHashchange();
             }
 
             return NS_OK;

docshell/base/nsDocShell.h

@@ -364,6 +364,12 @@ protected:
                             PRUint32 aLoadType, nscoord *cx, nscoord *cy,
                             PRBool * aDoHashchange);
 
+    // Dispatches the hashchange event to the current thread, if the document's
+    // readystate is "complete".
+    nsresult DispatchAsyncHashchange();
+
+    nsresult FireHashchange();
+
     // Returns PR_TRUE if would have called FireOnLocationChange,
     // but did not because aFireOnLocationChange was false on entry.
     // In this case it is the caller's responsibility to ensure

docshell/test/test_bug385434.html

@@ -25,7 +25,6 @@ SimpleTest.waitForExplicitFinish();
 
 var gNumHashchanges = 0;
 var gCallbackOnIframeLoad = false;
-var gCallbackOnHashchange = false;
 
 function statusMsg(msg) {
   var msgElem = document.createElement("p");
@@ -43,14 +42,7 @@ function longWait() {
 // event which was fired.
 function onIframeHashchange() {
   gNumHashchanges++;
-  if (gCallbackOnHashchange) {
-    gCallbackOnHashchange = false;
-    gGen.next();
-  }
-}
-
-function enableHashchangeCallback() {
-  gCallbackOnHashchange = true;
+  gGen.next();
 }
 
 function onIframeLoad() {
@@ -121,35 +113,36 @@ function run_test() {
 
   noEventExpected("No hashchange expected initially.");
 
-  enableHashchangeCallback();
   sendMouseEvent({type: "click"}, "link1", frameCw);
   yield;
   eventExpected("Clicking link1 should trigger a hashchange.");
 
-  enableHashchangeCallback();
   sendMouseEvent({type: "click"}, "link1", frameCw);
   longWait();
   yield;
   // succeed if a hashchange event wasn't triggered while we were waiting
   noEventExpected("Clicking link1 again should not trigger a hashchange.");
 
-  enableHashchangeCallback();
   sendMouseEvent({type: "click"}, "link2", frameCw);
   yield;
   eventExpected("Clicking link2 should trigger a hashchange.");
 
   frameCw.history.go(-1);
+  yield;
   eventExpected("Going back should trigger a hashchange.");
 
   frameCw.history.go(1);
+  yield;
   eventExpected("Going forward should trigger a hashchange.");
 
   frameCw.window.location.hash = "";
+  yield;
   eventExpected("Changing window.location.hash should trigger a hashchange.");
 
   // window.location has a trailing '#' right now, so we append "link1", not
   // "#link1".
   frameCw.window.location = frameCw.window.location + "link1";
+  yield;
   eventExpected("Assigning to window.location should trigger a hashchange.");
 
   // Set up history in the iframe which looks like:
@@ -184,8 +177,9 @@ function run_test() {
   yield;
 
   frameCw.document.location = "file_bug385434_2.html#foo";
-  eventExpected("frame onhashchange should fire events.");
+  yield;
 
+  eventExpected("frame onhashchange should fire events.");
   // iframe should set gSampleEvent
   is(gSampleEvent.target, frameCw,
      "The hashchange event should be targeted to the window.");
@@ -201,7 +195,6 @@ function run_test() {
    *     hashchange is dispatched if the current document readyState is
    *     not "complete" (bug 504837).
    */
-  enableHashchangeCallback();
   frameCw.document.location = "file_bug385434_3.html";
   yield;
   eventExpected("Hashchange should fire even if the document " +

dom/base/nsGlobalWindow.cpp

@@ -6932,11 +6932,20 @@ nsGlobalWindow::PageHidden()
 }
 
 nsresult
-nsGlobalWindow::DispatchSyncHashchange()
+nsGlobalWindow::DispatchAsyncHashchange()
 {
-  FORWARD_TO_INNER(DispatchSyncHashchange, (), NS_OK);
-  NS_ASSERTION(nsContentUtils::IsSafeToRunScript(),
-               "Must be safe to run script here.");
+  FORWARD_TO_INNER(DispatchAsyncHashchange, (), NS_OK);
+
+  nsCOMPtr<nsIRunnable> event =
+    NS_NEW_RUNNABLE_METHOD(nsGlobalWindow, this, FireHashchange);
+   
+  return NS_DispatchToCurrentThread(event);
+}
+
+nsresult
+nsGlobalWindow::FireHashchange()
+{
+  NS_ENSURE_TRUE(IsInnerWindow(), NS_ERROR_FAILURE);
 
   // Don't do anything if the window is frozen.
   if (IsFrozen())

dom/base/nsGlobalWindow.h

@@ -443,7 +443,7 @@ public:
   virtual PRBool TakeFocus(PRBool aFocus, PRUint32 aFocusMethod);
   virtual void SetReadyForFocus();
   virtual void PageHidden();
-  virtual nsresult DispatchSyncHashchange();
+  virtual nsresult DispatchAsyncHashchange();
   virtual nsresult SetArguments(nsIArray *aArguments, nsIPrincipal *aOrigin);
 
   static PRBool DOMWindowDumpEnabled();
@@ -574,7 +574,8 @@ protected:
                            const nsAString &aPopupWindowName,
                            const nsAString &aPopupWindowFeatures);
   void FireOfflineStatusEvent();
-
+  nsresult FireHashchange();
+  
   void FlushPendingNotifications(mozFlushType aType);
   void EnsureReflowFlushAndPaint();
   nsresult CheckSecurityWidthAndHeight(PRInt32* width, PRInt32* height);

dom/base/nsPIDOMWindow.h

@@ -454,9 +454,10 @@ public:
   virtual void PageHidden() = 0;
 
   /**
-   * Instructs this window to synchronously dispatch a hashchange event.
+   * Instructs this window to asynchronously dispatch a hashchange event.  This
+   * method must be called on an inner window.
    */
-  virtual nsresult DispatchSyncHashchange() = 0;
+  virtual nsresult DispatchAsyncHashchange() = 0;
 
 
   /**

dom/tests/mochitest/general/test_bug504220.html

@@ -23,8 +23,6 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=504220
 /** Test for Bug 504220 **/
 
 function run_test() {
-  SimpleTest.waitForExplicitFinish();
-
   ok("onhashchange" in document.body,
      "document.body should contain 'onhashchange'.");
 
@@ -37,20 +35,30 @@ function run_test() {
 
   // Likewise, document.body.hashchange should mirror window.onhashchange
   numEvents = 0;
-  var func2 = function() { numEvents++; };
+  var func2 = function() { numEvents++; gGen.next() };
   window.onhashchange = func2;
   is(document.body.onhashchange, func2);
 
-  // Change the document's hash.  If we've been running this test manually,
-  // the hash might already be "#foo", so we need to check in order to be
-  // sure we trigger a hashchange.
-  if (location.hash != "#foo")
-    location.hash = "#foo";
-  else
-    location.hash = "#bar";
+  SimpleTest.waitForExplicitFinish();
 
-  is(numEvents, 1, "Exactly one hashchange should have been fired.");
-  SimpleTest.finish();
+  function waitForHashchange() {
+    // Change the document's hash.  If we've been running this test manually,
+    // the hash might already be "#foo", so we need to check in order to be
+    // sure we trigger a hashchange.
+    if (location.hash != "#foo")
+      location.hash = "#foo";
+    else
+      location.hash = "#bar";
+
+    yield;
+
+    is(numEvents, 1, "Exactly one hashchange should have been fired.");
+    SimpleTest.finish();
+    yield;
+  }
+
+  var gGen = waitForHashchange();
+  gGen.next();
 }
 
 </script>