[pitivi] undo: Start the action log and the observers only when the project is loaded

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

commit 8eeb2be6f839ff4a91a6d5e07999316a296bdc37
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Thu Apr 7 21:36:40 2016 +0200

    undo: Start the action log and the observers only when the project is loaded
    
    By recreating the undo/redo manager for each project, we don't have
    to worry about resetting it when a new project is created or loaded.
    
    This change reflects more the idea that an undo/redo manager and the
    associated observers are per project.
    
    Differential Revision: https://phabricator.freedesktop.org/D860

 pitivi/application.py       |   56 ++++++++++++++++++++----------------------
 pitivi/effects.py           |    5 +--
 pitivi/mainwindow.py        |    9 +------
 pitivi/titleeditor.py       |    5 +--
 pitivi/undo/project.py      |   23 ++++++++---------
 pitivi/undo/timeline.py     |   32 +++++++++---------------
 pitivi/undo/undo.py         |   16 +++---------
 tests/Makefile.am           |    1 +
 tests/test_undo_project.py  |   49 +++++++++++++++++++++++++++++++++++++
 tests/test_undo_timeline.py |   26 +++++++------------
 10 files changed, 119 insertions(+), 103 deletions(-)
---
diff --git a/pitivi/application.py b/pitivi/application.py
index 7df353e..285ca5e 100644
--- a/pitivi/application.py
+++ b/pitivi/application.py
@@ -39,8 +39,8 @@ from pitivi.project import ProjectManager
 from pitivi.settings import get_dir
 from pitivi.settings import GlobalSettings
 from pitivi.settings import xdg_cache_home
-from pitivi.undo.project import ProjectLogObserver
-from pitivi.undo.timeline import TimelineLogObserver
+from pitivi.undo.project import ProjectObserver
+from pitivi.undo.timeline import TimelineObserver
 from pitivi.undo.undo import UndoableActionLog
 from pitivi.utils import loggable
 from pitivi.utils.loggable import Loggable
@@ -58,6 +58,7 @@ class Pitivi(Gtk.Application, Loggable):
     Pitivi's application.
 
     Attributes:
+        action_log (UndoableActionLog): The undo/redo log for the current project.
         effects (EffectsManager): The effects which can be applied to a clip.
         gui (PitiviMainWindow): The main window of the app.
         project_manager (ProjectManager): The holder of the current project.
@@ -80,9 +81,7 @@ class Pitivi(Gtk.Application, Loggable):
         self.system = None
         self.project_manager = ProjectManager(self)
 
-        self.action_log = UndoableActionLog(self)
-        self.timeline_log_observer = None
-        self.project_log_observer = None
+        self.action_log = None
         self._last_action_time = Gst.util_get_timestamp()
 
         self.gui = None
@@ -142,12 +141,6 @@ class Pitivi(Gtk.Application, Loggable):
         self.proxy_manager = ProxyManager(self)
         self.system = getSystem()
 
