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

Issue 602494 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

NativeDesktopMediaListTest failing on Linux Valgrind bots

Project Member Reported by thestig@chromium.org, Apr 11 2016

Issue description

Maybe related to r386129? Maybe a timing issue?

----
https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%283%29/builds/50556/

[ RUN      ] NativeDesktopMediaListTest.UpdateThumbnail
Xlib:  extension "RANDR" missing on display ":9".

GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: OnSourceRemoved(0x1cd9a540, 2)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See http://code.google.com/p/googlemock/wiki/CookBook#Knowing_When_to_Expect for details.

GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: OnSourceAdded(0x1cd9a540, 2)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See http://code.google.com/p/googlemock/wiki/CookBook#Knowing_When_to_Expect for details.
unknown file: Failure

Unexpected mock function call - returning directly.
    Function call: OnSourceThumbnailChanged(0x1cd9a540, 2)
Google Mock tried the following 1 expectation, but it didn't match:

../../chrome/browser/media/native_desktop_media_list_unittest.cc:478: EXPECT_CALL(observer_, OnSourceThumbnailChanged(model_.get(), 1))...
  Expected arg #1: is equal to 1
           Actual: 2
         Expected: to be called once
           Actual: called once - saturated and active
[  FAILED  ] NativeDesktopMediaListTest.UpdateThumbnail (1877 ms)
----

----
https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%284%29/builds/48397/

[ RUN      ] NativeDesktopMediaListTest.AddNativeWindow
Xlib:  extension "RANDR" missing on display ":9".

GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: OnSourceThumbnailChanged(0x19932bf0, 3)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See http://code.google.com/p/googlemock/wiki/CookBook#Knowing_When_to_Expect for details.

GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: OnSourceRemoved(0x19932bf0, 2)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See http://code.google.com/p/googlemock/wiki/CookBook#Knowing_When_to_Expect for details.
unknown file: Failure

Unexpected mock function call - returning directly.
    Function call: OnSourceAdded(0x19932bf0, 2)
Google Mock tried the following 1 expectation, but it didn't match:

../../chrome/browser/media/native_desktop_media_list_unittest.cc:377: EXPECT_CALL(observer_, OnSourceAdded(model_.get(), index))...
  Expected arg #1: is equal to 3
           Actual: 2
         Expected: to be called once
           Actual: called once - saturated and active
[  FAILED  ] NativeDesktopMediaListTest.AddNativeWindow (1249 ms)
[ RUN      ] NativeDesktopMediaListTest.RemoveNativeWindow
Xlib:  extension "RANDR" missing on display ":9".
../../chrome/browser/media/native_desktop_media_list_unittest.cc:416: Failure
Mock function called more times than expected - returning directly.
    Function call: OnSourceRemoved(0x1eda8f50, 1)
         Expected: to be called once
           Actual: called twice - over-saturated and active

GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: OnSourceAdded(0x1eda8f50, 1)
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See http://code.google.com/p/googlemock/wiki/CookBook#Knowing_When_to_Expect for details.
[  FAILED  ] NativeDesktopMediaListTest.RemoveNativeWindow (1531 ms)

----
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 12 2016

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

commit abc448256e3cdf70fa47cb921536d3e39d5055c0
Author: thestig <thestig@chromium.org>
Date: Tue Apr 12 00:14:19 2016

Valgrind: Address redness on the memory.fyi bots.

1) Update a suppression
2) Disable failing ash_unittests test cases.
3) Disable failing NativeDesktopMediaListTests.

TBR=glider@chromium.org
BUG=562431, 602487 , 602494 

Review URL: https://codereview.chromium.org/1877113002

Cr-Commit-Position: refs/heads/master@{#386529}

[modify] https://crrev.com/abc448256e3cdf70fa47cb921536d3e39d5055c0/tools/valgrind/gtest_exclude/ash_unittests.gtest-memcheck.txt
[modify] https://crrev.com/abc448256e3cdf70fa47cb921536d3e39d5055c0/tools/valgrind/gtest_exclude/unit_tests.gtest_linux.txt
[modify] https://crrev.com/abc448256e3cdf70fa47cb921536d3e39d5055c0/tools/valgrind/memcheck/suppressions.txt

Comment 2 by gyzhou@chromium.org, Apr 18 2016

There are two issues associated with this bug. 

First the message_loop is supposed to quit after each native desktop media list refresh. However, it often quits after a few refreshes. 

Second, in NativeDesktopMediaList, aura::Window* DesktopWindowTreeHostX11::GetContentWindowForXID(XID xid) is used to get the aura window based on its native window handle in linux oS. This function is not reliable for at least test aura windows created for this unit test. Typically, after several refreshes, and the function will incorrectly return a null aura window. The message loop typically will quit after this.

The failed test case senario can be explained like this. When AddWindowsAndVerify() is called, the first fresh is going to be all right and all associated verification will pass. There are going to be more freshes until DesktopWindowTreeHostX11::GetContentWindowForXID() get an incorrect null aura window and aura_id=0 for this window . Then TEST_F(NativeDesktopMediaListTest, *) runs the message loop again, this time DesktopWindowTreeHostX11::GetContentWindowForXID() gets the correct aura window and aura_id>0. However DesktopMediaList will think this window is not the same as the one in the previous refresh since their aura_ids are different though they are actually the exact same windows with identical native ids. This leads to OnSourceRemoved(0x1cd9a540, index) and OnSourceRemoved(0x1cd9a540, index) and OnSourceThumbnailChanged(0x1cd9a540, index). Those extra call backs mess up the unit test.

https://bugs.chromium.org/p/chromium/issues/detail?id=603823 should have similar issues of this bug.

Comment 3 by gyzhou@chromium.org, Apr 18 2016

The issue of quit message loop itself will not break the unit tests through it will make the unit tests take longer time. I have seen this issue in try-bolt other than valgrind. Typically, the issue in other try-bolt can be solved by adding a little delay between two adjacent refresh() such as 
model_->SetUpdatePeriod(base::TimeDelta::FromMilliseconds(5));

However, in valgrind, this issue is much worse, 5 millisecond delay will lead to TEST_F(NativeDesktopMediaListTest, WindowsOnly) has refresh for 3-5 times. While 50 millisecond will lead to 2-3 times of refresh().

A temporarily work around for broken unit tests is to disable aura windows capture test in linux. It can avoid DesktopWindowTreeHostX11::GetContentWindowForXID() to be called.

 

Comment 5 by gyzhou@chromium.org, Apr 30 2016

Status: Fixed (was: Untriaged)

Sign in to add a comment