mozilla-inbound: changeset 312345:6d30a609081db505484615065263515faa225ca1

author	Boris Zbarsky <bzbarsky@mit.edu>
	Thu, 01 Sep 2016 21:31:22 -0400
changeset 312345	6d30a609081db505484615065263515faa225ca1
parent 312344	657ba2f2611c714d718fb0268acfd5e60ff44310
child 312346	b7e5426f358a1ffb9ca6ac458a8537ba794ea52a
push id	81342
push user	bzbarsky@mozilla.com
push date	Fri, 02 Sep 2016 01:31:42 +0000
treeherder	mozilla-inbound@b7e5426f358a [default view] [failures only]
perfherder	[talos] [build metrics] [platform microbench] (compared to previous push)
reviewers	dolske
bugs	1167575
milestone	51.0a1
first release with	nightly linux32 d0830980ffdb / 51.0a1 / 20160903030202 / files nightly linux64 d0830980ffdb / 51.0a1 / 20160903030202 / files nightly mac d0830980ffdb / 51.0a1 / 20160903030202 / files nightly win32 d0830980ffdb / 51.0a1 / 20160903030202 / files nightly win64 d0830980ffdb / 51.0a1 / 20160903030202 / files
last release without	nightly linux32 b7f7ae14590a / 51.0a1 / 20160901030202 / files nightly linux64 b7f7ae14590a / 51.0a1 / 20160901030202 / files nightly mac b7f7ae14590a / 51.0a1 / 20160901030202 / files nightly win32 b7f7ae14590a / 51.0a1 / 20160901030202 / files nightly win64 b7f7ae14590a / 51.0a1 / 20160901030202 / files

Bug 1167575. Ensure that we don't exit modal state while we're still waiting on a Gecko runnable in the event-loop-spinning code in nsPrompter.js. r=dolske If we synchronously shut down the prompt, that will exit modal state while we're still processing our mouse or key event, which happens _before_ Gecko runnable processing. Exiting modal state will generally post a runnable to run timeouts, so we can end up processing that runnable before we return from processNextEvent and unwind to whatever code did the alert() call on the web page. The upshot is that the webpage will see timeouts fire before the alert() call returns. If we exit modal state off a Gecko runnable instead, that will ensure that we return to the nsPrompter code immediately after exiting modal state, see that the prompt is no longer active, and return to the calling web page code _before_ processing any more runnables.

--- a/browser/base/content/test/tabPrompts/browser_multiplePrompts.js
+++ b/browser/base/content/test/tabPrompts/browser_multiplePrompts.js
@@ -51,16 +51,22 @@ add_task(function*() {
       if (i !== promptsCount) {
         is(prompt.hidden, true, "This prompt should be hidden.");
         i++;
         continue;
       }
 
       is(prompt.hidden, false, "The last prompt should not be hidden.");
       prompt.onButtonClick(0);
+
+      // The click is handled async; wait for an event loop turn for that to
+      // happen.
+      yield new Promise(function(resolve) {
+        Services.tm.mainThread.dispatch(resolve, Ci.nsIThread.DISPATCH_NORMAL);
+      });
     }
   }
 
   let prompts = tab.linkedBrowser.parentNode.querySelectorAll("tabmodalprompt");
   is(prompts.length, 0, "Prompts should all be dismissed.");
 
   yield BrowserTestUtils.removeTab(tab);
 });

--- a/browser/base/content/test/tabPrompts/browser_openPromptInBackgroundTab.js
+++ b/browser/base/content/test/tabPrompts/browser_openPromptInBackgroundTab.js
@@ -37,16 +37,20 @@ add_task(function*() {
   let row = ourPrompt.querySelector("row");
   ok(row, "Should have found the row with our checkbox");
   let checkbox = row.querySelector("checkbox[label*='example.com']");
   ok(checkbox, "The checkbox should be there");
   ok(!checkbox.checked, "Checkbox shouldn't be checked");
   // tick box and accept dialog
   checkbox.checked = true;
   ourPrompt.onButtonClick(0);
+  // Wait for that click to actually be handled completely.
+  yield new Promise(function(resolve) {
+    Services.tm.mainThread.dispatch(resolve, Ci.nsIThread.DISPATCH_NORMAL);
+  });
   // check permission is set
   let ps = Services.perms;
   is(ps.ALLOW_ACTION, ps.testPermission(makeURI(pageWithAlert), "focus-tab-by-prompt"),
      "Tab switching should now be allowed");
 
   let openedTabSelectedPromise = BrowserTestUtils.waitForAttribute("selected", openedTab, "true");
   // switch to other tab again
   yield BrowserTestUtils.switchTab(gBrowser, firstTab);

--- a/toolkit/components/prompts/content/tabprompts.xml
+++ b/toolkit/components/prompts/content/tabprompts.xml
@@ -265,18 +265,31 @@
             ]]>
             </body>
         </method>
 
         <method name="onButtonClick">
             <parameter name="buttonNum"/>
             <body>
             <![CDATA[
-                this.Dialog["onButton" + buttonNum]();
-                this.shutdownPrompt();
+                // We want to do all the work her asynchronously off a Gecko
+                // runnable, because of situations like the one described in
+                // https://bugzilla.mozilla.org/show_bug.cgi?id=1167575#c35 : we
+                // get here off processing of an OS event and will also process
+                // one more Gecko runnable before we break out of the event loop
+                // spin whoever posted the prompt is doing.  If we do all our
+                // work sync, we will exit modal state _before_ processing that
+                // runnable, and if exiting moral state posts a runnable we will
+                // incorrectly process that runnable before leaving our event
+                // loop spin.
+                Services.tm.mainThread.dispatch(() => {
+                    this.Dialog["onButton" + buttonNum]();
+                    this.shutdownPrompt();
+                  },
+                  Ci.nsIThread.DISPATCH_NORMAL);
             ]]>
             </body>
         </method>
 
         <method name="onKeyAction">
             <parameter name="action"/>
             <parameter name="event"/>
             <body>

browser/base/content/test/tabPrompts/browser_multiplePrompts.js		file \| annotate \| diff \| comparison \| revisions
browser/base/content/test/tabPrompts/browser_openPromptInBackgroundTab.js		file \| annotate \| diff \| comparison \| revisions
toolkit/components/prompts/content/tabprompts.xml		file \| annotate \| diff \| comparison \| revisions