-        self.action_log.connect("commit", self._actionLogCommit)
-        self.action_log.connect("undo", self._actionLogUndo)
-        self.action_log.connect("redo", self._actionLogRedo)
-        self.timeline_log_observer = TimelineLogObserver(self.action_log)
-        self.project_log_observer = ProjectLogObserver(self.action_log)
-
         self.project_manager.connect(
             "new-project-loading", self._newProjectLoadingCb)
         self.project_manager.connect(
@@ -155,6 +148,8 @@ class Pitivi(Gtk.Application, Loggable):
         self.project_manager.connect("project-closed", self._projectClosed)
 
         self._createActions()
+        self._syncDoUndo()
+
         self._checkVersion()
 
     def _createActions(self):
@@ -268,16 +263,21 @@ class Pitivi(Gtk.Application, Loggable):
         self._setScenarioFile(project.get_uri())
 
     def _newProjectLoaded(self, unused_project_manager, project):
-        self.action_log.clean()
-        self._syncDoUndo(self.action_log)
+        self.action_log = UndoableActionLog(self)
+        self.action_log.connect("commit", self._actionLogCommit)
+        self.action_log.connect("undo", self._actionLogUndo)
+        self.action_log.connect("redo", self._actionLogRedo)
+
+        timeline_observer = TimelineObserver(self.action_log)
+        timeline_observer.startObserving(project.timeline)
 
-        self.timeline_log_observer.startObserving(project.timeline)
-        self.project_log_observer.startObserving(project)
+        project_observer = ProjectObserver(self.action_log)
+        project_observer.startObserving(project)
 
     def _projectClosed(self, unused_project_manager, project):
         if project.loaded:
-            self.project_log_observer.stopObserving(project)
-            self.timeline_log_observer.stopObserving(project.timeline)
+            self.action_log = None
+            self._syncDoUndo()
 
         if self._scenario_file:
             self.write_action("stop")
@@ -355,29 +355,27 @@ class Pitivi(Gtk.Application, Loggable):
     def _actionLogCommit(self, action_log, unused_stack):
         if action_log.is_in_transaction():
             return
-        self._syncDoUndo(action_log)
+        self._syncDoUndo()
 
     def _actionLogUndo(self, action_log, unused_stack):
-        self._syncDoUndo(action_log)
+        self._syncDoUndo()
 
     def _actionLogRedo(self, action_log, unused_stack):
-        self._syncDoUndo(action_log)
+        self._syncDoUndo()
 
-    def _syncDoUndo(self, action_log):
+    def _syncDoUndo(self):
+        can_undo = self.action_log and bool(self.action_log.undo_stacks)
         # TODO: Remove this once we revisit undo/redo T3360
-        can_undo = in_devel()
-
-        if can_undo:
-            can_undo = bool(action_log.undo_stacks)
-        self.undo_action.set_enabled(can_undo)
+        can_undo = can_undo and in_devel()
+        self.undo_action.set_enabled(bool(can_undo))
 
-        can_redo = bool(action_log.redo_stacks)
-        self.redo_action.set_enabled(can_redo)
+        can_redo = self.action_log and bool(self.action_log.redo_stacks)
+        self.redo_action.set_enabled(bool(can_redo))
 
         if not self.project_manager.current_project:
             return
 
-        dirty = action_log.dirty()
+        dirty = self.action_log and self.action_log.dirty()
         self.project_manager.current_project.setModificationState(dirty)
         # In the tests we do not want to create any gui
         if self.gui is not None:
diff --git a/pitivi/effects.py b/pitivi/effects.py
index b4b04bb..dfd3abb 100644
--- a/pitivi/effects.py
+++ b/pitivi/effects.py
@@ -545,7 +545,6 @@ class EffectsPropertiesManager:
     def __init__(self, app):
         self.cache_dict = {}
         self._current_element_values = {}
-        self.action_log = app.action_log
         self.app = app
 
     def getEffectConfigurationUI(self, effect):
@@ -594,9 +593,9 @@ class EffectsPropertiesManager:
             value = Gst.Fraction(int(value.num), int(value.denom))
 
         if value != self._current_element_values.get(prop.name):
-            self.action_log.begin("Effect property change")
+            self.app.action_log.begin("Effect property change")
             effect.set_child_property(prop.name, value)
-            self.action_log.commit()
+            self.app.action_log.commit()
 
             self.app.project_manager.current_project.pipeline.flushSeek()
             self._current_element_values[prop.name] = value
diff --git a/pitivi/mainwindow.py b/pitivi/mainwindow.py
index 039bc83..c9124b3 100644
--- a/pitivi/mainwindow.py
+++ b/pitivi/mainwindow.py
@@ -721,8 +721,6 @@ class PitiviMainWindow(Gtk.ApplicationWindow, Loggable):
         project.pipeline.activatePositionListener()
         self._setProject(project)
 
-        # FIXME GES we should re-enable this when possible
-        # self._syncDoUndo(self.app.action_log)
         self.updateTitle()
 
         if project_manager.disable_save is True:
@@ -757,7 +755,6 @@ class PitiviMainWindow(Gtk.ApplicationWindow, Loggable):
     def _projectManagerProjectSavedCb(self, unused_project_manager, project, uri):
         # FIXME GES: Reimplement Undo/Redo
         # self.app.action_log.checkpoint()
-        # self._syncDoUndo(self.app.action_log)
         self.updateTitle()
 
         self.save_action.set_enabled(False)
@@ -1057,14 +1054,10 @@ class PitiviMainWindow(Gtk.ApplicationWindow, Loggable):
             "rendering-settings-changed", self._renderingSettingsChangedCb)
 
         self.viewer.setPipeline(project.pipeline)
