Revert of In rebaseline.py, update SCM and expectations all at once after all commands.

third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py b/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
index deebf32b1c3007b64acd24a572c224068ddf8bc4..78d3011ce956a8cf498a29b47acf6128c0065aee 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/tool/commands/rebaseline.py
@@ -87,42 +87,29 @@ class ChangeSet(object):
     TODO(qyearsley): Refactor this so that lines to remove are only tracked in one format.
     """
     def __init__(self, files_to_add=None, files_to_delete=None, lines_to_remove=None):
-        self._files_to_add = set(files_to_add or [])
-        self._files_to_delete = set(files_to_delete or [])
-        lines_to_remove = lines_to_remove or {}
-        self._lines_to_remove = {t: set(builders) for t, builders in lines_to_remove.iteritems()}
+        self.files_to_add = files_to_add or []
+        self.files_to_delete = files_to_delete or []
+        self.lines_to_remove = lines_to_remove or {}
 
     def add_file(self, path):
-        self._files_to_add.add(path)
+        self.files_to_add.append(path)
 
     def delete_file(self, path):
-        self._files_to_delete.add(path)
+        self.files_to_delete.append(path)
 
     def remove_line(self, test, builder):
-        if test not in self._lines_to_remove:
-            self._lines_to_remove[test] = set()
-        self._lines_to_remove[test].add(builder)
-
-    @property
-    def files_to_add(self):
-        return sorted(self._files_to_add - self._files_to_delete)
-
-    @property
-    def files_to_delete(self):
-        return sorted(self._files_to_delete - self._files_to_add)
-
-    @property
-    def lines_to_remove(self):
-        return {t: sorted(set(builders)) for t, builders in sorted(self._lines_to_remove.iteritems())}
+        if test not in self.lines_to_remove:
+            self.lines_to_remove[test] = []
+        self.lines_to_remove[test].append(builder)
 
     def to_dict(self):
         remove_lines = []
-        for test in sorted(self.lines_to_remove):
-            for builder in sorted(set(self.lines_to_remove[test])):
+        for test in self.lines_to_remove:
+            for builder in self.lines_to_remove[test]:
                 remove_lines.append({'test': test, 'builder': builder})
         return {
-            'add': self.files_to_add,
-            'delete': self.files_to_delete,
+            'add': list(self.files_to_add),
+            'delete': list(self.files_to_delete),
             'remove-lines': remove_lines,
         }
 
@@ -143,18 +130,19 @@ class ChangeSet(object):
                     lines_to_remove[test] = []
                 lines_to_remove[test].append(builder)
         return ChangeSet(
-            files_to_add=files_to_add,
-            files_to_delete=files_to_delete,
+            files_to_add=list(files_to_add),
+            files_to_delete=list(files_to_delete),
             lines_to_remove=lines_to_remove)
 
     def update(self, other):
         assert isinstance(other, ChangeSet)
-        self._files_to_add.update(other.files_to_add)
-        self._files_to_delete.update(other.files_to_delete)
+        assert type(other.lines_to_remove) is dict
+        self.files_to_add.extend(other.files_to_add)
+        self.files_to_delete.extend(other.files_to_delete)
         for test in other.lines_to_remove:
-            if test not in self._lines_to_remove:
-                self._lines_to_remove[test] = set()
-            self._lines_to_remove[test].update(other.lines_to_remove[test])
+            if test not in self.lines_to_remove:
+                self.lines_to_remove[test] = []
+            self.lines_to_remove[test].extend(other.lines_to_remove[test])
 
 
 class BaseInternalRebaselineCommand(AbstractRebaseliningCommand):
@@ -499,18 +487,28 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
         return (SKIP in full_expectations.get_expectations(test) and
                 SKIP not in generic_expectations.get_expectations(test))
 
-    def _run_in_parallel(self, commands):
+    def _run_in_parallel(self, commands, update_scm=True):
         if not commands:
-            return ChangeSet()
+            return {}
 
         command_results = self._tool.executive.run_in_parallel(commands)
         for _, _, stderr in command_results:
             if stderr:
                 _log.error(stderr)
 
-        return self._extract_scm_changes(command_results)
+        change_set = self._extract_scm_changes(command_results)
+
+        # TODO(qyearsley): Instead of updating the SCM state here, aggregate changes
+        # and update once in _rebaseline. See http://crbug.com/639410.
+        if update_scm:
+            if change_set.files_to_delete:
+                self._tool.scm().delete_list(change_set.files_to_delete)
+            if change_set.files_to_add:
+                self._tool.scm().add_list(change_set.files_to_add)
 
-    def _rebaseline(self, options, test_prefix_list):
+        return change_set.lines_to_remove
+
+    def _rebaseline(self, options, test_prefix_list, update_scm=True):
         """Downloads new baselines in parallel, then updates expectations files
         and optimizes baselines.
 
@@ -528,6 +526,7 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
                 "some/other.html" but only from builder-1.
                 TODO(qyearsley): Replace test_prefix_list everywhere with some
                 sort of class that contains the same data.
+            update_scm: If True, commands like `git add` and `git rm` will be run.
         """
         for test, builds_to_check in sorted(test_prefix_list.items()):
             _log.info("Rebaselining %s", test)
@@ -536,25 +535,25 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
 
         copy_baseline_commands, rebaseline_commands, extra_lines_to_remove = self._rebaseline_commands(
             test_prefix_list, options)
+        lines_to_remove = {}
 
-        change_set = ChangeSet(lines_to_remove=extra_lines_to_remove)
+        self._run_in_parallel(copy_baseline_commands, update_scm=update_scm)
+        lines_to_remove = self._run_in_parallel(rebaseline_commands, update_scm=update_scm)
 
-        change_set.update(self._run_in_parallel(copy_baseline_commands))
-        change_set.update(self._run_in_parallel(rebaseline_commands))
+        for test in extra_lines_to_remove:
+            if test in lines_to_remove:
+                lines_to_remove[test] = lines_to_remove[test] + extra_lines_to_remove[test]
+            else:
+                lines_to_remove[test] = extra_lines_to_remove[test]
 
-        if change_set.lines_to_remove:
-            self._update_expectations_files(change_set.lines_to_remove)
+        if lines_to_remove:
+            self._update_expectations_files(lines_to_remove)
 
         if options.optimize:
             # TODO(wkorman): Consider changing temporary branch to base off of HEAD rather than
             # origin/master to ensure we run baseline optimization processes with the same code as
             # auto-rebaseline itself.
-            change_set.update(self._run_in_parallel(self._optimize_baselines(test_prefix_list, options.verbose)))
-
-        if change_set.files_to_delete:
-            self._tool.scm().delete_list(change_set.files_to_delete)
-        if change_set.files_to_add:
-            self._tool.scm().add_list(change_set.files_to_add)
+            self._run_in_parallel(self._optimize_baselines(test_prefix_list, options.verbose), update_scm=update_scm)
 
     def _suffixes_for_actual_failures(self, test, build, existing_suffixes):
         """Gets the baseline suffixes for actual mismatch failures in some results.