That proposal seems reasonable.

Possible next steps before marking this bug as resolved:
 1. Add a note to //docs/testing/layout_test_expectations.md (or somewhere else) explicitly stating the policy with regards to NeedsManualRebaseline.
 2. Perhaps add a presubmit warning message when NeedsManualRebaseline lines are added.

Should this bug be updated given the move to rebaseline-cl?

Removing rebaseline-o-matic doesn't change anything about the use of NeedsManualRebasline yet.

It's still possible for people to manually rebaseline by basically disabling the tests with "NeedsManualRebaseline" lines and then later following up (hopefully) by rebaselining and removing these lines.

For a related bug, see bug 735176 -- I think we should be able to remove NeedsManualRebaseline or Rebaseline or both.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I think at this point we shouldn't be supporting NeedsManualRebaseline at all, because I don't think there's an excuse for why you'd rebaseline things after the fact (rather than before a CL landed).

We might want to confirm this on blink-dev, but I'd suggest removing this option and any supporting tooling.

+1 to remove this. I generally don't support any forms of "let's delay it for another day".

I may take this in Q3

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

commit 96753422ee78482f53241a9da7a5045b80f6f24f
Author: nednguyen <nednguyen@google.com>
Date: Mon Jul 16 19:42:50 2018

Remove stale comment in LayoutTests/TestExpectations that references NeedsManualRebaseline

This comment was added in https://chromium-review.googlesource.com/c/chromium/src/+/834758 but the test has been fixed since then.

Bug: 621126
Change-Id: I22f77f9548bef8741e9aaf7325346c860efca292
Reviewed-on: https://chromium-review.googlesource.com/1138724
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575387}
[modify] https://crrev.com/96753422ee78482f53241a9da7a5045b80f6f24f/third_party/WebKit/LayoutTests/TestExpectations

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

commit bbc0f4f447371f311f8fc0a65e6577fe344607a5
Author: Ned Nguyen <nednguyen@google.com>
Date: Tue Jul 17 17:07:20 2018

Rebaseline fast/forms/select/menulist-appearance-basic.html

Bug:  680407 ,621126
Change-Id: I4be6aeb3cdce35c89abaea8ce756aba456aa36b9
Reviewed-on: https://chromium-review.googlesource.com/1138794
Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#575693}
[modify] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/select/menulist-appearance-basic-expected.png
[modify] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/select/menulist-appearance-basic-expected.txt
[add] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac-mac10.11/fast/forms/select/menulist-appearance-basic-expected.png
[add] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac-mac10.11/fast/forms/select/menulist-appearance-basic-expected.txt
[add] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac-mac10.12/fast/forms/select/menulist-appearance-basic-expected.png
[add] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac-mac10.12/fast/forms/select/menulist-appearance-basic-expected.txt
[modify] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac/fast/forms/select/menulist-appearance-basic-expected.png
[modify] https://crrev.com/bbc0f4f447371f311f8fc0a65e6577fe344607a5/third_party/WebKit/LayoutTests/platform/mac/fast/forms/select/menulist-appearance-basic-expected.txt

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

commit 1b277b01fb2b57f68b1c7d9a63586274f1d40b8d
Author: nednguyen <nednguyen@google.com>
Date: Wed Jul 18 18:53:41 2018

Mark compositing/masks/mask-with-added-filters.html as Fail

This is to remove the last usage of NeedsManualRebaseline

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1bc7e63d41acf1f7813d8ac5fba4ae29f3b6691b

BUG: 865039, 621126
Change-Id: I1bc7e63d41acf1f7813d8ac5fba4ae29f3b6691b
Reviewed-on: https://chromium-review.googlesource.com/1142194
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576151}
[modify] https://crrev.com/1b277b01fb2b57f68b1c7d9a63586274f1d40b8d/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees

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

commit 30e38a4446035a41c16ddd36727557ce8bbbd320
Author: Ned Nguyen <nednguyen@google.com>
Date: Wed Jul 18 20:48:47 2018

Remove support of NeedsManualRebaseline in layout test