-        self.app.timeline_log_observer.pipeline = project.pipeline
         self._renderingSettingsChangedCb(project)
         self.clipconfig.project = project
-        # FIXME GES port undo/redo
-        # self.app.timelineLogObserver.pipeline = project.pipeline
 
-        # When creating a blank project, medialibrary will eventually trigger
-        # this _setProject method, but there's no project URI yet.
+        # When creating a blank project there's no project URI yet.
         if project.uri:
             folder_path = os.path.dirname(path_from_uri(project.uri))
             self.settings.lastProjectFolder = folder_path
diff --git a/pitivi/titleeditor.py b/pitivi/titleeditor.py
index d1fe508..d3812fc 100644
--- a/pitivi/titleeditor.py
+++ b/pitivi/titleeditor.py
@@ -52,7 +52,6 @@ class TitleEditor(Loggable):
     def __init__(self, app):
         Loggable.__init__(self)
         self.app = app
-        self.action_log = app.action_log
         self.settings = {}
         self.source = None
         self._project = None
@@ -104,13 +103,13 @@ class TitleEditor(Loggable):
             self.settings["halignment"].append(en, n)
 
     def _setChildProperty(self, name, value):
-        self.action_log.begin("Title %s change" % name)
+        self.app.action_log.begin("Title %s change" % name)
         self._setting_props = True
         try:
             assert self.source.set_child_property(name, value)
         finally:
             self._setting_props = False
-        self.action_log.commit()
+        self.app.action_log.commit()
 
     def _backgroundColorButtonCb(self, widget):
         color = gdk_rgba_to_argb(widget.get_rgba())
diff --git a/pitivi/undo/project.py b/pitivi/undo/project.py
index 381977b..06f7805 100644
--- a/pitivi/undo/project.py
+++ b/pitivi/undo/project.py
@@ -83,27 +83,26 @@ class ProjectSettingsChanged(UndoableAction):
         self._undone()
 
 
-class ProjectLogObserver(UndoableAction):
+class ProjectObserver():
+    """Monitors a project instance and reports UndoableActions.
+
+    Attributes:
+        log (UndoableActionLog): The action log where to report actions.
+    """
 
     def __init__(self, log):
-        UndoableAction.__init__(self)
         self.log = log
 
     def startObserving(self, project):
+        """Starts monitoring the specified Project.
+
+        Args:
+            project (Project): The project to be monitored.
+        """
         project.connect("notify-meta", self._settingsChangedCb)
         project.connect("asset-added", self._assetAddedCb)
         project.connect("asset-removed", self._assetRemovedCb)
 
