Fix flakiness in postMessage-related SitePerProcessBrowserTests.

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index 72a0993ac650b43371969a814e44bbe16431bd8e..ebd71b1b1ab3ce4dbb2352f1ed6936ded68f501a 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -49,6 +49,10 @@ namespace {
 void PostMessageAndWaitForReply(FrameTreeNode* sender_ftn,
                                 const std::string& post_message_script,
                                 const std::string& reply_status) {
+  // Subtle: msg_queue needs to be declared before the ExecuteScript below.

[Charlie Reis, 2015/08/14 22:54:13]

..., or else it might miss the message of interest.

[alexmos, 2015/08/14 23:51:32]

Done.

+  // See https://crbug.com/518729.

[Charlie Reis, 2015/08/14 22:54:13]

Looks like WebUIMojoTest.ConnectToApplication might have the same bug?

[alexmos, 2015/08/14 23:51:32]

That one actually seems fine to me - it's using DOMMessageQueue after
NavigateToURL, which doesn't mess with domAutomationController messages, right?

I did check other uses of it, and I actually didn't find anything that's broken
in the same way, at least nothing obvious.  The tests I checked are fine because
they declare the queue before ExecuteScript*, if using the latter.

[Charlie Reis, 2015/08/15 00:09:50]

I'm not 100% sure, but that test doesn't do anything between creating the queue
and waiting for something.

The message it waits for comes from the page that it navigates to.  Couldn't
that queue up before the DOMMessageQueue is created, the same way we see here?

At any rate, that doesn't need to be fixed in this CL, if it is indeed a
problem.

[alexmos, 2015/08/15 00:57:24]

Ah, yes, it may be possible that the message could be sent from the page before
NavigateToURL returns, in which case it wouldn't get into the queue, since the
queue isn't created yet.  I didn't try to dig deeper into how the test is
structured (i.e., what other conditions must be true for the message to be sent)
to tell whether that's actually possible, as it gets quite involved with mojo
stuff.  I couldn't find it on the flakiness dashboard, so at least that's a good
sign.

+  content::DOMMessageQueue msg_queue;

[nasko, 2015/08/14 23:21:13]

nit: no need for content:: prefix, as it is already in content namespace.

[alexmos, 2015/08/14 23:51:32]

Done.  Thanks for noticing!

+
   bool success = false;
   EXPECT_TRUE(ExecuteScriptAndExtractBool(
       sender_ftn->current_frame_host(),
@@ -56,7 +60,6 @@ void PostMessageAndWaitForReply(FrameTreeNode* sender_ftn,
       &success));
   EXPECT_TRUE(success);
 
-  content::DOMMessageQueue msg_queue;
   std::string status;
   while (msg_queue.WaitForMessage(&status)) {
     if (status == reply_status)