Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Merge pull request #12405 from adobe/swmitra/RefCountFix
Browse files Browse the repository at this point in the history
Fix for documents refCount issue
  • Loading branch information
nethip committed May 19, 2016
2 parents 681069f + 0c8b2cd commit 2a4b491
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 29 deletions.
50 changes: 32 additions & 18 deletions src/document/Document.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,12 @@ define(function (require, exports, module) {
this.file = file;
this._updateLanguage();
this.refreshText(rawText, initialTimestamp, true);
// List of full editors which are initialized as master editors for this doc.
this._associatedFullEditors = [];
}

EventDispatcher.makeEventDispatcher(Document.prototype);

/**
* List of editors which were initialized as master editors for this doc.
*/
Document.prototype._associatedFullEditors = [];

/**
* Number of clients who want this Document to stay alive. The Document is listed in
* DocumentManager._openDocuments whenever refCount > 0.
Expand Down Expand Up @@ -200,12 +197,6 @@ define(function (require, exports, module) {
* @param {!Editor} masterEditor
*/
Document.prototype._makeEditable = function (masterEditor) {
if (this._masterEditor) {
//Already a master editor is associated , so preserve the old editor in list of full editors
if (this._associatedFullEditors.indexOf(this._masterEditor) < 0) {
this._associatedFullEditors.push(this._masterEditor);
}
}

this._text = null;
this._masterEditor = masterEditor;
Expand Down Expand Up @@ -241,17 +232,29 @@ define(function (require, exports, module) {
*/
Document.prototype._toggleMasterEditor = function (masterEditor) {
// Do a check before processing the request to ensure inline editors are not being set as master editor
if (this._associatedFullEditors.indexOf(masterEditor) >= 0) {
if (this._masterEditor) {
// Already a master editor is associated , so preserve the old editor in list of editors
if (this._associatedFullEditors.indexOf(this._masterEditor) < 0) {
this._associatedFullEditors.push(this._masterEditor);
}
}
if (this.file === masterEditor.document.file && this._associatedFullEditors.indexOf(masterEditor) >= 0) {
this._masterEditor = masterEditor;
}
};


/**
* Checks and returns if a full editor exists for the provided pane attached to this document
* @param {String} paneId
* @return {Editor} Attached editor bound to the provided pane id
*/
Document.prototype._checkAssociatedEditorForPane = function (paneId) {
var editorCount, editorForPane;
for (editorCount = 0; editorCount < this._associatedFullEditors.length; ++editorCount) {
if (this._associatedFullEditors[editorCount]._paneId === paneId) {
editorForPane = this._associatedFullEditors[editorCount];
break;
}
}

return editorForPane;
};

/**
* Disassociates an editor from this document if present in the associated editor list
* To be used internally by Editor only when destroyed and not the current master editor for the document
Expand All @@ -263,6 +266,17 @@ define(function (require, exports, module) {
}
};

/**
* Aassociates a full editor to this document
* To be used internally by Editor only when pane marking happens
*/
Document.prototype._associateEditor = function (editor) {
// Do a check before processing the request to ensure inline editors are not being handled
if (this._associatedFullEditors.indexOf(editor) === -1) {
this._associatedFullEditors.push(editor);
}
};

/**
* Guarantees that _masterEditor is non-null. If needed, asks EditorManager to create a new master
* editor bound to this Document (which in turn causes Document._makeEditable() to be called).
Expand Down
21 changes: 13 additions & 8 deletions src/editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,15 @@ define(function (require, exports, module) {
this.on("change", function (event, editor, changeList) {
self._handleEditorChange(changeList);
});
this.on("focus", function (event, editor) {
if (self._hostEditor) {
// Mark the host editor as the master editor for the hosting document
self._hostEditor.document._toggleMasterEditor(self._hostEditor);
} else {
// Set this full editor as master editor for the document
self.document._toggleMasterEditor(self);
}
});

// Set code-coloring mode BEFORE populating with text, to avoid a flash of uncolored text
this._codeMirror.setOption("mode", mode);
Expand Down Expand Up @@ -468,6 +477,10 @@ define(function (require, exports, module) {

Editor.prototype.markPaneId = function (paneId) {
this._paneId = paneId;

// Also add this to the pool of full editors
this.document._associateEditor(this);

// In case this Editor is initialized not as the first full editor for the document
// and the document is already dirty and present in another working set, make sure
// to add this documents to the new panes working set.
Expand Down Expand Up @@ -993,14 +1006,6 @@ define(function (require, exports, module) {

// Convert CodeMirror onFocus events to EditorManager activeEditorChanged
this._codeMirror.on("focus", function () {
if (self._hostEditor) {
// Mark the host editor as the master editor for the hosting document
self._hostEditor.document._toggleMasterEditor(self._hostEditor);
} else {
// Set this full editor as master editor for the document
self.document._toggleMasterEditor(self);
}

self._focused = true;
self.trigger("focus", self);

Expand Down
16 changes: 13 additions & 3 deletions src/editor/EditorManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,15 @@ define(function (require, exports, module) {
var createdNewEditor = false,
editor = document._masterEditor;

//Check if a master editor is not set already or the current master editor doesn't belong
//to the pane container requested - to support creation of multiple full editors
if (!editor || editor._paneId !== pane.id) {
// Check if a master editor is not set already or the current master editor doesn't belong
// to the pane container requested - to support creation of multiple full editors
// This check is required as _masterEditor is the active full editor for the document
// and there can be existing full editor created for other panes
if (editor && editor._paneId && editor._paneId !== pane.id) {
editor = document._checkAssociatedEditorForPane(pane.id);
}

if (!editor) {
// Performance (see #4757) Chrome wastes time messing with selection
// that will just be changed at end, so clear it for now
if (window.getSelection && window.getSelection().empty) { // Chrome
Expand All @@ -555,6 +561,10 @@ define(function (require, exports, module) {
// Editor doesn't exist: populate a new Editor with the text
editor = _createFullEditorForDocument(document, pane, editorOptions);
createdNewEditor = true;
} else if (editor.$el.parent()[0] !== pane.$content[0]) {
// editor does exist but is not a child of the pane so add it to the
// pane (which will switch the view's container as well)
pane.addView(editor);
}

// show the view
Expand Down

0 comments on commit 2a4b491

Please sign in to comment.