-    def stopObserving(self, project):
-        try:
-            project.disconnect_by_func(self._settingsChangedCb)
-            project.disconnect_by_func(self._assetAddedCb)
-            project.disconnect_by_func(self._assetRemovedCb)
-        except Exception:
-            # This can happen when we interrupt the loading of a project,
-            # such as in mainwindow's _projectManagerMissingUriCb
-            pass
-
     def _settingsChangedCb(self, project, item, value):
         """
         FIXME Renable undo/redo
diff --git a/pitivi/undo/timeline.py b/pitivi/undo/timeline.py
index b8083a0..3606a6b 100644
--- a/pitivi/undo/timeline.py
+++ b/pitivi/undo/timeline.py
@@ -81,7 +81,6 @@ class TrackElementChildPropertyTracker(Loggable):
         Loggable.__init__(self)
         self._tracked_track_elements = {}
         self.action_log = action_log
-        self.pipeline = None
 
     def addTrackElement(self, track_element):
         if track_element in self._tracked_track_elements:
@@ -516,7 +515,13 @@ class ActivePropertyChanged(UndoableAction):
         self._undone()
 
 
-class TimelineLogObserver(Loggable):
+class TimelineObserver(Loggable):
+    """Monitors a project's timeline and reports UndoableActions.
+
+    Attributes:
+        log (UndoableActionLog): The action log where to report actions.
+    """
+
     timelinePropertyChangedAction = ClipPropertyChanged
     activePropertyChangedAction = ActivePropertyChanged
 
@@ -527,31 +532,18 @@ class TimelineLogObserver(Loggable):
         self.clip_property_trackers = {}
         self.control_source_keyframe_trackers = {}
         self.children_props_tracker = TrackElementChildPropertyTracker(log)
-        self._pipeline = None
-
-    @property
-    def pipeline(self):
-        return self._pipeline
-
-    @pipeline.setter
-    def pipeline(self, pipeline):
-        self._pipeline = pipeline
-        self.children_props_tracker.pipeline = pipeline
 
     def startObserving(self, timeline):
+        """Starts monitoring the specified Timeline.
+
+        Args:
+            timeline (GES.Timeline): The timeline to be monitored.
+        """
         self._connectToTimeline(timeline)
         for layer in timeline.get_layers():
             for clip in layer.get_clips():
                 self._connectToClip(clip)
 
-    def stopObserving(self, timeline):
-        self._disconnectFromTimeline(timeline)
-        for layer in timeline.layers:
-            for clip in layer.get_clips():
-                self._disconnectFromClip(clip)
-                for track_element in clip.get_children(True):
-                    self._disconnectFromTrackElement(track_element)
-
     def _connectToTimeline(self, timeline):
         for layer in timeline.get_layers():
             layer.connect("clip-added", self._clipAddedCb)
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index ae28a9b..690becc 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -29,19 +29,16 @@ from pitivi.utils.loggable import Loggable
 
 
 class UndoError(Exception):
-
     """ Any exception related to the undo/redo feature."""
     pass
 
 
 class UndoWrongStateError(UndoError):
-
     """ Exception related to the current state of the undo/redo stack. """
     pass
 
 
 class UndoableAction(GObject.Object, Loggable):
-
     """
     An action that can be undone.
     In other words, when your object's state changes, create an UndoableAction
@@ -83,7 +80,6 @@ class FinalizingAction:
 
 
 class UndoableActionStack(UndoableAction):
-
     """
     Simply a stack of UndoableAction objects.
     """
@@ -121,11 +117,12 @@ class UndoableActionStack(UndoableAction):
 
 
 class UndoableActionLog(GObject.Object, Loggable):
-
     """
-    This is the "master" class that handles all the undo/redo system. There is
-    only one instance of it in Pitivi: application.py's "action_log" property.
+    The undo/redo manager.
+
+    A separate instance should be created for each Project instance.
     """
