New issue
Advanced search Search tips

Issue 668707 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 670844

Blocking:
issue 824431



Sign in to add a comment

Use base::RunLoop instead of content::MessageLoopRunner, use deferred quit only when needed

Project Member Reported by ah...@yandex-team.ru, Nov 25 2016

Issue description

Discussion: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/pr0Zv5v83rA

There are problems with MessageLoopRunner.
First, its Quit uses DeferredQuitRunLoop, which results in test flakiness and suboptimal performance.
Second, it does not add much to base::RunLoop other than this deferred quit behavior.

Solution: gradually replace MessageLoopRunner with RunLoop, examining and fixing any test failures. DeferredQuitRunLoop uses should be sparse and justified.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 28 2016

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

commit ba2d61725438457920b12f562894b7aa7eb1fea1
Author: ahest <ahest@yandex-team.ru>
Date: Mon Nov 28 13:06:53 2016

NavigationControllerBrowserTest.RaceCrossOriginNavigationAndSamePageHistoryNavigation:
Since cross_origin_commit_observer.Wait() does not really stop right
after commit, nothing prevents the history nav to commit too during this
wait.

SitePerProcessBrowserTest.NavigateInUnloadHandler:
Navigation to c.com could commit while ExecuteScript was spinning the
loop, then deleted_observer binds to C's RFH and waits forever.

SitePerProcessBrowserTest.RenderViewHostIsNotReusedAfterDelayedSwapOutACK:
During handling of navigation commit in the browser, SwapOut message is
sent to the renderer, which asks its host to shut down;
commit_observer.WaitForCommit may spin the loop until this request is
satisfied, and after return the test accesses dead objects.

Regarding the change in DOMMessageQueue::WaitForMessage:
It was possible for NavigateInUnloadHandler to fail because of this.
With reordering code in the test itself, this change is not really
necessary, but to me it seems right to keep it - this is not the place
where the loop should be spinned always, unconditionally. BTW, I'm
planning to discuss (on chromium-dev) a broader proposal to change
things like this.

BUG=668707

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

[modify] https://crrev.com/ba2d61725438457920b12f562894b7aa7eb1fea1/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/ba2d61725438457920b12f562894b7aa7eb1fea1/content/public/test/test_frame_navigation_observer.cc
[modify] https://crrev.com/ba2d61725438457920b12f562894b7aa7eb1fea1/content/public/test/test_utils.cc
[modify] https://crrev.com/ba2d61725438457920b12f562894b7aa7eb1fea1/content/public/test/test_utils.h
[modify] https://crrev.com/ba2d61725438457920b12f562894b7aa7eb1fea1/content/test/content_browser_test_utils_internal.cc

Owner: ah...@yandex-team.ru
Status: Started (was: Unconfirmed)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 2 2016

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

commit dd10720676efff002490f702118e052dcd210f18
Author: ahest <ahest@yandex-team.ru>
Date: Fri Dec 02 12:32:34 2016

Add thread checker to base::RunLoop.

The purpose is to make sure that after switching users of
MessageLoopRunner to RunLoop we won't lose any correctness checks.

Also add a comment that encourages to use RunLoop where possible, and
rename GetQuitTaskForRunLoop to GetDeferredQuitTaskForRunLoop to convey
is purpose clearly.

BUG=668707

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

