[gnome-documents] overview, selections: A child shouldn't hold a reference to its parent



commit 29a975cc69598a1b6c121c03dd1c1dc9259f9e20
Author: Debarshi Ray <debarshir gnome org>
Date:   Thu May 11 20:07:32 2017 +0200

    overview, selections: A child shouldn't hold a reference to its parent
    
    SelectionToolbar is a child of OverviewStack. The parent container
    inherently holds a reference to its children. Therefore, having a child
    also hold a reference to its parent makes us vulnerable to reference
    cycles.
    
    In this case, SelectionToolbar only needs OverviewStack to access the
    selection-mode GAction. So, let's only refer to the GAction.
    
    For this to work, OverviewStack needs to populate its action group
    before instantiating SelectionToolbar.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=782528

 src/overview.js   |    8 ++++----
 src/selections.js |   14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)
---
diff --git a/src/overview.js b/src/overview.js
index d11910e..896478d 100644
--- a/src/overview.js
+++ b/src/overview.js
@@ -1037,6 +1037,10 @@ const OverviewStack = new Lang.Class({
         this.parent({ orientation: Gtk.Orientation.VERTICAL,
                       visible: true });
 
+        let actions = this._getDefaultActions();
+        this.actionGroup = new Gio.SimpleActionGroup();
+        Utils.populateActionGroup(this.actionGroup, actions, 'view');
+
         this._stack = new Gtk.Stack({ visible: true });
         this.pack_start(this._stack, true, true, 0);
 
@@ -1044,10 +1048,6 @@ const OverviewStack = new Lang.Class({
         this._selectionToolbar = new Selections.SelectionToolbar(this);
         this.pack_end(this._selectionToolbar, false, false, 0);
 
-        let actions = this._getDefaultActions();
-        this.actionGroup = new Gio.SimpleActionGroup();
-        Utils.populateActionGroup(this.actionGroup, actions, 'view');
-
         // now create the actual content widgets
         this._documents = new ViewContainer(this, WindowMode.WindowMode.DOCUMENTS);
         let label = Application.application.isBooks ? _('Books') : _("Documents");
diff --git a/src/selections.js b/src/selections.js
index 449840b..0086c05 100644
--- a/src/selections.js
+++ b/src/selections.js
@@ -861,10 +861,10 @@ const SelectionToolbar = new Lang.Class({
         this._docBeginPrintId = 0;
         this._itemListeners = {};
         this._insideRefresh = false;
-        this._overview = overview;
 
         this.parent();
 
+        this._selectionModeAction = overview.getAction('selection-mode');
 
         this._toolbarOpen.connect('clicked', Lang.bind(this, this._onToolbarOpen));
         this._toolbarPrint.connect('clicked', Lang.bind(this, this._onToolbarPrint));
@@ -1001,13 +1001,13 @@ const SelectionToolbar = new Lang.Class({
 
         let dialog = new OrganizeCollectionDialog(toplevel);
         dialog.connect('destroy', Lang.bind(this, function() {
-            this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false));
+            this._selectionModeAction.change_state(GLib.Variant.new('b', false));
         }));
     },
 
     _onToolbarOpen: function(widget) {
         let selection = Application.selectionController.getSelection();
-        this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false));
+        this._selectionModeAction.change_state(GLib.Variant.new('b', false));
 
         selection.forEach(Lang.bind(this,
             function(urn) {
@@ -1038,7 +1038,7 @@ const SelectionToolbar = new Lang.Class({
             }));
 
         let deleteNotification = new Notifications.DeleteNotification(docs);
-        this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false));
+        this._selectionModeAction.change_state(GLib.Variant.new('b', false));
     },
 
     _onToolbarProperties: function(widget) {
@@ -1048,7 +1048,7 @@ const SelectionToolbar = new Lang.Class({
         dialog.connect('response', Lang.bind(this,
             function(widget, response) {
                 dialog.destroy();
-                this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false));
+                this._selectionModeAction.change_state(GLib.Variant.new('b', false));
             }));
     },
 
@@ -1058,7 +1058,7 @@ const SelectionToolbar = new Lang.Class({
        dialog.connect('response', Lang.bind(this,
            function(widget, response) {
                dialog.destroy();
-               this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false));
+               this._selectionModeAction.change_state(GLib.Variant.new('b', false));
            }));
     },
 
@@ -1073,7 +1073,7 @@ const SelectionToolbar = new Lang.Class({
         this._docToPrint = Application.documentManager.getItemById(selection[0]);
         this._docBeginPrintId = this._docToPrint.connect('begin-print', Lang.bind(this,
             function(doc) {
-                this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false));
+                this._selectionModeAction.change_state(GLib.Variant.new('b', false));
             }));
 
         this._docToPrint.print(this.get_toplevel());


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]