diff options
| author | alexmos <alexmos@chromium.org> | 2015-08-14 19:54:36 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2015-08-15 02:55:14 +0000 |
| commit | 17e5753ea2e213295dc6a959be2ff286082faacd (patch) | |
| tree | 8eaa7bb606f5e79c620c1ef1cd2b2740c27ecd0d | |
| parent | 780b193b7e92cef04c6c1063068e6f5edbf8e3f2 (diff) | |
| download | chromium_src-17e5753ea2e213295dc6a959be2ff286082faacd.zip chromium_src-17e5753ea2e213295dc6a959be2ff286082faacd.tar.gz chromium_src-17e5753ea2e213295dc6a959be2ff286082faacd.tar.bz2 | |
Fix flakiness in postMessage-related SitePerProcessBrowserTests.
See https://crbug.com/518729 for the complete description.
This CL fixes two underlying issues for flakiness in
SitePerProcessBrowserTest.PostMessageWithSubframeOnOpenerChain:
1. ExecuteScript* wasn't properly dealing with
domAutomationController messages that might have arrived right after
the expected response to ExecuteScript*, but before its message loop
shut down.
2. DOMMessageQueue was instantiated after the ExecuteScript* in the
test, causing it to never see the message it was waiting for if it
happened to arrive on ExecuteScript's message loop above.
BUG=518729
Review URL: https://codereview.chromium.org/1297713004
Cr-Commit-Position: refs/heads/master@{#343548}
| -rw-r--r-- | content/browser/site_per_process_browsertest.cc | 15 | ||||
| -rw-r--r-- | content/public/test/browser_test_utils.cc | 8 |
2 files changed, 10 insertions, 13 deletions
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc index 9cbeceb..2464a38 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, or + // else it might miss the message of interest. See https://crbug.com/518729. + DOMMessageQueue msg_queue; + 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) @@ -2479,16 +2482,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, SubframePostMessage) { // Check that postMessage can be sent from a subframe on a cross-process opener // tab, and that its event.source points to a valid proxy. -// Very flaky on windows (crbug.com/518729). -#if defined(OS_WIN) -#define MAYBE_PostMessageWithSubframeOnOpenerChain \ - DISABLED_PostMessageWithSubframeOnOpenerChain -#else -#define MAYBE_PostMessageWithSubframeOnOpenerChain \ - PostMessageWithSubframeOnOpenerChain -#endif // defined(OS_WIN) IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, - MAYBE_PostMessageWithSubframeOnOpenerChain) { + PostMessageWithSubframeOnOpenerChain) { GURL main_url(embedded_test_server()->GetURL( "a.com", "/frame_tree/page_with_post_message_frames.html")); EXPECT_TRUE(NavigateToURL(shell(), main_url)); diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc index 6042396..214b2fd 100644 --- a/content/public/test/browser_test_utils.cc +++ b/content/public/test/browser_test_utils.cc @@ -75,9 +75,11 @@ class DOMOperationObserver : public NotificationObserver, const NotificationDetails& details) override { DCHECK(type == NOTIFICATION_DOM_OPERATION_RESPONSE); Details<DomOperationNotificationDetails> dom_op_details(details); - response_ = dom_op_details->json; - did_respond_ = true; - message_loop_runner_->Quit(); + if (!did_respond_) { + response_ = dom_op_details->json; + did_respond_ = true; + message_loop_runner_->Quit(); + } } // Overridden from WebContentsObserver: |