After discussion with other sheriffs and on the #chromium channel, we've decided to disable all those tests.

My git checkout got hosed sometime today.  This is taking longer than I expected.

CC relevant folks

Change pending in CQ:
https://codereview.chromium.org/2184503002/

(didn't want to disable the CQ in case there was a compile error in my change.  My browser_tests build was taking forever--it would be faster to CQ it.)

Looking at https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/50963/steps/browser_tests%20on%20Windows-7-SP1/logs/stdio, it seems like more than just AppBanner tests are failing on that bot. There are several other tests that fail with these error messages:

[3616:5904:0725/101014:ERROR:singleton_hwnd.cc(34)] Cannot create windows on non-UI thread!
 ...
Assertion failed: ptr_ != NULL, file c:\b\c\b\win\src\base\memory\ref_counted.h, line 322

Those two appear hundreds on times in that log, but buildbot only seems to be picking out the app banner tests to blame.

Also, these tests haven't been touched in a long time, and the underlying app banner code doesn't have any recent changes. I'd be very surprised if this was specific to app banners and not due to some other change. I wouldn't disable these tests as they're probably indicating that something else went bad.

Two sheriffs and two separate people from the IRC channel looked at this and all couldn't figure out a better answer.  The bot is red and this redness may hide other issues.  It needs to be fixed.  If you can find something to revert, go ahead.  The only vaguely remote possibility from the blamelist in 
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/50963
is change 7837.

If you and your knowledge of the test can figure out a rational connecting it to these failures, go ahead and revert it.

Otherwise, the disabling should land.  Then the problem is localized and can fix without causing trouble on the tree.

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

commit 3429dff80be28f18b66b7d93718d131dc4aea9de
Author: mpearson <mpearson@chromium.org>
Date: Tue Jul 26 00:40:36 2016

Disable Many AppBannerDataFetcherBrowserTest Failing Tests

TBR=dominickn@chromium.org

BUG= 631190 

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

[modify] https://crrev.com/3429dff80be28f18b66b7d93718d131dc4aea9de/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc

I missed a few tests in my first round.  Follow-up round is in the CQ:
https://codereview.chromium.org/2183743002/

Once the bot turns green, we sheriffs should assign this bug to dominickn@ and remove the Sheriff-Chromium label.

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

commit 8945bfac334ff9b1ec226e158a1f29e3fdce5e0c
Author: mpearson <mpearson@chromium.org>
Date: Tue Jul 26 04:36:46 2016

Disable A Few More AppBannerDataFetcherBrowserTest Failing Tests

I didn't see these failures in the original lists I checked.

TBR=dominickn@chromium.org

BUG= 631190 

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

[modify] https://crrev.com/8945bfac334ff9b1ec226e158a1f29e3fdce5e0c/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc

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

commit 6cfda680d992933c594d0866eb2183600d98a2ed
Author: dominickn <dominickn@chromium.org>
Date: Tue Jul 26 05:32:01 2016

Revert "Speculatively launch Service Workers on mouse/touch events. [3/5]"

This reverts commit 2cba6671596841c77457423e70e9d12070425dd1.

Reason: Speculative revert so that crrev.com/2181553003 can be reverted.
This may have caused  crbug.com/631190 .

This CL will be relanded if reverting the dependent CL and re-enabling
the disabled app banner data fetcher tests still causes failures on
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29

TBR=horo@chromium.org,jochen@chromium.org,nhiroki@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 631190 

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

[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/chrome/browser/renderer_host/chrome_render_message_filter.cc
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/chrome/browser/renderer_host/chrome_render_message_filter.h
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/content/browser/DEPS
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/content/browser/service_worker/service_worker_metrics.cc
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/content/browser/service_worker/service_worker_metrics.h
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/content/public/browser/service_worker_context.h
[modify] https://crrev.com/6cfda680d992933c594d0866eb2183600d98a2ed/tools/metrics/histograms/histograms.xml

#6: change 7837 looks very relevant, as it makes changes to methods which the app banner data fetcher test is going to call through to check if a service worker exists on a page. Since the test is timing out, it's well possible that that change is causing the async service worker check to never call back.

I'm in the process of reverting that change (and a dependent change) as well as the test disabling to see if that greens up the bot.

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

commit 178c5ea9779ea4eaab30b913548542bf2f096794
Author: dominickn <dominickn@chromium.org>
Date: Tue Jul 26 05:46:36 2016

Revert of Disable Many AppBannerDataFetcherBrowserTest Failing Tests (patchset #3 id:40001 of https://codereview.chromium.org/2184503002/ )

Reason for revert:
Speculative revert to see if reverting crrev.com/2183783002 fixes the failing tests in  crbug.com/631190 .

App banners use methods touched by that CL to check if a service worker is registered.

This CL will be relanded if the re-enabled tests still fail on https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29

Original issue's description:
> Disable Many AppBannerDataFetcherBrowserTest Failing Tests
>
> TBR=dominickn@chromium.org
>
> BUG= 631190 
>
> Committed: https://crrev.com/3429dff80be28f18b66b7d93718d131dc4aea9de
> Cr-Commit-Position: refs/heads/master@{#407653}

TBR=mpearson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 631190 

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

[modify] https://crrev.com/178c5ea9779ea4eaab30b913548542bf2f096794/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc

Sorry, my change caused these failures.
https://codereview.chromium.org/2181553003/

In ServiceWorkerContextCore::DidFindRegistrationForCheckHasServiceWorker():

    registration->RegisterRegistrationFinishedCallback(
        base::Bind(&ServiceWorkerContextCore::
                       OnRegistrationFinishedForCheckHasServiceWorker,
                   AsWeakPtr(), callback, std::move(registration)));

We can't call std::move() here.

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

commit 3d1d8ade55ffbe65c1b4dfe399b8ef42a196c377
Author: dominickn <dominickn@chromium.org>
Date: Tue Jul 26 09:16:18 2016

Revert of Disable A Few More AppBannerDataFetcherBrowserTest Failing Tests (patchset #1 id:1 of https://codereview.chromium.org/2183743002/ )

Reason for revert:
These test failures were caused by a bug in https://codereview.chromium.org/2181553003/  Re-enabling.

Original issue's description:
> Disable A Few More AppBannerDataFetcherBrowserTest Failing Tests
>
> I didn't see these failures in the original lists I checked.
>
> TBR=dominickn@chromium.org
>
> BUG= 631190 
>
> Committed: https://crrev.com/8945bfac334ff9b1ec226e158a1f29e3fdce5e0c
> Cr-Commit-Position: refs/heads/master@{#407707}

TBR=mpearson@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 631190 

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

[modify] https://crrev.com/3d1d8ade55ffbe65c1b4dfe399b8ef42a196c377/chrome/browser/banners/app_banner_data_fetcher_browsertest.cc

dominickn@: thanks for putting things in a good state again.  Can we close this bug?

Also, separately, can you figure out why horo@'s original change passed the CQ?  Are AppBannerDataFetcherBrowserTests not being run by the CQ?

Dropping Sheriff-Chromium as this is currently handed off from the sheriff.

AppBannerDataFetcherBrowserTests are (except for a few hours yesterday) enabled everywhere that browser tests run, and have been stable for months.

All the failing tests also had a NULL ptr error in base/memory/ref_counted.h right under the error quoted in the initial description. That's very suggestive that some std::move call caused a DecRef operation which made the SW registration object die before it was used again. I'm guessing when it passed the CQ, it didn't run on the exact buildbot combination / flags / OS / whatever and didn't explode. But really, I have no concrete idea.