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

Clean-up servers when closing LiveDevelopment #10453

Merged
merged 4 commits into from
May 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/LiveDevelopment/LiveDevServerManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ define(function (require, exports, module) {
}

/**
* The method by which a server registers itself.
* The method by which a server registers itself. It returns an
* object handler that can be used to remove that server from the list.
*
* @param {BaseServer|{create: function():BaseServer}} provider
* The provider to be registered, described below.
Expand All @@ -88,6 +89,7 @@ define(function (require, exports, module) {
* particular url. Providers that register with a higher priority will
* have the opportunity to provide a given url before those with a
* lower priority. The higher the number, the higher the priority.
* @return {{object}}
*/
function registerServer(provider, priority) {
if (!provider.create) {
Expand All @@ -102,6 +104,22 @@ define(function (require, exports, module) {

_serverProviders.push(providerObj);
_serverProviders.sort(_providerSort);

return providerObj;
}

/**
* Remove a server from the list of the registered providers.
*
* @param {{object}} provider The provider to be removed.
*/
function removeServer(provider) {
var i;
for (i = 0; i < _serverProviders.length; i++) {
if (provider === _serverProviders[i]) {
_serverProviders.splice(i, 1);
}
}
}

// Backwards compatibility
Expand All @@ -111,4 +129,5 @@ define(function (require, exports, module) {
// Define public API
exports.getServer = getServer;
exports.registerServer = registerServer;
exports.removeServer = removeServer;
});
51 changes: 31 additions & 20 deletions src/LiveDevelopment/LiveDevelopment.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,12 @@ define(function LiveDevelopment(require, exports, module) {
* @type {BaseServer}
*/
var _server;

/**
* @private
* Handles of registered servers
*/
var _regServers = [];

function _isPromisePending(promise) {
return promise && promise.state() === "pending";
Expand Down Expand Up @@ -865,6 +871,11 @@ define(function LiveDevelopment(require, exports, module) {
var closeDeferred = (brackets.platform === "mac") ? NativeApp.closeLiveBrowser() : $.Deferred().resolve();
closeDeferred.done(function () {
_setStatus(STATUS_INACTIVE, reason || "explicit_close");
// clean-up registered servers
_regServers.forEach(function (server) {
LiveDevServerManager.removeServer(server);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't close servers when the LivePreview session closes. Perhaps, it would be the right moment to close the server. This, though, will require exposing closeServer in at least StaticServer... It seems like the node domain command is there, but it's never used. I'll file a separate issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be good since the server keeps running after closing the session. Let me know if you've created the issue, I can take a look and submit a PR for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #11108.

});
_regServers = [];
_closeDeferred.resolve();
}).fail(function (err) {
if (err) {
Expand Down Expand Up @@ -1288,6 +1299,22 @@ define(function LiveDevelopment(require, exports, module) {
return deferred.promise();
}

function getCurrentProjectServerConfig() {
return {
baseUrl: ProjectManager.getBaseUrl(),
pathResolver: ProjectManager.makeProjectRelativeIfPossible,
root: ProjectManager.getProjectRoot().fullPath
};
}

function _createUserServer() {
return new UserServer(getCurrentProjectServerConfig());
}

function _createFileServer() {
return new FileServer(getCurrentProjectServerConfig());
}

/**
* Open the Connection and go live
*
Expand All @@ -1314,6 +1341,10 @@ define(function LiveDevelopment(require, exports, module) {
}
}

// Register user defined server provider and keep handlers for further clean-up
_regServers.push(LiveDevServerManager.registerServer({ create: _createUserServer }, 99));
_regServers.push(LiveDevServerManager.registerServer({ create: _createFileServer }, 0));

// TODO: need to run _onFileChanged() after load if doc != currentDocument here? Maybe not, since activeEditorChange
// doesn't trigger it, while inline editors can still cause edits in doc other than currentDoc...
_getInitialDocFromCurrent().done(function (doc) {
Expand Down Expand Up @@ -1441,22 +1472,6 @@ define(function LiveDevelopment(require, exports, module) {
}
}

function getCurrentProjectServerConfig() {
return {
baseUrl: ProjectManager.getBaseUrl(),
pathResolver: ProjectManager.makeProjectRelativeIfPossible,
root: ProjectManager.getProjectRoot().fullPath
};
}

function _createUserServer() {
return new UserServer(getCurrentProjectServerConfig());
}

function _createFileServer() {
return new FileServer(getCurrentProjectServerConfig());
}

/** Initialize the LiveDevelopment Session */
function init(theConfig) {
exports.config = theConfig;
Expand All @@ -1475,10 +1490,6 @@ define(function LiveDevelopment(require, exports, module) {
.on("dirtyFlagChange", _onDirtyFlagChange);
ProjectManager
.on("beforeProjectClose beforeAppClose", close);

// Register user defined server provider
LiveDevServerManager.registerServer({ create: _createUserServer }, 99);
LiveDevServerManager.registerServer({ create: _createFileServer }, 0);

// Initialize exports.status
_setStatus(STATUS_INACTIVE);
Expand Down