[pitivi] Do not commit timeline during splitting section

[Alexandru Băluț <alexbalut src gnome org>]

commit d32d027649065381a1ef5d18000c26b927c4bb19
Author: Thibault Saunier <tsaunier igalia com>
Date:   Sat Sep 29 11:05:02 2018 -0300

    Do not commit timeline during splitting section
    
    If we commit a timeline while splitting clips we might end up commiting
    "half splited" clips. This happens because while splitting, the keyframe
    are moved and `KeyframeCurve.__controlSourceChangedCb` is called.
    
    Introduce a context manager on the pipeline so that we ensure that
    a section of code is commited atomically.
    
    Fixes #2251

 pitivi/timeline/timeline.py | 30 +++++++++++++++++-------------
 pitivi/utils/pipeline.py    | 14 +++++++++++++-
 tests/test_pipeline.py      | 22 ++++++++++++++++++++++
 3 files changed, 52 insertions(+), 14 deletions(-)
---
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index 834769be..ae8353e9 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -1836,19 +1836,23 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
 
         position = self._project.pipeline.getPosition()
         splitted = False
-        for clip in clips:
-            start = clip.get_start()
-            end = start + clip.get_duration()
-            if start < position and end > position:
-                clip.get_layer().splitting_object = True
-
-                self.app.write_action("split-clip",
-                    clip_name=clip.get_name(),
-                    position=float(position / Gst.SECOND))
-
-                clip.split(position)
-                clip.get_layer().splitting_object = False
-                splitted = True
+
+        with self._project.pipeline.commit_timeline_after():
+            for clip in clips:
+                start = clip.get_start()
+                end = start + clip.get_duration()
+                if start < position and end > position:
+                    layer = clip.get_layer()
+                    layer.splitting_object = True
+                    try:
+                        self.app.write_action("split-clip",
+                            clip_name=clip.get_name(),
+                            position=float(position / Gst.SECOND))
+
+                        clip.split(position)
+                        splitted = True
+                    finally:
+                        layer.splitting_object = False
 
         if not splitted and splitting_selection:
             self._splitElements()
diff --git a/pitivi/utils/pipeline.py b/pitivi/utils/pipeline.py
index e5a0b05c..fe008139 100644
--- a/pitivi/utils/pipeline.py
+++ b/pitivi/utils/pipeline.py
@@ -19,6 +19,7 @@
 # the Free Software Foundation; either version 3, or (at your option)
 # any later version.
 """High-level pipelines."""
+import contextlib
 import os
 
 from gi.repository import GES
@@ -533,6 +534,7 @@ class Pipeline(GES.Pipeline, SimplePipeline):
 
         self._was_empty = False
         self._commit_wanted = False
+        self._prevent_commits = 0
 
         if "watchdog" in os.environ.get("PITIVI_UNSTABLE_FEATURES", ''):
             watchdog = Gst.ElementFactory.make("watchdog", None)
@@ -623,8 +625,18 @@ class Pipeline(GES.Pipeline, SimplePipeline):
         else:
             SimplePipeline._busMessageCb(self, bus, message)
 
+    @contextlib.contextmanager
+    def commit_timeline_after(self):
+        self._prevent_commits += 1
+        self.info("Disabling commits during action execution")
+        try:
+            yield
+        finally:
+            self._prevent_commits -= 1
+            self.commit_timeline()
+
     def commit_timeline(self):
-        if self.getState() == Gst.State.NULL:
+        if self._prevent_commits > 0 or self.getState() == Gst.State.NULL:
             # No need to commit. NLE will do it automatically when
             # changing state from READY to PAUSED.
             return
diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py
index e9d44255..ee235295 100644
--- a/tests/test_pipeline.py
+++ b/tests/test_pipeline.py
@@ -133,3 +133,25 @@ class TestPipeline(common.TestCase):
         self.assertEqual(pipe._recovery_state, SimplePipeline.RecoveryState.NOT_RECOVERING)
         self.assertFalse(pipe._busy_async)
         self.assertIsNone(pipe._next_seek)
+
+    def test_commit_timeline_after(self):
+        """Checks the recovery mechanism."""
+        pipe = Pipeline(common.create_pitivi_mock())
+        timeline = GES.Timeline()
+        pipe.set_timeline(timeline)
+
+        with mock.patch.object(pipe, "getState") as get_state:
+            get_state.return_value = (0, Gst.State.PAUSED, 0)
+            with mock.patch.object(timeline, "commit") as commit:
+                with pipe.commit_timeline_after():
+                    pipe.commit_timeline()
+                self.assertEqual(commit.call_count, 1)
+
+            with mock.patch.object(timeline, "commit") as commit:
+                with pipe.commit_timeline_after():
+                    self.assertEqual(pipe._prevent_commits, 1)
+                    with pipe.commit_timeline_after():
+                        self.assertEqual(pipe._prevent_commits, 2)
+                        pipe.commit_timeline()
+                        self.assertEqual(commit.call_count, 0)
+                self.assertEqual(commit.call_count, 1)