+
     __gsignals__ = {
         "begin": (GObject.SIGNAL_RUN_LAST, None, (object,)),
         "push": (GObject.SIGNAL_RUN_LAST, None, (object, object)),
@@ -236,10 +233,6 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.undo_stacks.append(stack)
         self.emit("redo", stack)
 
-    def clean(self):
-        self.redo_stacks = []
-        self.undo_stacks = []
-
     def _takeSnapshot(self):
         return list(self.undo_stacks)
 
@@ -276,7 +269,6 @@ class UndoableActionLog(GObject.Object, Loggable):
 
 
 class PropertyChangeTracker(GObject.Object):
-
     """
     BaseClass to track a class property, Used for undo/redo
     """
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a3bbe25..4fd1946 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,6 +20,7 @@ tests =       \
        test_timeline_layer.py \
        test_timeline_timeline.py \
        test_undo.py \
+       test_undo_project.py \
        test_undo_timeline.py \
        test_utils.py \
        test_utils_timeline.py \
diff --git a/tests/test_undo_project.py b/tests/test_undo_project.py
new file mode 100644
index 0000000..b263136
--- /dev/null
+++ b/tests/test_undo_project.py
@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+#
+#       tests/test_undo_project.py
+#
+# Copyright (c) 2016, Alex Băluț <alexandru balut gmail com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the
+# Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+# Boston, MA 02110-1301, USA.
+from unittest import TestCase
+
+from pitivi.application import Pitivi
+from tests import common
+
+
+class TestProjectUndo(TestCase):
+
+    def setUp(self):
+        app = Pitivi()
+        app._startupCb(app)
+        self.assertTrue(app.project_manager.newBlankProject())
+
+        self.project = app.project_manager.current_project
+        self.action_log = app.action_log
+
+    def test_new_project_has_nothing_to_undo(self):
+        mainloop = common.create_main_loop()
+
+        def loaded_cb(project, timeline):
+            mainloop.quit()
+
+        self.project.connect_after("loaded", loaded_cb)
+
+        mainloop.run()
+
+        self.assertFalse(self.action_log.is_in_transaction())
+        self.assertFalse(self.action_log.undo_stacks)
+
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index 7f0d971..d35c3a6 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -28,36 +28,36 @@ from pitivi.application import Pitivi
 from pitivi.undo.timeline import ClipAdded
 from pitivi.undo.timeline import ClipPropertyChanged
 from pitivi.undo.timeline import ClipRemoved
-from pitivi.undo.timeline import TimelineLogObserver
+from pitivi.undo.timeline import TimelineObserver
 from pitivi.undo.timeline import TrackElementAdded
 from pitivi.undo.undo import UndoableActionLog
 from tests import common
 
 
-class TimelineLogObserverSpy(TimelineLogObserver):
+class TimelineObserverSpy(TimelineObserver):
 
     def _connectToTimeline(self, timeline):
-        TimelineLogObserver._connectToTimeline(self, timeline)
+        TimelineObserver._connectToTimeline(self, timeline)
         timeline.connected = True
 
     def _disconnectFromTimeline(self, timeline):
-        TimelineLogObserver._disconnectFromTimeline(self, timeline)
+        TimelineObserver._disconnectFromTimeline(self, timeline)
         timeline.connected = False
 
     def _connectToClip(self, clip):
-        TimelineLogObserver._connectToClip(self, clip)
+        TimelineObserver._connectToClip(self, clip)
         clip.connected = True
 
     def _disconnectFromClip(self, clip):
-        TimelineLogObserver._disconnectFromClip(self, clip)
+        TimelineObserver._disconnectFromClip(self, clip)
         clip.connected = False
 
     def _connectToTrackElement(self, track_element):
-        TimelineLogObserver._connectToTrackElement(self, track_element)
+        TimelineObserver._connectToTrackElement(self, track_element)
         track_element.connected = True
 
     def _disconnectFromTrackElement(self, track_element):
-        TimelineLogObserver._disconnectFromTrackElement(self, track_element)
+        TimelineObserver._disconnectFromTrackElement(self, track_element)
         track_element.connected = False
 
 
@@ -65,7 +65,7 @@ class TestTimelineLogObserver(TestCase):
 
     def setUp(self):
         self.action_log = UndoableActionLog()
-        self.observer = TimelineLogObserverSpy(self.action_log)
+        self.observer = TimelineObserverSpy(self.action_log)
 
     def testConnectionAndDisconnection(self):
         timeline = GES.Timeline.new_audio_video()
@@ -94,12 +94,6 @@ class TestTimelineLogObserver(TestCase):
         self.assertFalse(track_element1.connected)
         self.assertTrue(track_element2.connected)
 
-        self.observer.stopObserving(timeline)
-        self.assertFalse(timeline.connected)
-        self.assertFalse(clip1.connected)
-        self.assertFalse(track_element1.connected)
-        self.assertFalse(track_element2.connected)
-
 
 class TestTimelineUndo(TestCase):
 
@@ -112,7 +106,7 @@ class TestTimelineUndo(TestCase):
         self.layer = GES.Layer()
         self.timeline.add_layer(self.layer)
         self.action_log = UndoableActionLog()
-        self.observer = TimelineLogObserverSpy(self.action_log)
+        self.observer = TimelineObserverSpy(self.action_log)
         self.observer.startObserving(self.timeline)
 
     def getTimelineClips(self):