Bug: 621126
Change-Id: If111cc2b1f980f79362d5e52d101e915ae7ba6ef
Reviewed-on: https://chromium-review.googlesource.com/1140927
Commit-Queue: Robert Ma <robertma@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576200}
[modify] https://crrev.com/30e38a4446035a41c16ddd36727557ce8bbbd320/third_party/blink/tools/blinkpy/web_tests/models/test_expectations.py
[modify] https://crrev.com/30e38a4446035a41c16ddd36727557ce8bbbd320/third_party/blink/tools/blinkpy/web_tests/models/test_expectations_unittest.py
[modify] https://crrev.com/30e38a4446035a41c16ddd36727557ce8bbbd320/third_party/blink/tools/blinkpy/web_tests/models/test_run_results_unittest.py
[modify] https://crrev.com/30e38a4446035a41c16ddd36727557ce8bbbd320/third_party/blink/tools/blinkpy/web_tests/port/test.py
[modify] https://crrev.com/30e38a4446035a41c16ddd36727557ce8bbbd320/third_party/blink/tools/blinkpy/web_tests/update_expectations_unittest.py

The NextAction date has arrived: 2018-07-19

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

commit bd8cc342d976b3d6232d3dc34856059373c6d3fc
Author: Ned Nguyen <nednguyen@google.com>
Date: Thu Jul 19 16:04:29 2018

Remove references to NEEDSMANUALREBASELINE in markdown docs and testing/scripts/common.py file

NOTRY=true  # chromedriver_py_tests flake ( https://crbug.com/864205 )

Bug: 621126
Change-Id: I61d2722044cffc44e5bc93b5ffc7ff9d3ab6d313
Reviewed-on: https://chromium-review.googlesource.com/1142359
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576513}
[modify] https://crrev.com/bd8cc342d976b3d6232d3dc34856059373c6d3fc/docs/testing/json_test_results_format.md
[modify] https://crrev.com/bd8cc342d976b3d6232d3dc34856059373c6d3fc/docs/testing/layout_test_expectations.md
[modify] https://crrev.com/bd8cc342d976b3d6232d3dc34856059373c6d3fc/testing/scripts/common.py

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

commit d74062992950a5705ed578fe674cdf4a75e4408a
Author: Ned Nguyen <nednguyen@google.com>
Date: Tue Aug 07 22:23:33 2018

Remove support of the REBASELINE expectation from web tests

along with the "rebaseline-expectations" subcommand in blink_tool.py.

Also clean up the remaining references to NEEDS_MANUAL_REBASELINE left
from https://crrev.com/c/1140927.

Bug: 621126,  738152 
Change-Id: I33c109e4343e7bf41aac6e7527237702e4cce94a
Reviewed-on: https://chromium-review.googlesource.com/1164405
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581366}
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/WebKit/LayoutTests/fast/harness/results.html
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/tool/blink_tool.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/tool/commands/rebaseline.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/tool/commands/rebaseline_unittest.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/web_tests/layout_package/bot_test_expectations.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/web_tests/models/test_expectations.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/web_tests/models/test_expectations_unittest.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/web_tests/models/test_run_results_unittest.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/web_tests/update_expectations.py
[modify] https://crrev.com/d74062992950a5705ed578fe674cdf4a75e4408a/third_party/blink/tools/blinkpy/web_tests/update_expectations_unittest.py

> I think at this point we shouldn't be supporting NeedsManualRebaseline at
> all, because I don't think there's an excuse for why you'd rebaseline things
> after the fact (rather than before a CL landed).

Sometimes, a change in V8 causes Blink layout test expectations to require rebaselining. However, since V8 lives outside of the Chromium repository, and is merely auto-rolled into Chromium DEPS, there needs to be a way to mark such a test as "NeedsManualRebaseline" (or something similar).

I don't mind if NeedsManualRebaseline is no longer what we're supposed to use, but then please document what we should be using instead. (Existing docs still point to NeedsManualRebaseline.) Is it `[ Failure Pass ]`?

@mathias - is it feasible to address that situation by doing a manual roll of v8 that acts as any other change to blink would (i.e., run all of the blink try jobs to update baselines, run rebaseline-cl, etc.)?

