commit 7870bb641795a9b7e6c55046288293ef19b5b470
author Gabriel Charette <gab@chromium.org> Tue Nov 30 14:32:50 2021
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Nov 30 14:32:50 2021
tree 5d86b9f01bd189fb1dbdf84c4a5402126dfe3d97
parent 79d01721b976e2687ad08dfdfa8f2a1f0b0c9733

Reland "Reland "[content] Standardize "message loop starts" signal across Desktop, Android, and browser tests""

This reverts commit 572ed6584aa927d5744fc3e3929de9212ae2d9f1.

Reason for reland: Fixed FontUniqueNameBrowserTest. This CL
enables all of BrowserMainLoop::PreMainMessageLoopRun() to complete
before running the test hook. The test was flaking because
BrowserMainLoop::PreMainMessageLoopRun() initializes
DWriteFontLookupTableBuilder already and the browser test fixture
was built like a unit test (invoking ResetStateForTesting) which
raced with the BrowserMainLoop's live init request. It is no longer
necessary to explicitly init before the test and never was to
reset after the test (browser tests don't reuse procs).

Original change's description:
> Revert "Reland "[content] Standardize "message loop starts" signal across Desktop, Android, and browser tests""
>
> This reverts commit eea969e4bed54b8e9ba593cc5a05d7acbd5e9516.
>
> Reason for revert: lines up as culprit for a second time for
> crbug.com/1269720... must be it even though I don't see why.
>
> Original change's description:
> > Reland "[content] Standardize "message loop starts" signal across Desktop, Android, and browser tests"
> >
> > This reverts commit 865c12f322f76fc35407775aff890da597737cb2.
> >
> > Reason for revert: attempting reland.
> >
> > Original change's description:
> > > Revert "[content] Standardize "message loop starts" signal across Desktop, Android, and browser tests"
> > >
> > > This reverts commit 67bcc9c2d16d1b50fc85450528be2163cdd7d3b4.
> > >
> > > Reason for revert: suspect for crbug.com/1269720 though I can't
> > > explain why it would cause that failure (and that one alone)
> > >
> > > Original change's description:
> > > > [content] Standardize "message loop starts" signal across Desktop, Android, and browser tests
> > > >
> > > > And make content (BrowserMainLoop) responsible for running the
> > > > integration test hook (`MainFunctionParams::ui_task`) instead of each
> > > > BrowserMainParts impl doing so slightly differently.
> > > >
> > > > This is mostly a no-op but for some BrowserMainParts which did more work
> > > > after running `ui_task`, that work will now happen before `ui_task`
> > > > runs.
> > > >
> > > > Metrics recording in //chrome is centralized at the end of
> > > > PreMainMessageLoopRun on all platforms.
> > > >
> > > > Standardizing cross-platform ordering avoids forcing 4 awkward ordering
> > > > requirements in
> > > > https://chromium-review.googlesource.com/c/chromium/src/+/2892251 (see
> > > > diff of TODOs no longer needed after rebasing on this CL :
> > > > https://chromium-review.googlesource.com/c/chromium/src/+/2892251/44..47)
> > > >
> > > > This also allows getting rid of a force function in
> > > > startup_metrics_browsertest.cc to pretend some metrics were logged
> > > > (they now actually are without interfering with the product code).
> > > >
> > > > Part of the uncovered portion of BrowserMainLoop::PreMainMessageLoopRun
> > > > in browser tests was the part setting disallowance to do Blocking/Wait
> > > > operations on main thread. Unfortunately adding this ban breaks many
> > > > tests, browser_test_base.cc explicitly inverts those new thread
> > > > restrictions in this CL with a TODO to bring them in-line with the
> > > > product restrictions.
> > > >
> > > > BrowserMainParts overrides can get also rid of their |run_message_loop_|
> > > > variable when moving handling of the browser test hook (`ui_task`) in
> > > > WillRunMainMessageLoop(). And in a follow-up they will be updated to
> > > > only receive a `bool is_integration_test` from CreateBrowserMainParts
> > > > instead of the full MainFunctionParams which none use beyond that bool.
> > > >
> > > > ContentBrowserTestSanityTests verify that browser test bodies still run
> > > > as expected with this change.
> > > >
> > > > Bug: 1175074, 1253634
> > > > Change-Id: Ib757a02bbd982ef2b1132e2791e8ec1ce0305038
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3187153
> > > > Reviewed-by: Alexander Timin <altimin@chromium.org>
> > > > Reviewed-by: Sean Topping <seantopping@chromium.org>
> > > > Reviewed-by: Scott Violet <sky@chromium.org>
> > > > Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
> > > > Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> > > > Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> > > > Reviewed-by: Bo <boliu@chromium.org>
> > > > Commit-Queue: Gabriel Charette <gab@chromium.org>
> > > > Cr-Commit-Position: refs/heads/main@{#940917}
> > >
> > > TBR=sky@chromium.org,gab@chromium.org,boliu@chromium.org,rdevlin.cronin@chromium.org,sergeyu@chromium.org,skyostil@chromium.org,altimin@chromium.org,seantopping@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com
> > >
> > > Change-Id: I5ebcb181286d31e3a0bfb293c394da00e7970da4
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Bug: 1175074, 1253634, 1269720
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3276956
> > > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> > > Reviewed-by: Gabriel Charette <gab@chromium.org>
> > > Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> > > Auto-Submit: Gabriel Charette <gab@chromium.org>
> > > Owners-Override: anthonyvd <anthonyvd@chromium.org>
> > > Cr-Commit-Position: refs/heads/main@{#941225}
> >
> > # Not skipping CQ checks because this is a reland.
> >
> > Bug: 1175074, 1253634, 1269720
> > Change-Id: Ib0a6a93300425b62458206e2650735850fd68aec
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3279623
> > Reviewed-by: Gabriel Charette <gab@chromium.org>
> > Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> > Reviewed-by: Sean Topping <seantopping@chromium.org>
> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> > Auto-Submit: Gabriel Charette <gab@chromium.org>
> > Commit-Queue: anthonyvd <anthonyvd@chromium.org>
> > Commit-Queue: Sean Topping <seantopping@chromium.org>
> > Owners-Override: anthonyvd <anthonyvd@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#941336}
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1175074, 1253634, 1269720
> Change-Id: I8744b1abc5d68e5cc3462e6513ff3b466b3232c6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3278582
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Gabriel Charette <gab@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#941764}

Test: Explicitly ran win7-rel
Bug: 1175074, 1253634, 1269720
Change-Id: Ie75d87045e6e756ef85799afc79519d1fbbe8c9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3306906
Commit-Queue: Alexander Timin <altimin@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Owners-Override: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#946449}