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

fix hardcoded port in MultiBrowser LiveDevelopment and minor fix in PerfUtil #11957

Closed
wants to merge 5 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ define(function (require, exports, module) {

var FileUtils = require("file/FileUtils"),
EventDispatcher = require("utils/EventDispatcher"),
NodeDomain = require("utils/NodeDomain");
NodeDomain = require("utils/NodeDomain"),
PreferencesManager = require("preferences/PreferencesManager");

var prefs = PreferencesManager.getExtensionPrefs("livedev");
var PREF_SOCKETPORT = "socketport";

// The script that will be injected into the previewed HTML to handle the other side of the socket connection.
var NodeSocketTransportRemote = require("text!LiveDevelopment/MultiBrowserImpl/transports/remote/NodeSocketTransportRemote.js");
Expand All @@ -46,9 +50,8 @@ define(function (require, exports, module) {
var NodeSocketTransportDomain = new NodeDomain("nodeSocketTransport", domainPath);

// This must match the port declared in NodeSocketTransportDomain.js.
// TODO: randomize this?
var SOCKET_PORT = 8123;

var SOCKET_PORT = prefs.get(PREF_SOCKETPORT) || 8123;

/**
* Returns the script that should be injected into the browser to handle the other end of the transport.
* @return {string}
Expand All @@ -60,6 +63,12 @@ define(function (require, exports, module) {
"</script>\n";
}

function start () {
SOCKET_PORT = prefs.get(PREF_SOCKETPORT);
console.log("NodeSocketTransport - start on port:" + SOCKET_PORT);
NodeSocketTransportDomain.exec('start', SOCKET_PORT);
}

// Events

// We can simply retrigger the events we receive from the node domain directly, since they're in
Expand All @@ -77,10 +86,11 @@ define(function (require, exports, module) {

// Exports
exports.getRemoteScript = getRemoteScript;
exports.start = start;

// Proxy the node domain methods directly through, since they have exactly the same
// signatures as the ones we're supposed to provide.
["start", "send", "close"].forEach(function (method) {
["send", "close"].forEach(function (method) {
exports[method] = function () {
var args = Array.prototype.slice.call(arguments);
args.unshift(method);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
var _clients = {};

// This must match the port declared in NodeSocketTransport.js.
// TODO: randomize this?
// Default socket port number
var SOCKET_PORT = 8123;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there is a pref for the socket port, this should be read from the prefs too.

Copy link
Author

Choose a reason for hiding this comment

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

Are you sure? This is part of NodeDomain, can I read prefs there? Anyway: starting server sends port number, this here will be overwritten.


/**
Expand All @@ -76,11 +76,13 @@
/**
* @private
* Creates the WebSocketServer and handles incoming connections.
* @param {number} port
*/
function _createServer() {
function _createServer(port) {
if (!_wsServer) {
// TODO: make port configurable, or use random port
_wsServer = new WebSocketServer({port: SOCKET_PORT});
port = port || SOCKET_PORT;
_wsServer = new WebSocketServer({port: port});
_wsServer.on("connection", function (ws) {
ws.on("message", function (msg) {
console.log("WebSocketServer - received - " + msg);
Expand Down Expand Up @@ -140,10 +142,10 @@

/**
* Initializes the socket server.
* @param {string} url
* @param {number} port
*/
function _cmdStart(url) {
_createServer();
function _cmdStart(port) {
_createServer(port);
}

/**
Expand Down Expand Up @@ -192,7 +194,9 @@
_cmdStart, // command handler function
false, // this command is synchronous in Node
"Creates the WS server",
[]
[
{name: "port", type: "number", description: "number of WS port"}
]
);
domainManager.registerCommand(
"nodeSocketTransport", // domain name
Expand Down
12 changes: 12 additions & 0 deletions src/LiveDevelopment/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ define(function main(require, exports, module) {
var multiBrowserPref = prefs.definePreference(PREF_MULTIBROWSER, "boolean", false, {
description: Strings.DESCRIPTION_LIVE_DEV_MULTIBROWSER
});
var PREF_SOCKETPORT = "socketport";
var transportSocketPort = prefs.definePreference(PREF_SOCKETPORT, "number", 8123, {
description: Strings.DESCRIPTION_LIVE_DEV_SOCKETPORT
});

/** Toggles or sets the preference **/
function _togglePref(key, value) {
Expand Down Expand Up @@ -355,6 +359,14 @@ define(function main(require, exports, module) {
_setImplementation(prefs.get(PREF_MULTIBROWSER));
}
});
transportSocketPort
.on("change", function () {
// Stop current session if it is open and multibrowser
if (LiveDevImpl && LiveDevImpl === MultiBrowserLiveDev &&
LiveDevImpl.status >= LiveDevImpl.STATUS_ACTIVE) {
LiveDevImpl.close();
}
});

});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@
"requestFilter",
[{
name: "location",
type: "{hostname: string, pathname: string, port: number, root: string: id: number}",
type: "{hostname: string, pathname: string, port: number, root: string, id: number}",
description: "request path"
}]
);
Expand Down
1 change: 1 addition & 0 deletions src/nls/root/strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,7 @@ define({
"DESCRIPTION_ASYNC_TIMEOUT" : "The time in milliseconds after which asynchronous linters time out",
"DESCRIPTION_LINTING_PREFER" : "Array of linters to run first",
"DESCRIPTION_LIVE_DEV_MULTIBROWSER" : "true to enable experimental Live Preview",
"DESCRIPTION_LIVE_DEV_SOCKETPORT" : "Experimental Live Preview communication socket port number",
"DESCRIPTION_USE_PREFERED_ONLY" : "true to run providers specified in linting.prefer only",
"DESCRIPTION_MAX_CODE_HINTS" : "Maximum code hints displayed at once",
"DESCRIPTION_PATH" : "Path specific settings",
Expand Down
6 changes: 6 additions & 0 deletions src/utils/PerfUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ define(function (require, exports, module) {
* @param {Object} id Timer id.
*/
function updateMeasurement(id) {
if (!enabled) {
return;
}
var elapsedTime = brackets.app.getElapsedMilliseconds();

if (updatableTests[id.id]) {
Expand Down Expand Up @@ -282,6 +285,9 @@ define(function (require, exports, module) {
* @param {Object} id Timer id.
*/
function finalizeMeasurement(id) {
if (!enabled) {
return;
}
if (activeTests[id.id]) {
delete activeTests[id.id];
}
Expand Down