This is perhaps obvious, but we should be trying to avoid rolling in changes that causes tests to fail; that's the whole reason for this bug :).

Re #18: NeedsManualRebaseline is almost no better than Skip. A test that NeedsManualRebaseline still runs on the bots but the result is completely ignored (where). If NeedsManualRebaseline is used as a stopgap for the roll, then I would suggest simply using Skip instead ([Failure Pass] might not be enough, if the test starts to timeout or crash because of the roll).

That said, skipping the tests is definitely not ideal (though not worse than NeedsManualRebaseline I think). We should discuss some long-term plans like integrating with rebaseline-cl suggested by dpranke.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/9169c95308ee6d991680ec59e064e287f1f6877d

commit 9169c95308ee6d991680ec59e064e287f1f6877d
Author: Nghia Nguyen <nednguyen@google.com>
Date: Fri Nov 02 16:51:51 2018

Remove references to REBASELINE results type

Bug:621126
Change-Id: I9e12fdbf3ec50f96d3a3301bbe872cd02ebefd39
Reviewed-on: https://chromium-review.googlesource.com/c/1315507
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/9169c95308ee6d991680ec59e064e287f1f6877d/scripts/slave/recipe_modules/test_utils/util.py

We still have some stale references of NeedsManualRebaseline in few places:
https://cs.chromium.org/search/?q=NeedsManualRebaseline&type=cs

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/9716603f9736651d54256d1c80505d9c24668cac

commit 9716603f9736651d54256d1c80505d9c24668cac
Author: Ned Nguyen <nednguyen@google.com>
Date: Fri Jan 11 17:14:06 2019

Update skia docs to stop referring to NeedsManualRebaseline

No-Try: true
Docs-Preview: https://skia.org/?cl=179782
Bug: chromium:621126
Change-Id: Iee8d6d86a5cda31d84d4ebe708c7f6fd9f80f78b
Reviewed-on: https://skia-review.googlesource.com/c/179782
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

[modify] https://crrev.com/9716603f9736651d54256d1c80505d9c24668cac/site/dev/chrome/blink.md
[modify] https://crrev.com/9716603f9736651d54256d1c80505d9c24668cac/site/dev/sheriffing/index.md

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

commit 879fcd202fbb31d8c885e69e297a703bba73497f
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Jan 11 19:39:11 2019

Roll src/third_party/skia cb42fc0dc56f..9716603f9736 (10 commits)

https://skia.googlesource.com/skia.git/+log/cb42fc0dc56f..9716603f9736


git log cb42fc0dc56f..9716603f9736 --date=short --no-merges --format='%ad %ae %s'
2019-01-11 nednguyen@google.com Update skia docs to stop referring to NeedsManualRebaseline
2019-01-11 fmalita@chromium.org Hack up SkShaper to pass additional callback info
2019-01-11 recipe-roller@chromium.org Roll recipe dependencies (trivial).
2019-01-11 ethannicholas@google.com fixed SPIR-V decoration
2019-01-11 jvanverth@google.com Revert "Add target SkColorSpace to SkImage_GpuYUVA."
2019-01-11 robertphillips@google.com Reduce reliance on the SkImage_Gpu's GrContext pointer
2019-01-11 benjaminwagner@google.com Omit another gm for IntelIris655 vk.
2019-01-11 jvanverth@google.com Add target SkColorSpace to SkImage_GpuYUVA.
2019-01-11 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/angle2 18af9a5a50c6..92ee1f9f3032 (1 commits)
2019-01-11 ethannicholas@google.com Fixed GPU perlin noise shaders under rotation.


Created with:
  gclient setdep -r src/third_party/skia@9716603f9736

The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:621126
TBR=bsalomon@chromium.org

Change-Id: Ic2fe98f2713fec5d3914ac2a13589999b9617e13
Reviewed-on: https://chromium-review.googlesource.com/c/1406959
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#622097}
[modify] https://crrev.com/879fcd202fbb31d8c885e69e297a703bba73497f/DEPS