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

Clean up Live Development agent loading #3203

Closed
iwehrman wants to merge 3 commits into from
Closed

Clean up Live Development agent loading #3203

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d5d003
Agents should not finish loading until the services they rely on have…
iwehrman Mar 21, 2013
4e2c7de
Send a null event when rejecting for compatibility with the _onError …
Mar 21, 2013
f2b27a9
Merge branch 'master' into iwehrman/synchronize-live-development-agen…
Apr 9, 2013
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
13 changes: 12 additions & 1 deletion src/LiveDevelopment/Agents/ConsoleAgent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,22 @@ define(function ConsoleAgent(require, exports, module) {

/** Initialize the agent */
function load() {
Inspector.Console.enable();
var deferred = $.Deferred();

Inspector.Console.enable(function (response) {
Copy link
Copy Markdown
Member

@jasonsanjose jasonsanjose Apr 15, 2013

Choose a reason for hiding this comment

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

Instead of adding a callback each time (as shown here) I was wondering if we could update Inspector._send to always return a promise if no explicit callback is specified.

Copy link
Copy Markdown
Contributor Author

@iwehrman iwehrman Apr 15, 2013

Choose a reason for hiding this comment

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

I suppose so. I don't have a good feeling though for whether that can be done reliably across all API calls and whether that would have any other ramifications.

Copy link
Copy Markdown
Member

@jasonsanjose jasonsanjose Apr 15, 2013

Choose a reason for hiding this comment

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

I made the change to Inspector._send to fix #3402 (in pull #3442). I'm not concerned about the reliability since the debugger API always sends a response for every method call. I think it makes sense if I reconcile your changes into my bug fix. What do you think?

Copy link
Copy Markdown
Contributor Author

@iwehrman iwehrman Apr 16, 2013

Choose a reason for hiding this comment

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

Yes, I think that makes a lot of sense. If you're bangin' away on Live Development this sprint, you might also take a look at the race I mentioned a few comments up.

Copy link
Copy Markdown
Member

@jasonsanjose jasonsanjose Apr 17, 2013

Choose a reason for hiding this comment

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

I probably won't dig into the loadAgents/unloadAgents race condition here since it doesn't immediately impact the Chrome 27 bug as far as I know. Randy did file a new bug #3390, so I'll reference your comment there.

if (response.error) {
deferred.reject(null, response.error);
} else {
deferred.resolve();
}
});

$(Inspector.Console)
.on("messageAdded.ConsoleAgent", _onMessageAdded)
.on("messageRepeatCountUpdated.ConsoleAgent", _onMessageRepeatCountUpdated)
.on("messagesCleared.ConsoleAgent", _onMessagesCleared);

return deferred.promise();
}

/** Clean up */
Expand Down
12 changes: 10 additions & 2 deletions src/LiveDevelopment/Agents/DOMAgent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ define(function DOMAgent(require, exports, module) {

/** Initialize the agent */
function load() {
var deferred = $.Deferred();

_load = new $.Deferred();
$(Inspector.Page)
.on("frameNavigated.DOMAgent", _onFrameNavigated)
Expand All @@ -312,8 +314,14 @@ define(function DOMAgent(require, exports, module) {
.on("childNodeCountUpdated.DOMAgent", _onChildNodeCountUpdated)
.on("childNodeInserted.DOMAgent", _onChildNodeInserted)
.on("childNodeRemoved.DOMAgent", _onChildNodeRemoved);
Inspector.Page.enable();
return _load.promise();
Inspector.Page.enable(function (response) {
Copy link
Copy Markdown
Contributor

@redmunds redmunds Mar 21, 2013

Choose a reason for hiding this comment

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

When playing round with disabling the DOMAgent, I discovered that the CSSAgent and RemoteAgent both depend on Inspector.Page being enabled so they get the loadEventFired event, so this needs to be moved out of DOMAgent.

Copy link
Copy Markdown
Contributor

@redmunds redmunds Apr 9, 2013

Choose a reason for hiding this comment

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

This issue is captured in #2599, but I thought I'd mention it here :)

Copy link
Copy Markdown
Member

@jasonsanjose jasonsanjose Apr 17, 2013

Choose a reason for hiding this comment

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

Moved Inspector.Page.enable() to LiveDevelopment onInterstitialPageLoad.

if (response.error) {
deferred.reject(null, response.error);
} else {
deferred.resolve();
}
});
return $.when(_load.promise(), deferred.promise());
}

/** Clean up */
Expand Down
12 changes: 11 additions & 1 deletion src/LiveDevelopment/Agents/NetworkAgent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,19 @@ define(function NetworkAgent(require, exports, module) {

/** Initialize the agent */
function load() {
var deferred = $.Deferred();

_urlRequested = {};
Inspector.Network.enable();
Inspector.Network.enable(function (response) {
if (response.error) {
deferred.reject(null, response.error);
} else {
deferred.resolve();
}
});
$(Inspector.Network).on("requestWillBeSent.NetworkAgent", _onRequestWillBeSent);

return deferred.promise();
}

/** Unload the agent */
Expand Down
20 changes: 10 additions & 10 deletions src/LiveDevelopment/Agents/RemoteAgent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ define(function RemoteAgent(require, exports, module) {
// WebInspector Event: Page.loadEventFired
function _onLoadEventFired(event, res) {
// res = {timestamp}
var request = new XMLHttpRequest();
request.open("GET", "LiveDevelopment/Agents/RemoteFunctions.js");
request.onload = function onLoad() {
var run = "window._LD=" + request.response + "(" + LiveDevelopment.config.experimental + ")";
Inspector.Runtime.evaluate(run, function onEvaluate(res) {
console.assert(!res.wasThrown, res.result.description);
_objectId = res.result.objectId;
var body = require("text!LiveDevelopment/Agents/RemoteFunctions.js"),
command = "window._LD=" + body + "(" + LiveDevelopment.config.experimental + ")";

Inspector.Runtime.evaluate(command, function onEvaluate(response) {
if (response.error || response.wasThrown) {
_load.reject(null, response.error);
} else {
_objectId = response.result.objectId;
_load.resolve();
});
};
request.send(null);
}
});
}

// WebInspector Event: DOM.attributeModified
Expand Down
4 changes: 1 addition & 3 deletions src/LiveDevelopment/Agents/RemoteFunctions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
*
*/


/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, forin: true, maxerr: 50, regexp: true */
/*global define, $, window, document, navigator */
/*jslint vars: true, plusplus: true, browser: true, nomen: true, indent: 4, forin: true, maxerr: 50, regexp: true */

/**
* RemoteFunctions define the functions to be executed in the browser. This
Expand Down
21 changes: 18 additions & 3 deletions src/LiveDevelopment/Agents/ScriptAgent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,33 @@ define(function ScriptAgent(require, exports, module) {

/** Initialize the agent */
function load() {
var enableDeferred = $.Deferred(),
pauseDeferred = $.Deferred();

_urlToScript = {};
_idToScript = {};
_load = new $.Deferred();
Inspector.Debugger.enable();
Inspector.Debugger.setPauseOnExceptions("uncaught");
Inspector.Debugger.enable(function (response) {
if (response.error) {
enableDeferred.reject(null, response.error);
} else {
enableDeferred.resolve();
}
});
Inspector.Debugger.setPauseOnExceptions("uncaught", function (response) {
if (response.error) {
pauseDeferred.reject(null, response.error);
} else {
pauseDeferred.resolve();
}
});
$(DOMAgent).on("getDocument.ScriptAgent", _onGetDocument);
$(Inspector.Debugger)
.on("scriptParsed.ScriptAgent", _onScriptParsed)
.on("scriptFailedToParse.ScriptAgent", _onScriptFailedToParse)
.on("paused.ScriptAgent", _onPaused);
$(Inspector.DOM).on("childNodeInserted.ScriptAgent", _onChildNodeInserted);
return _load;
return $.when(_load.promise(), enableDeferred.promise(), pauseDeferred.promise());
}

/** Clean up */
Expand Down
⚡