Revert of Make Rebaseline/NeedsRebaseline/NeedsManualRebaseline not conflict...

Revert of Make Rebaseline/NeedsRebaseline/NeedsManualRebaseline not conflict with Pass lines. (patchset #2 id:20001 of https://codereview.chromium.org/1412533002/ )

Reason for revert:
Interim fix for http://crbug.com/569175 until we can update script to not strip Pass/Failure lines for tests also matching NeedsRebaseline.

Original issue's description:
> Make Rebaseline/NeedsRebaseline/NeedsManualRebaseline not conflict with Pass lines.
>
> As per https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/OI_gXMX7yUs.
>
> Committed: https://crrev.com/30fe1b01a50cefa35a0578311944e3b09cb27f38
> Cr-Commit-Position: refs/heads/master@{#357741}

TBR=dpranke@chromium.org,ojan@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=569175

Review URL: https://codereview.chromium.org/1550703002

Cr-Commit-Position: refs/heads/master@{#366947}

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py

@@ -679,27 +679,17 @@ class TestExpectationsModel(object):
 
     def add_expectation_line(self, expectation_line,
                              model_all_expectations=False):
+        """Returns a list of warnings encountered while matching specifiers."""
+
         if expectation_line.is_invalid():
             return
 
         for test in expectation_line.matching_tests:
-            lines_involve_rebaseline = False
-            prev_expectation_line = self.get_expectation_line(test)
-
-            if prev_expectation_line:
-                # The previous path matched more of the test.
-                if len(prev_expectation_line.path) > len(expectation_line.path):
-                    continue
-
-                if self._lines_conflict(prev_expectation_line, expectation_line):
-                    continue
-
-                lines_involve_rebaseline = self._expects_rebaseline(prev_expectation_line) or self._expects_rebaseline(expectation_line)
+            if self._already_seen_better_match(test, expectation_line):
+                continue
 
-            # Exact path matches that conflict should be merged, e.g.
-            # [ Pass Timeout ] + [ NeedsRebaseline ] ==> [ Pass Timeout NeedsRebaseline ].
-            if model_all_expectations or lines_involve_rebaseline:
-                expectation_line = TestExpectationLine.merge_expectation_lines(prev_expectation_line, expectation_line, model_all_expectations)
+            if model_all_expectations:
+                expectation_line = TestExpectationLine.merge_expectation_lines(self.get_expectation_line(test), expectation_line, model_all_expectations)
 
             self._clear_expectations_for_test(test)
             self._test_to_expectation_line[test] = expectation_line
@@ -753,20 +743,41 @@ class TestExpectationsModel(object):
             if test in set_of_tests:
                 set_of_tests.remove(test)
 
-    def _expects_rebaseline(self, expectation_line):
-        expectations = expectation_line.parsed_expectations
-        return REBASELINE in expectations or NEEDS_REBASELINE in expectations or NEEDS_MANUAL_REBASELINE in expectations
+    def _already_seen_better_match(self, test, expectation_line):
+        """Returns whether we've seen a better match already in the file.
 
-    def _lines_conflict(self, prev_expectation_line, expectation_line):
-        if prev_expectation_line.path != expectation_line.path:
+        Returns True if we've already seen a expectation_line.name that matches more of the test
+            than this path does
+        """
+        # FIXME: See comment below about matching test configs and specificity.
+        if not self.has_test(test):
+            # We've never seen this test before.
             return False
 
-        if PASS in expectation_line.parsed_expectations and self._expects_rebaseline(prev_expectation_line):
+        prev_expectation_line = self._test_to_expectation_line[test]
+
+        if prev_expectation_line.filename != expectation_line.filename:
+            # We've moved on to a new expectation file, which overrides older ones.
             return False
 
-        if PASS in prev_expectation_line.parsed_expectations and self._expects_rebaseline(expectation_line):
+        if len(prev_expectation_line.path) > len(expectation_line.path):
+            # The previous path matched more of the test.
+            return True
+
+        if len(prev_expectation_line.path) < len(expectation_line.path):
+            # This path matches more of the test.
             return False
 
+        # At this point we know we have seen a previous exact match on this
+        # base path, so we need to check the two sets of specifiers.
+
+        # FIXME: This code was originally designed to allow lines that matched
+        # more specifiers to override lines that matched fewer specifiers.
+        # However, we currently view these as errors.
+        #
+        # To use the "more specifiers wins" policy, change the errors for overrides
+        # to be warnings and return False".
+
         if prev_expectation_line.matching_configurations == expectation_line.matching_configurations:
             expectation_line.warnings.append('Duplicate or ambiguous entry lines %s:%s and %s:%s.' % (
                 self._shorten_filename(prev_expectation_line.filename), prev_expectation_line.line_numbers,

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py

@@ -480,10 +480,6 @@ class SemanticTests(Base):
 Bug(exp) failures/expected/text.html [ Failure ]
 Bug(exp) failures/expected/text.html [ Timeout ]""", is_lint_mode=True)
 
-        self.assertRaises(ParseError, self.parse_exp, """
-Bug(exp) failures/expected/text.html [ Failure ]
-Bug(exp) failures/expected/text.html [ NeedsRebaseline ]""", is_lint_mode=True)
-
         self.assertRaises(ParseError, self.parse_exp,
             self.get_basic_expectations(), overrides="""
 Bug(override) failures/expected/text.html [ Failure ]
@@ -495,51 +491,6 @@ Bug(exp) [ Release ] failures/expected/text.html [ Failure ]
 Bug(exp) [ Debug ] failures/expected/text.html [ Failure ]
 """)
 
-    def test_duplicates_rebaseline_no_conflict(self):
-        # Rebaseline throws in lint mode, so only test it in non-lint mode.
-        # Check Rebaseline
-        self.parse_exp("""
-Bug(exp) failures/expected/text.html [ Rebaseline ]
-Bug(exp) failures/expected/text.html [ Pass Failure ]
-""", is_lint_mode=False)
-        self.assert_exp_list('failures/expected/text.html', [PASS, FAIL, REBASELINE])
-
-        # Check NeedsRebaseline.
-        self.parse_exp("""
-Bug(exp) failures/expected/text.html [ Pass Failure ]
-Bug(exp) failures/expected/text.html [ NeedsRebaseline ]
-""", is_lint_mode=False)
-        self.assert_exp_list('failures/expected/text.html', [PASS, FAIL, NEEDS_REBASELINE])
-
-        # Check NeedsManualRebaseline
-        self.parse_exp("""
-Bug(exp) failures/expected/text.html [ Pass Failure ]
-Bug(exp) failures/expected/text.html [ NeedsManualRebaseline ]
-""", is_lint_mode=False)
-        self.assert_exp_list('failures/expected/text.html', [PASS, FAIL, NEEDS_MANUAL_REBASELINE])
-
-    def test_duplicates_rebaseline_no_conflict_linting(self):
-        # Check NeedsRebaseline
-        self.parse_exp("""
-Bug(exp) failures/expected/text.html [ NeedsRebaseline ]
-Bug(exp) failures/expected/text.html [ Pass Failure ]
-""", is_lint_mode=True)
-        self.assert_exp_list('failures/expected/text.html', [PASS, FAIL, NEEDS_REBASELINE])
-
-        # Check lines in reverse order.
-        self.parse_exp("""
-Bug(exp) failures/expected/text.html [ Pass Failure ]
-Bug(exp) failures/expected/text.html [ NeedsRebaseline ]
-""", is_lint_mode=True)
-        self.assert_exp_list('failures/expected/text.html', [PASS, FAIL, NEEDS_REBASELINE])
-
-        # Check NeedsManualRebaseline
-        self.parse_exp("""
-Bug(exp) failures/expected/text.html [ Pass Failure ]
-Bug(exp) failures/expected/text.html [ NeedsManualRebaseline ]
-""", is_lint_mode=True)
-        self.assert_exp_list('failures/expected/text.html', [PASS, FAIL, NEEDS_MANUAL_REBASELINE])
-
     def test_missing_file(self):
         self.parse_exp('Bug(test) missing_file.html [ Failure ]')
         self.assertTrue(self._exp.has_warnings(), 1)