[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/base/run_loop.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/base/run_loop.h
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/chrome/browser/chromeos/file_manager/zip_file_creator_browsertest.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/chrome/browser/sync/test/integration/dictionary_helper.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/chrome/browser/sync_file_system/sync_file_system_service_unittest.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/content/browser/loader/async_revalidation_manager_browsertest.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/content/public/test/test_launcher.h
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/content/public/test/test_utils.cc
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/content/public/test/test_utils.h
[modify] https://crrev.com/dd10720676efff002490f702118e052dcd210f18/extensions/test/result_catcher.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 2 2016

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

commit f8675b380dff12f69ad3b61920f9e8fdcfcadae8
Author: rockot <rockot@chromium.org>
Date: Fri Dec 02 21:33:31 2016

Revert of Add thread checking to RunLoop, deprecate MessageLoopRunner. (patchset #4 id:20002 of https://codereview.chromium.org/2537893002/ )

Reason for revert:
Speculative revert in an effort to pinpoint the cause of  http://crbug.com/670844  - will reland if the flake doesn't clear

Original issue's description:
> Add thread checker to base::RunLoop.
>
> The purpose is to make sure that after switching users of
> MessageLoopRunner to RunLoop we won't lose any correctness checks.
>
> Also add a comment that encourages to use RunLoop where possible, and
> rename GetQuitTaskForRunLoop to GetDeferredQuitTaskForRunLoop to convey
> is purpose clearly.
>
> BUG=668707
>
> Committed: https://crrev.com/dd10720676efff002490f702118e052dcd210f18
> Cr-Commit-Position: refs/heads/master@{#435924}

TBR=avi@chromium.org,thakis@chromium.org,asargent@chromium.org,ahest@yandex-team.ru
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=668707

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

[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/base/run_loop.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/base/run_loop.h
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/chrome/browser/chromeos/file_manager/zip_file_creator_browsertest.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/chrome/browser/sync/test/integration/dictionary_helper.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/chrome/browser/sync_file_system/sync_file_system_service_unittest.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/content/browser/loader/async_revalidation_manager_browsertest.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/content/public/test/test_launcher.h
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/content/public/test/test_utils.cc
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/content/public/test/test_utils.h
[modify] https://crrev.com/f8675b380dff12f69ad3b61920f9e8fdcfcadae8/extensions/test/result_catcher.cc

Comment 5 by kbr@chromium.org, Dec 5 2016

Blockedon: 670844
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 9 2016

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

commit 72c1b445bdcf34230617c75b282104019fde3614
Author: ahest <ahest@yandex-team.ru>
Date: Fri Dec 09 20:40:38 2016

Reland "Add thread checking to RunLoop, deprecate MessageLoopRunner. (patchset #4 id:20002 of https://codereview.chromium.org/2537893002/ )"

Reverted in https://codereview.chromium.org/2548883002/
Reason for relanding: turns out something else was causing these failures.
See details in https://bugs.chromium.org/p/chromium/issues/detail?id=670844#c17

TBR=avi@chromium.org,thakis@chromium.org,asargent@chromium.org
BUG=668707

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

[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/base/run_loop.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/base/run_loop.h
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/chrome/browser/chromeos/file_manager/zip_file_creator_browsertest.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/chrome/browser/sync/test/integration/dictionary_helper.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/chrome/browser/sync_file_system/sync_file_system_service_unittest.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/chrome/browser/ui/views/ash/tab_scrubber_browsertest.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/content/browser/loader/async_revalidation_manager_browsertest.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/content/public/test/test_launcher.h
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/content/public/test/test_utils.cc
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/content/public/test/test_utils.h
[modify] https://crrev.com/72c1b445bdcf34230617c75b282104019fde3614/extensions/test/result_catcher.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 13 2016

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

commit 68c9f104d87f31212b353d0fc20c3c57ec23125e
Author: ahest <ahest@yandex-team.ru>
Date: Tue Dec 13 11:34:38 2016

Fix cases where RunLoop is created without a MessageLoop.

The tests in these cases can pass only if Quit is called before Run, or
if Run is never called at all. If this is guaranteed to hold true, then
RunLoop is not needed (such as in timer_unittest.cc). Other cases (where
it is not guaranteed) are tests that can crash occasionally.

BUG=668707

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

[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/base/run_loop.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/base/timer/timer_unittest.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/chrome/browser/chromeos/login/screenshot_testing/screenshot_tester.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/chrome/browser/chromeos/login/screenshot_testing/screenshot_tester.h
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/chrome/browser/chromeos/login/screenshot_testing/screenshot_testing_mixin.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/chrome/browser/chromeos/login/signin/device_id_browsertest.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/chrome/browser/chromeos/system/device_disabling_browsertest.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/content/browser/loader/async_revalidation_manager_browsertest.cc
[modify] https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e/media/base/audio_renderer_mixer_input_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 12 2017

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

commit e7469132628fa7bd530d68c134e4159d2cfc1804
Author: ahest <ahest@yandex-team.ru>
Date: Thu Jan 12 23:56:32 2017

Convert more test helpers to base::RunLoop, fix page title checks.

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

Regarding title checks in browser_browsertest.cc: in these tests there
is no guarantee on when the new tab will start and stop loading. It
can start loading after WindowedNotificationObserver::Wait returns; also
it can stop loading while we're still inside it. We can explicitly wait
for the title to handle all these cases, and this makes the call to
WaitForLoadStop unnecessary.

BUG=668707
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/browser_test_base.cc
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/browser_test_utils.h
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/test_frame_navigation_observer.cc
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/test_frame_navigation_observer.h
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/test_utils.cc
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/public/test/test_utils.h
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/test/content_browser_test_utils_internal.cc
[modify] https://crrev.com/e7469132628fa7bd530d68c134e4159d2cfc1804/content/test/content_browser_test_utils_internal.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 13 2017

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

commit 0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667
Author: xlai <xlai@chromium.org>
Date: Fri Jan 13 21:18:06 2017

Revert of Convert more test helpers to base::RunLoop, fix page title checks. (patchset #5 id:80001 of https://codereview.chromium.org/2601843002/ )

Reason for revert:
ClickModifierTest.WindowOpenBasicClickTest and ClickModifierTest.WindowOpenControlShiftClickTest become very flaky recently. Example builds are all
recorded at  crbug.com/681035 .

By looking at the error log file (https://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20%281%29/builds/19997/steps/browser_tests/logs/stdio), I notice that
the error comes from src\chrome\browser\ui\browser_browsertest.cc(2451) which is recently modified by this CL. Therefore the revert.

Original issue's description:
> Convert more test helpers to base::RunLoop, fix page title checks.
>
> This CL removes the "deferred quit" from most of the touched helpers.
>
> The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
> They were already using immediate quit mode, so there is no change in
> their behavior, just cleanup.
>
> Regarding title checks in browser_browsertest.cc: in these tests there
> is no guarantee on when the new tab will start and stop loading. It
> can start loading after WindowedNotificationObserver::Wait returns; also
> it can stop loading while we're still inside it. We can explicitly wait
> for the title to handle all these cases, and this makes the call to
> WaitForLoadStop unnecessary.
>
>
> BUG=668707
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
>
> Review-Url: https://codereview.chromium.org/2601843002
> Cr-Commit-Position: refs/heads/master@{#443395}
> Committed: https://chromium.googlesource.com/chromium/src/+/e7469132628fa7bd530d68c134e4159d2cfc1804

TBR=nasko@chromium.org,pkasting@chromium.org,ahest@yandex-team.ru
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=668707, 681035 

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

[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/browser_test_base.cc
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/browser_test_utils.h
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/test_frame_navigation_observer.cc
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/test_frame_navigation_observer.h
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/test_utils.cc
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/public/test/test_utils.h
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/test/content_browser_test_utils_internal.cc
[modify] https://crrev.com/0fe6b1a44d65f685dc90aa7ef6c2b44dca2c9667/content/test/content_browser_test_utils_internal.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 19 2017

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

commit 29276c3a4db55690390a4a2c7d072f21cd7b34d9
Author: ahest <ahest@yandex-team.ru>
Date: Thu Jan 19 07:36:31 2017

Convert more test helpers to base::RunLoop, fix page title checks. (Reland)

This CL removes the "deferred quit" from most of the touched helpers.

The exclusions are UrlCommitObserver and TestFrameNavigationObserver.
They were already using immediate quit mode, so there is no change in
their behavior, just cleanup.

This is a reland of https://codereview.chromium.org/2601843002. It
caused flakiness in a pair of ClickModifierTests, because I was wrong
when removed the WaitForLoadStop call. After that the TitleWatcher could
be created after the load start, but before the title arrives. In this
case WebContents::GetTitle basically returns the url, and TitleWatcher
was modified to simply return it.
Turns out that waiting this way (without specific expectations) for a
title can easily be done wrong, so I removed changes related to it.
These tests now use TestNavigationObserver, which should be robust
(unless there is some unrelated feature that can create WebContents at
a random time, which I believe is not the case in chromium browser tests).

The first patch set contains the original CL, the second one includes
the fix.

BUG=668707,  681035 

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

[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/browser_test_base.cc
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/browser_test_utils.h
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/test_frame_navigation_observer.cc
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/test_frame_navigation_observer.h
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/test_utils.cc
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/public/test/test_utils.h
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/test/content_browser_test_utils_internal.cc
[modify] https://crrev.com/29276c3a4db55690390a4a2c7d072f21cd7b34d9/content/test/content_browser_test_utils_internal.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 25 2017

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

commit c2f9b2a000040b13963a67b6eac0075ec11fa12c
Author: ahest <ahest@yandex-team.ru>
Date: Wed Jan 25 17:13:52 2017

Quit immediately in TestNavigationObserver.

TestNavigationObserver is currently the most often used test helper that
employs a MessageLoopRunner with deferred quit. Combined with the calls
to the two NavigateToURL* functions, which use it internally (one in
ui_test_utils in chrome/ and the other in content_browser_test_utils in
content/), there are around 3200 instantiations in the codebase. This CL
switches around 40% of them to quit immediately.

The remaining part is the calls to ui_test_utils::NavigateToURL. It is
left out because there are several problematic cases which I think are
better to investigate at once instead of patching with calls to
RunAllPendingInMessageLoop, and looks like this is going to be a
completely different story.

BUG=668707
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/c2f9b2a000040b13963a67b6eac0075ec11fa12c/chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc
[modify] https://crrev.com/c2f9b2a000040b13963a67b6eac0075ec11fa12c/chrome/test/base/ui_test_utils.cc
[modify] https://crrev.com/c2f9b2a000040b13963a67b6eac0075ec11fa12c/content/browser/frame_host/navigation_handle_impl_browsertest.cc
[modify] https://crrev.com/c2f9b2a000040b13963a67b6eac0075ec11fa12c/content/public/test/test_navigation_observer.cc
[modify] https://crrev.com/c2f9b2a000040b13963a67b6eac0075ec11fa12c/content/public/test/test_navigation_observer.h

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 24 2017

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

commit 474e1f148ab83dde92b93a93e53cf83d77b9be13
Author: ahest <ahest@yandex-team.ru>
Date: Fri Feb 24 03:24:49 2017

Switch WindowedNotificationObserver to use base::RunLoop.

Now it will quit the message loop immediately after receiving the
notification. Also it does not allow nested tasks anymore.

BUG=668707

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

[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/ui/login/login_handler_test_utils.h
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/content/public/test/test_utils.cc
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/content/public/test/test_utils.h
[modify] https://crrev.com/474e1f148ab83dde92b93a93e53cf83d77b9be13/extensions/test/extension_test_notification_observer.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 24 2017

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

commit 0fc562c98c420de40daa775a1aacb52a4760a9b4
Author: treib <treib@chromium.org>
Date: Fri Feb 24 12:49:11 2017

Revert of Switch WindowedNotificationObserver to use base::RunLoop. (patchset #4 id:60001 of https://codereview.chromium.org/2701473007/ )

Reason for revert:
Suspected of causing flakiness in PrintPreviewDialogControllerBrowserTest.ReloadInitiatorTab

More details in  crbug.com/695835 

Original issue's description:
> Switch WindowedNotificationObserver to use base::RunLoop.
>
> Now it will quit the message loop immediately after receiving the
> notification. Also it does not allow nested tasks anymore.
>
> BUG=668707
>
> Review-Url: https://codereview.chromium.org/2701473007
> Cr-Commit-Position: refs/heads/master@{#452739}
> Committed: https://chromium.googlesource.com/chromium/src/+/474e1f148ab83dde92b93a93e53cf83d77b9be13

TBR=jochen@chromium.org,rdevlin.cronin@chromium.org,ahest@yandex-team.ru
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=668707

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

[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/ui/login/login_handler_test_utils.h
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/content/public/test/test_utils.cc
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/content/public/test/test_utils.h
[modify] https://crrev.com/0fc562c98c420de40daa775a1aacb52a4760a9b4/extensions/test/extension_test_notification_observer.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 6 2017

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

commit d6ecbaf377cb296924168a77832c80ef53c75667
Author: ahest <ahest@yandex-team.ru>
Date: Mon Mar 06 16:06:46 2017

Reland: Switch WindowedNotificationObserver to use base::RunLoop.

Now it will quit the message loop immediately after receiving the
notification. Also it does not allow nested tasks anymore.

Relanding after fixing test failures (bugs  695835 ,  695780 ).

Original CL (https://codereview.chromium.org/2701473007) is in patchset #1.

BUG=668707

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

[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/ui/login/login_handler_test_utils.h
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/content/public/test/test_utils.cc
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/content/public/test/test_utils.h
[modify] https://crrev.com/d6ecbaf377cb296924168a77832c80ef53c75667/extensions/test/extension_test_notification_observer.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 7 2017

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

commit 84fa74314b5e4c1da24ee66dea4f81ec0fd5a464
Author: jam <jam@chromium.org>
Date: Tue Mar 07 20:48:22 2017

Revert of Reland: Switch WindowedNotificationObserver to use base::RunLoop. (patchset #3 id:40001 of https://codereview.chromium.org/2720513003/ )

Reason for revert:
Sorry, I realize this isn't on a waterfall yet but we're very closed to launching PlzNavigate.

I bisected the failure of NavigatingExtensionPopupBrowserTest.DownloadViaPost on Mac with --enable-browser-side-navigation

Original issue's description:
> Reland: Switch WindowedNotificationObserver to use base::RunLoop.
>
> Now it will quit the message loop immediately after receiving the
> notification. Also it does not allow nested tasks anymore.
>
> Relanding after fixing test failures (bugs  695835 ,  695780 ).
>
> Original CL (https://codereview.chromium.org/2701473007) is in patchset #1.
>
> BUG=668707
>
> Review-Url: https://codereview.chromium.org/2720513003
> Cr-Commit-Position: refs/heads/master@{#454872}
> Committed: https://chromium.googlesource.com/chromium/src/+/d6ecbaf377cb296924168a77832c80ef53c75667

TBR=jochen@chromium.org,bartfab@chromium.org,rdevlin.cronin@chromium.org,ahest@yandex-team.ru
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=668707

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

[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/ui/login/login_handler_test_utils.h
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/content/public/test/test_utils.cc
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/content/public/test/test_utils.h
[modify] https://crrev.com/84fa74314b5e4c1da24ee66dea4f81ec0fd5a464/extensions/test/extension_test_notification_observer.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 20 2017

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

commit 25911df55b8f939560df1f664f8cc8941ddc4677
Author: ahest <ahest@yandex-team.ru>
Date: Thu Apr 20 15:28:34 2017

Reland #2: Switch WindowedNotificationObserver to use base::RunLoop.

Now it will quit the message loop immediately after receiving the
notification. Also it does not allow nested tasks anymore.

Previous CL (https://codereview.chromium.org/2720513003) is in patchset #1.
Added fixes for:
 - browser_tests.NavigatingExtensionPopupBrowserTest.DownloadViaPost with PlzNavigate;
 - interactive_ui_tests.ConstrainedWindowViewTest.NavigationOnBackspace.
Also rephrased comment in print_preview_dialog_controller_browsertest.cc.

BUG=668707

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

[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/apps/drive/drive_app_provider_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/extensions/chrome_extension_test_notification_observer.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/login/login_handler_test_utils.h
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/views/constrained_window_views_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/chrome/browser/unload_browsertest.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/content/public/test/test_utils.cc
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/content/public/test/test_utils.h
[modify] https://crrev.com/25911df55b8f939560df1f664f8cc8941ddc4677/extensions/test/extension_test_notification_observer.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 20 2017

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

commit 3d232ee96d76f7ab59d2b0cd89bb6949e91dd120
Author: sgurun <sgurun@chromium.org>
Date: Thu Apr 20 18:44:33 2017

Revert of Revert "Revert of Reland: Switch WindowedNotificationObserver to use base::RunLoop. (patchset #3 id… (patchset #11 id:200001 of https://codereview.chromium.org/2740783003/ )

Reason for revert:
speculative revert for  crbug.com/713820 

Original issue's description:
> Reland #2: Switch WindowedNotificationObserver to use base::RunLoop.
>
> Now it will quit the message loop immediately after receiving the
> notification. Also it does not allow nested tasks anymore.
>
> Previous CL (https://codereview.chromium.org/2720513003) is in patchset #1.
> Added fixes for:
>  - browser_tests.NavigatingExtensionPopupBrowserTest.DownloadViaPost with PlzNavigate;
>  - interactive_ui_tests.ConstrainedWindowViewTest.NavigationOnBackspace.
> Also rephrased comment in print_preview_dialog_controller_browsertest.cc.
>
> BUG=668707
>
> Review-Url: https://codereview.chromium.org/2740783003
> Cr-Commit-Position: refs/heads/master@{#466016}
> Committed: https://chromium.googlesource.com/chromium/src/+/25911df55b8f939560df1f664f8cc8941ddc4677

TBR=jam@chromium.org,vabr@chromium.org,ahest@yandex-team.ru
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=668707

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

[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/apps/drive/drive_app_provider_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/extensions/chrome_extension_test_notification_observer.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/cocoa/apps/native_app_window_cocoa_browsertest.mm
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/login/login_handler_test_utils.h
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/views/constrained_window_views_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/chrome/browser/unload_browsertest.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/content/public/test/test_utils.cc
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/content/public/test/test_utils.h
[modify] https://crrev.com/3d232ee96d76f7ab59d2b0cd89bb6949e91dd120/extensions/test/extension_test_notification_observer.cc

Comment 19 by gab@chromium.org, Mar 21 2018

Blocking: 824431
What's the status of this? I'm looking to get rid of DeferredQuit and other task running APIs in content/public/test/test_utils.h in favor of RunLoop and new TestBrowserThreadBundle member APIs.

Comment 20 by gab@chromium.org, Mar 21 2018

Cc: gab@chromium.org
I think that most "high impact" usages are cleared, but there is still a lot of work to do.

There were problems with rare test flakes in the attempt to "fix" WindowedNotificationObserver. At that time tests had to pass both with PlzNavigate and in the old navigation mode, which added a lot of complexity. Recently I rebased that patch and ran some tests, but did not have time to push it further. Now that you're blocked on this, I'll resume my work on that patch and hope to send a CL for review soon.

Besides WindowedNotificationObserver there may be other widely used classes/functions which use deferred quit, and there is a number of "local" usages in tests. Feel free to take and fix any of these :)  It would definitely good to post a notification here beforehand to prevent us doing the same work twice.

Comment 22 by gab@chromium.org, Mar 26 2018

Cc: robliao@chromium.org
Components: Test Internals
+robliao who's working in this area as well and investigating problematic MessageLoopRunners.
Project Member

Comment 23 by bugdroid1@chromium.org, Mar 27 2018

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

commit 089385b65f91ae1ec33f281ec68cb6a52ef247be
Author: Robert Liao <robliao@chromium.org>
Date: Tue Mar 27 16:29:13 2018

Convert BrowsingDataRemover Test Utils from MessageLoopRunner to RunLoop

This makes migrating to FlushAsyncForTesting() clearer.

BUG=668707,749312

Change-Id: I074c71c21e2cbbc3fc2b1ea87d08ebd957b6c7f7
Reviewed-on: https://chromium-review.googlesource.com/978839
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546130}
[modify] https://crrev.com/089385b65f91ae1ec33f281ec68cb6a52ef247be/content/public/test/browsing_data_remover_test_util.cc
[modify] https://crrev.com/089385b65f91ae1ec33f281ec68cb6a52ef247be/content/public/test/browsing_data_remover_test_util.h

Project Member

Comment 24 by bugdroid1@chromium.org, May 23 2018

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

commit 450d4a7d8c0437fad9af4e591b1190b9103e40c5
Author: Roger McFarlane <rogerm@chromium.org>
Date: Tue May 22 23:59:48 2018

[autofill] Fix flaky interatcive/isolation test

The test has an inherent race between triggering the showing of
suggestions and starting the loop to wait for notification that
the suggestions have been shown.

The test was asserting that the loop had been started before
quiting. Removed this (racily invalid) insertion.

Also switched over to use base::RunLoop instead of using
content::MessageLoopRunner (per crbug.com/668707), to remove
additional flakiness.

Bug:  843935 , 668707
Change-Id: I3084b64697983b64c34fedd26e690d419807f2a8
Reviewed-on: https://chromium-review.googlesource.com/1069531
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560842}
[modify] https://crrev.com/450d4a7d8c0437fad9af4e591b1190b9103e40c5/chrome/browser/autofill/autofill_interactive_uitest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, May 29 2018

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

commit 1669311f81829663cd33a277aef769270d58e1cc
Author: Wez <wez@chromium.org>
Date: Tue May 29 21:59:42 2018

Migrate WebViewAPITest from MessageLoopRunner to RunLoop.

Bug: 668707
Change-Id: I3edf8561940289e4c6778c8f0acfd99ba4042b52
Reviewed-on: https://chromium-review.googlesource.com/1075858
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562615}
[modify] https://crrev.com/1669311f81829663cd33a277aef769270d58e1cc/extensions/browser/guest_view/web_view/web_view_apitest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Jun 2 2018

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

commit f2311d70f0c76e61af666daebce22df5df024961
Author: Wez <wez@chromium.org>
Date: Sat Jun 02 02:02:18 2018

Migrate browser_test_utils helpers from MessageLoopRunner to RunLoop.

Bug: 668707
Change-Id: I4f10192f17a7e9bc300e9ecbfb9774c8fe6b978c
Reviewed-on: https://chromium-review.googlesource.com/1075818
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563913}
[modify] https://crrev.com/f2311d70f0c76e61af666daebce22df5df024961/chrome/browser/extensions/api/runtime/runtime_apitest.cc
[modify] https://crrev.com/f2311d70f0c76e61af666daebce22df5df024961/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/f2311d70f0c76e61af666daebce22df5df024961/content/public/test/browser_test_utils.h

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 29 2018

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

commit 1748d4dd32511f3a488c17b02edd336f8dc77203
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 20:47:27 2018

Fix/deflake tests in /chrome/browser/ui/views.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=bsep@chromium.org

Bug: 668707
Change-Id: I5e19527ac309235112faed7a250bf344c2376242
Reviewed-on: https://chromium-review.googlesource.com/1120498
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571619}
[modify] https://crrev.com/1748d4dd32511f3a488c17b02edd336f8dc77203/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Jun 29 2018

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

commit ffceec4aa38b69cafd523f19cd08a8e619cc2454
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 20:48:26 2018

Fix/deflake tests in /chrome/browser/printing.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=dpapad@chromium.org

Bug: 668707
Change-Id: I723a927eadbb422cff9165e159bfcb72412f6c5a
Reviewed-on: https://chromium-review.googlesource.com/1120499
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571620}
[modify] https://crrev.com/ffceec4aa38b69cafd523f19cd08a8e619cc2454/chrome/browser/printing/print_preview_dialog_controller_browsertest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Jun 29 2018

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

commit 18e17a020a415767fab02726de4e83ca9aa262eb
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 20:54:42 2018

Fix/deflake tests in /chrome/browser/renderer_context_menu.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=avi@chromium.org

Bug: 668707
Change-Id: I890f55395c05eebbe390b1401dea3a0f4d21b19c
Reviewed-on: https://chromium-review.googlesource.com/1120495
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571623}
[modify] https://crrev.com/18e17a020a415767fab02726de4e83ca9aa262eb/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jun 29 2018

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

commit 18e17a020a415767fab02726de4e83ca9aa262eb
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 20:54:42 2018

Fix/deflake tests in /chrome/browser/renderer_context_menu.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=avi@chromium.org

Bug: 668707
Change-Id: I890f55395c05eebbe390b1401dea3a0f4d21b19c
Reviewed-on: https://chromium-review.googlesource.com/1120495
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571623}
[modify] https://crrev.com/18e17a020a415767fab02726de4e83ca9aa262eb/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Jun 29 2018

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

commit 0762c38830a791d2716f4830f518b8376ab84267
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 22:04:25 2018

Fix/deflake tests in /chrome/browser/unload_browsertest.cc.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=alexmos@chromium.org

Bug: 668707
Change-Id: I2d723e7938ac7e07c6334e58b26b60e7e493834e
Reviewed-on: https://chromium-review.googlesource.com/1120487
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571661}
[modify] https://crrev.com/0762c38830a791d2716f4830f518b8376ab84267/chrome/browser/unload_browsertest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Jun 29 2018

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

commit a05c9815bca791b0c1b22b88f8520d02495d3869
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 22:05:34 2018

Fix/deflake tests in /chrome/browser/chrome_site_per_process_browsertest.cc.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=alexmos@chromium.org

Bug: 668707
Change-Id: Ic3c72233d13ee53965bc250602462da4b12aa4c8
Reviewed-on: https://chromium-review.googlesource.com/1120489
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571663}
[modify] https://crrev.com/a05c9815bca791b0c1b22b88f8520d02495d3869/chrome/browser/chrome_site_per_process_browsertest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jun 29 2018

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

commit 7287e22585babb2143b7bfadd4363695b4103785
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 22:15:54 2018

Fix/deflake tests in /chrome/browser/ui/webui/net_internals.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=eroman@chromium.org

Bug: 668707
Change-Id: I35921def6a71f95de842a19995346182229f8ff4
Reviewed-on: https://chromium-review.googlesource.com/1120486
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#571671}
[modify] https://crrev.com/7287e22585babb2143b7bfadd4363695b4103785/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Jun 29 2018

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

commit c4ff8e3990189cee497960139a287012c5183241
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jun 29 23:33:08 2018

Fix/deflake tests in /extensions.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=reillyg@chromium.org

Bug: 668707
Change-Id: Ia0b9003fd30e75f975fe396fc0036274ac9142a3
Reviewed-on: https://chromium-review.googlesource.com/1120496
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571708}
[modify] https://crrev.com/c4ff8e3990189cee497960139a287012c5183241/extensions/test/extension_test_notification_observer.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Jul 2

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

commit 91ca88a0ea4b2cf820049d9158b0404c24935b47
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Mon Jul 02 12:36:46 2018

Fix/deflake tests in /chrome/browser/extensions.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=finnur@chromium.org

Bug: 668707
Change-Id: Idf89660cd9ced8c35a3a8f5b7dab945b57e7d57f
Reviewed-on: https://chromium-review.googlesource.com/1120491
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571874}
[modify] https://crrev.com/91ca88a0ea4b2cf820049d9158b0404c24935b47/chrome/browser/extensions/chrome_extension_test_notification_observer.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Jul 3

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

commit 9bde960d4848381b80d9a279dbcf5b4d4d0af267
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Tue Jul 03 11:40:07 2018

Simplify render_process_host_chrome_browsertest.cc

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner. There were two
nested waits for the same notification which does not make sense and
does not work as is with the upcoming change.

This CL was split out from https://crrev.com/c/982612 and uploaded by
git cl split.

R=jochen@chromium.org

Bug: 668707
Change-Id: I217e5ea431d4a58b15ed1eef51a912142124c8b8
Reviewed-on: https://chromium-review.googlesource.com/1120488
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#572171}
[modify] https://crrev.com/9bde960d4848381b80d9a279dbcf5b4d4d0af267/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jul 6

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

commit dba99b939185992c5839e8a207da6f65a605b235
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Fri Jul 06 16:49:17 2018

Fix/deflake tests in /chrome/browser/extensions/api/extension_action.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=jyasskin@chromium.org

Bug: 668707
Change-Id: I00e35677dac8a785dc9936a64d568cde53b02d1c
Reviewed-on: https://chromium-review.googlesource.com/1120500
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#572995}
[modify] https://crrev.com/dba99b939185992c5839e8a207da6f65a605b235/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Jul 9

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

commit 3eafc7f1009b64dfed21bac94e133e1629e1c8d3
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Mon Jul 09 17:05:15 2018

Fix/deflake tests in /chrome/browser/profiles.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

This CL is split out from https://crrev.com/c/982612.

This CL was uploaded by git cl split.

R=skuhne@chromium.org

Bug: 668707
Change-Id: I3bd19577a68a77d450b8c3bfaff5dda39813b66d
Reviewed-on: https://chromium-review.googlesource.com/1120497
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573338}
[modify] https://crrev.com/3eafc7f1009b64dfed21bac94e133e1629e1c8d3/chrome/browser/profiles/profile_list_desktop_browsertest.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Jul 9

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

commit aa72414951c47a474b12f873e6aed8ca677a4057
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Mon Jul 09 17:19:26 2018

Fix race and refactor multi-realm tests in login_handler_browsertest.cc.

This is a prerequisite for switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner.

These tests involve 2 auth realms and 4 LoginHandlers (i.e. 4 requests needing
auth). They respond to at least 2 auth requests and may stop after that, if
other requests are not received by that time.
The tests pass fine if all LoginHandlers for a realm are created "at once"
(during one WindowedNotificationObserver::Wait), because response for one
LoginHandler is used by all handlers of the same realm.
Actually LoginHandlers may be created with some delay and the tests should wait
for it explicitly, otherwise they time out because there are lingering requests
and page load does not stop.

The upcoming change greatly reduces the duration of
WindowedNotificationObserver::Wait which makes these tests always fail.
The patch fixes the race and factors out common code.

This CL is split out from https://crrev.com/c/982612 and uploaded by git cl
split.


R=davidben@chromium.org

Bug: 668707
Change-Id: I919ec47132494f9bcbb80f65be00e5bef875192a
Reviewed-on: https://chromium-review.googlesource.com/1120493
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#573347}
[modify] https://crrev.com/aa72414951c47a474b12f873e6aed8ca677a4057/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/aa72414951c47a474b12f873e6aed8ca677a4057/chrome/browser/ui/login/login_handler_test_utils.cc
[modify] https://crrev.com/aa72414951c47a474b12f873e6aed8ca677a4057/chrome/browser/ui/login/login_handler_test_utils.h

Project Member

Comment 40 by bugdroid1@chromium.org, Jul 18

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

commit 54fcb75f67b407d53769d6a7e5d499902702b9c1
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Wed Jul 18 17:26:16 2018

Reland #3: Switch WindowedNotificationObserver to use base::RunLoop.

Now it will quit the message loop immediately after receiving the
notification. Also it does not allow nested tasks anymore.

Bug: 668707
Change-Id: I8d8a0ffb073236c3bbd00877bd48cc496b1c5036
Reviewed-on: https://chromium-review.googlesource.com/982612
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Cr-Commit-Position: refs/heads/master@{#576110}
[modify] https://crrev.com/54fcb75f67b407d53769d6a7e5d499902702b9c1/content/public/test/test_utils.cc
[modify] https://crrev.com/54fcb75f67b407d53769d6a7e5d499902702b9c1/content/public/test/test_utils.h

Project Member

Comment 41 by bugdroid1@chromium.org, Jul 19

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

commit e0b982ec52e0e8f1478523787c3615d0df4f8662
Author: Gabriel Charette <gab@chromium.org>
Date: Thu Jul 19 16:26:57 2018

Revert "Reland #3: Switch WindowedNotificationObserver to use base::RunLoop."

This reverts commit 54fcb75f67b407d53769d6a7e5d499902702b9c1.

Reason for revert: landed too early and causing flakes

Original change's description:
> Reland #3: Switch WindowedNotificationObserver to use base::RunLoop.
> 
> Now it will quit the message loop immediately after receiving the
> notification. Also it does not allow nested tasks anymore.
> 
> Bug: 668707
> Change-Id: I8d8a0ffb073236c3bbd00877bd48cc496b1c5036
> Reviewed-on: https://chromium-review.googlesource.com/982612
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
> Cr-Commit-Position: refs/heads/master@{#576110}

TBR=gab@chromium.org,jam@chromium.org,ahest@yandex-team.ru

Change-Id: Ibc82b94b56e77bb9914c74517dd9f755527544f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 668707
Reviewed-on: https://chromium-review.googlesource.com/1143740
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576519}
[modify] https://crrev.com/e0b982ec52e0e8f1478523787c3615d0df4f8662/content/public/test/test_utils.cc
[modify] https://crrev.com/e0b982ec52e0e8f1478523787c3615d0df4f8662/content/public/test/test_utils.h

Project Member

Comment 42 by bugdroid1@chromium.org, Aug 15

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

commit 782c0e31591d55a13aa3d488357bf7bdbe174b46
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Wed Aug 15 12:26:49 2018

Reland #4: Switch WindowedNotificationObserver to use base::RunLoop.

Now it will quit the message loop immediately after receiving the
notification. Also it does not allow nested tasks anymore.

Flaky test DeviceLocalAccountExternalPolicyLoaderTest.ForceInstallListSet
is termporarily disabled.

TBR=jam@chromium.org

Bug: 668707, 865453 
Change-Id: I1cbceb298311a9b7905c0cec410cb82330effccd
Reviewed-on: https://chromium-review.googlesource.com/1146700
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583220}
[modify] https://crrev.com/782c0e31591d55a13aa3d488357bf7bdbe174b46/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc
[modify] https://crrev.com/782c0e31591d55a13aa3d488357bf7bdbe174b46/content/public/test/test_utils.cc
[modify] https://crrev.com/782c0e31591d55a13aa3d488357bf7bdbe174b46/content/public/test/test_utils.h

Project Member

Comment 43 by bugdroid1@chromium.org, Aug 21

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

commit 5232132f450489fb6fd6624f977503310d6032e1
Author: Alexander Semashko <ahest@yandex-team.ru>
Date: Tue Aug 21 09:40:42 2018

Fix and enable DeviceLocalAccountExternalPolicyLoaderTest.ForceInstallListSet.

The test was disabled because switching WindowedNotificationObserver to use
base::RunLoop instead of deprecated MessageLoopRunner made it flaky.

R=atwilson@chromium.org

Bug: 668707,  865453 
Change-Id: Ideff2420f51232bb7e590a0f242efaec64f598a0
Reviewed-on: https://chromium-review.googlesource.com/1120494
Commit-Queue: Alexander Semashko <ahest@yandex-team.ru>
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584700}
[modify] https://crrev.com/5232132f450489fb6fd6624f977503310d6032e1/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc

Components: -Internals
Project Member

Comment 45 by bugdroid1@chromium.org, Oct 30

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

commit 0439ffef9801e61eeed24fb15f4e9303b3555e1f
Author: Alexander Timin <altimin@chromium.org>
Date: Tue Oct 30 18:10:00 2018

[base] Make base::Thread::message_loop private.

We don't need to expose a MessageLoop from base::Thread --
task_runner() is enough to post tasks and other functionality
can be accessed via base::MessageLoopCurrent::Get when on the

R=gab@chromium.org
TBR=gab@chromium.org
BUG=891670,668707

Change-Id: I88996daa956f4a5e2dffda6fe31a9ee8102746eb
Reviewed-on: https://chromium-review.googlesource.com/c/1301437
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603962}
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/android/application_status_listener_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/files/file_descriptor_watcher_posix_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/files/file_path_watcher_fsevents.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/message_loop/message_loop_task_runner_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/message_loop/message_loop_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/message_loop/message_pump_libevent_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/message_loop/message_pump_perftest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/task/task_scheduler/task_scheduler_impl.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/task/task_scheduler/task_tracker_posix.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/task/task_scheduler/task_tracker_posix.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/task/task_scheduler/task_tracker_posix_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/test/test_io_thread.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/threading/thread.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/threading/thread_perftest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/base/threading/thread_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/chrome/browser/printing/print_job_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/chrome/browser/process_singleton_browsertest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/chrome/browser/sync/test/integration/single_client_directory_sync_test.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/chrome/chrome_cleaner/components/recovery_component.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/components/browser_sync/profile_sync_service.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/components/browser_sync/profile_sync_service.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/components/browser_watcher/window_hang_monitor_win_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/components/exo/wayland/clients/test/run_all_client_perftests.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/components/history/core/browser/history_service.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/components/sync/engine/net/http_bridge_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/content/browser/devtools/devtools_http_handler.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/content/child/child_process.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/content/public/test/browser_test_base.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/content/test/renderer_audio_output_stream_factory_context_impl_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/dbus/test_service.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/device/gamepad/gamepad_provider.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/ios/web/web_thread_impl.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/audio/audio_low_latency_input_output_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/cast/test/cast_benchmarks.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/gpu/android/codec_allocator.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/gpu/v4l2/v4l2_image_processor.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/media/gpu/vaapi/vaapi_video_decode_accelerator.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/net/base/network_change_notifier_win.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/net/cookies/cookie_store_test_callbacks.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/net/cookies/cookie_store_test_callbacks.h
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/ppapi/host/resource_message_filter_unittest.cc
[modify] https://crrev.com/0439ffef9801e61eeed24fb15f4e9303b3555e1f/services/device/battery/battery_status_manager_linux.cc

Sign in to add a comment