Fix flakiness in postMessage-related SitePerProcessBrowserTests.

content/browser/site_per_process_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/site_per_process_browsertest.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 31 matching lines @@
 
 namespace {
 
 // Helper function to send a postMessage and wait for a reply message.  The
 // |post_message_script| is executed on the |sender_ftn| frame, and the sender
 // frame is expected to post |reply_status| from the DOMAutomationController
 // when it receives a reply.
 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(),
       "window.domAutomationController.send(" + post_message_script + ");",
       &success));
   EXPECT_TRUE(success);
 
-  content::DOMMessageQueue msg_queue;
   std::string status;
   while (msg_queue.WaitForMessage(&status)) {
     if (status == reply_status)
       break;
   }
 }
 
 // Helper function to extract and return "window.receivedMessages" from the
 // |sender_ftn| frame.  This variable is used in post_message.html to count the
 // number of messages received via postMessage by the current window.
@@ skipping 2832 matching lines @@
       foo_root->render_manager()->GetRenderFrameProxyHost(site_instance_a));
 
   // Verify that the a.com popup can now find the "foo" popup by name.
   GURL named_frame_url(embedded_test_server()->GetURL("c.com", "/title2.html"));
   NavigateNamedFrame(a_com_shell->web_contents(), named_frame_url, "foo");
   EXPECT_TRUE(WaitForLoadStop(foo_shell->web_contents()));
   EXPECT_EQ(named_frame_url, foo_root->current_url());
 }
 
 }  // namespace content