New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 742443 link

Starred by 1 user
Status: Fixed
Owner:
█ imch...@chromium.org
Last visit > 30 days ago
Closed: Jul 2017
Cc:
cliffordcheng@chromium.org
mfo...@chromium.org
btolsch@chromium.org
powerb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[Media Router] Almost all integration tests are failing

Project Member Reported by imch...@chromium.org, Jul 13 2017 
Error message: ../../chrome/test/media_router/media_router_integration_browsertest.cc(454): error: Value of: ConditionalWait( base::TimeDelta::FromSeconds(30), base::TimeDelta::FromSeconds(1), base::Bind(&MediaRouterIntegrationBrowserTest::IsSinkDiscoveredOnUI, base::Unretained(this)))
  Actual: false
Expected: true


Suspecting a recent change in the test utils caused the failure. Investigation so far points to the scripts we execute from C++ using content::ExecuteScript APIs don't always ensure domAutomationController.send() gets called exactly once (e.g., "if (foo) { domAutomationController.send(true); } domAutomationController.send(false);")

I am working on a fix right now.
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 14 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad05564f2cb70ddd60a54ddad3d3551aa9c875c8

commit ad05564f2cb70ddd60a54ddad3d3551aa9c875c8
Author: Derek Cheng <imcheng@chromium.org>
Date: Fri Jul 14 01:58:35 2017

[MR integration tests] Fix domAutomationController usage.

The scripts we run in the integration tests via the
content::ExecuteScript* APIs are run in a shared environment. So when a
script calls domAutomationController.send() more than once, the result
of the subsequent calls would leak to the next ExecuteScript call,
causing all sorts of weird behavior. This patch makes sure send() is
called exactly once in all code paths in those scripts.

Bug:  742443 
Change-Id: Idcf4df408d1e0d0a5f85860c47bf85e0088e881b
Reviewed-on: https://chromium-review.googlesource.com/570600
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486626}
[modify] https://crrev.com/ad05564f2cb70ddd60a54ddad3d3551aa9c875c8/chrome/test/media_router/media_router_integration_browsertest.cc
[modify] https://crrev.com/ad05564f2cb70ddd60a54ddad3d3551aa9c875c8/chrome/test/media_router/resources/common.js

Comment 2 by imch...@chromium.org, Jul 14 2017 

Everything looks good except for MediaRouterIntegrationBrowserTest.MANUAL_Dialog_Basic on Mac.

[62980:779:0714/062721.922140:FATAL:media_router_integration_browsertest.cc(344)] Check failed: content::ExecuteScriptAndExtractBool(adapter, script, &result). 
0   browser_tests                       0x000000010c94088c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   browser_tests                       0x000000010c959550 logging::LogMessage::~LogMessage() + 224
2   browser_tests                       0x000000010b2163ee media_router::MediaRouterIntegrationBrowserTest::IsDialogLoaded(content::WebContents*) + 142
3   browser_tests                       0x000000010b210eda media_router::MediaRouterBaseBrowserTest::ConditionalWait(base::TimeDelta, base::TimeDelta, base::Callback<bool (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) + 58
4   browser_tests                       0x000000010b214ca3 media_router::MediaRouterIntegrationBrowserTest::WaitUntilDialogFullyLoaded(content::WebContents*) + 115
5   browser_tests                       0x000000010b21574f media_router::MediaRouterIntegrationBrowserTest::GetRouteId(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 127
6   browser_tests                       0x000000010b215690 media_router::MediaRouterIntegrationBrowserTest::IsRouteCreatedOnUI() + 48
7   browser_tests                       0x000000010b210eda media_router::MediaRouterBaseBrowserTest::ConditionalWait(base::TimeDelta, base::TimeDelta, base::Callback<bool (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) + 58
8   browser_tests                       0x000000010b21594c media_router::MediaRouterIntegrationBrowserTest::WaitUntilRouteCreated() + 108
9   browser_tests                       0x000000010b21baa4 media_router::MediaRouterIntegrationBrowserTest_MANUAL_Dialog_Basic_Test::RunTestOnMainThread() + 500
...


This could be due to the mouse event dispatch logic specific to Mac in that test.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 14 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec1790662dbd3cb93a057757fb5e6dd52d8cbb6f

commit ec1790662dbd3cb93a057757fb5e6dd52d8cbb6f
Author: Derek Cheng <imcheng@chromium.org>
Date: Fri Jul 14 22:00:37 2017

[MR integration test] Speculative fix for Dialog_Basic test on Mac.

Remove the domAutomationController.send call in the scripts to dispatch
mouse events. Since the returned value isn't consumed by the C++ code,
we shouldn't call domAutomationController.send.

Bug:  742443 
Change-Id: Ibd54468a68a96dd324897239b6196c1896906891
Reviewed-on: https://chromium-review.googlesource.com/571530
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486902}
[modify] https://crrev.com/ec1790662dbd3cb93a057757fb5e6dd52d8cbb6f/chrome/test/media_router/media_router_integration_ui_browsertest.cc

Comment 4 by imch...@chromium.org, Jul 17 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment