The discussion on that review suggests that some tests fail on other bots as well...maybe not clang-specific?

There is only one layout test fail and we know it's an under-invalidation and will fix it soon.

According to http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=blink_platform_unittests&tests=PaintControllerUnderInvalidationTest.MoreDrawing this bug is specific on windows clang and LTO Linux.

The test fails on official chrome-branded MSVC builds too -- no Clang necessary.

wangxianzhu: What's your plan for fixing this? It seems to me like it's time to revert that patch.

hans@ can you provide the link to the failing builds?

Please don't revert the change. This is actually the problem of the test and I will fix the test instead.

See the links in #0.

For the MSVC official build, I did that locally. I suspect we don't have Windows bots running this test in official builds, except the Clang ones.

I think the internal official builds do run tests, so you'll get a bug assigned to you the next time they cycle (they're very slow and might run only nightly for canary builds, not sure).

The CL was in between 9/2 8:59 and 9/5 1:30, and after 9/6 17:43. I would like to check the status of internal official builds, but for now the server returns 502 (or I did sth wrong to access the builds).

It seemed that during the following expected CHECK failure:
        CHECK(false) << "Can't find cached display item";
some logs from other part of the code mixed in, or the logging system didn't catch the output of the above CHECK failure.

This looks like a problem of the logging code or the gtest death testing code. However I'll create a patch to workaround it.

hans@ did you get exactly the same results as on the clang-win bots when you run official build locally?

Aha! Official builds don't include the string parameter to CHECK, this is actually a known bug type. You'll want something like https://codereview.chromium.org/1053013009/diff/20001/ppapi/shared_impl/media_stream_buffer_manager_unittest.cc (see the bug linked from that CL, and the bug blocked on that bug for several similar examples)

Thank you for helping investigate this Nico. The linked bugs seem to be about working around this issue but I'm worried the next person to use EXPECT_DEATH will hit this again in the future. Could we change EXPECT_DEATH to not have a string parameter, or add comments warning folks?

EXPECT_DEATH is in gtest, so we can't change it super easily. But if you can come up with something, that'd be cool! I'd say we hit this about once a quarter (there might be more instance I don't know about, but the clang official bots usually find official breakages first)

Would the following idea work?
1) Add a root PRESUBMIT.py check that bans the use of EXPECT_DEATH( and ASSERT_DEATH( in https://cs.chromium.org/chromium/src/PRESUBMIT.py?l=153
2) Add a new EXPECT_DEATH wrapper that drops the param in official builds, sort of like https://cs.chromium.org/chromium/src/third_party/crashpad/crashpad/test/gtest_death_check.h

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

commit 812a431578014239af1444e2967413140ab516de
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Wed Sep 07 23:17:59 2016

Fix PaintControllerUnderInvalidationTest.MoreDrawing on official builds

Official build doesn't output strings on CHECK failures.

BUG= 644768 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2319283002
Cr-Commit-Position: refs/heads/master@{#417112}

[modify] https://crrev.com/812a431578014239af1444e2967413140ab516de/third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp