Skip to content

Commit

Permalink
Fix fallback to old inspectors
Browse files Browse the repository at this point in the history
As @chancancode explained in emberjs#1120[1], the injected debug code previously did
too much work in order to report a version miss.

To help understand what went wrong, I'll give an abridged version of the
boot-up protocol, and how the fallback logic fits in.

1. The Ember inspector application boots up inside of a devtools iframe,
   and injects the version of `ember_debug.js` for the latest version
   of the Ember inspector.
2. `ember_debug.js` to become aware of any supported version of Ember,
   and when that happens, it checks whether the version of Ember on the
   current page is supported by the current version of `ember_debug.js`.
3. If it matches, everything proceeds as usual, and `ember_debug.js`
   starts communicating with the inspector pane.
4. If it doesn't match, `ember_debug.js` sends a `version-mismatch` message to
   the inspector pane along with the current version of Ember used on the current
   page. The inspector pane then calculates which version of the inspector to use
   (currently either `0-0-0` for Ember 0.0 to Ember 2.6 or `2-7-0` for Ember 2.8
   to Ember 3.2). When this happens, the inspector navigates itself to the older
   version of the inspector, allowing that version to bootstrap itself.

Note that the inspector uses the Ember registry and loader to register its
modules, so any modules for the latest version of ember_debug will *still be
registered* when the older version of `ember_debug.js` is evaluated.

This is not intrinsically a problem, since the calls to `define` in the new
version of `ember_debug.js` will replace the old modules. Any modules that were
present in the latest version but not the previous version will still be
registered, but won't be used by the older code.

The problem with all of that is that if any module is *required* in the process
of reporting the version miss, the module's exports will remain cached in
Ember's loader. Replacing the module with new source code has no effect,
because the next time someone tries to require the module, it will just return
the old cached exports, and the new `define` is completely ignored.

This means that, depending on which code is required as part of reporting the
version miss, a random Frankenstein combination of modules will be used at
runtime. It should come as no surprise that this could have problems. The
reason that it didn't cause any issues until recently is a coincidence: the
assortment of modules in question happened to work well enough to avoid any
problems that incited a user to report the bug.

The solution to the problem in this commit is to completely avoid requiring any
modules before the version is correctly matched. There were three modules
previously required:

1. Two utility modules: `on-ready` and `version`. These modules were small so I
   inlined the code next to the version reporting logic.
2. The ember_debug adapter. This is a much thornier dependency, because it sucks
   in a whole bunch of additional dependencies and is the root cause of the current
   issue. I addressed this dependency by hand-coding just enough logic to create
   a `MessageChannel`, post it to the current window, and `postMessage` a
   `version-mismatch` message through the channel.

This works as long as the adapter uses a `MessageChannel` to communicate the
version mismatch. This is how `web-extension.js` works, which covers Chrome,
Firefox, Edgium, and remote debugging using the remote debugging protocol.

It does not cover other backends, but those backends are currently broken for
other reasons. It doesn't substantially burden the work of making other
backends work again, it simply constrains backends to use a `MessageChannel`
(posted to the inspected app's window) to communicate version mismatches. It
doesn't otherwise constrain any adapter functionality in `ember_debug` or the
Ember Inspector. If this constraint isn't acceptable for a future backend, we
could also support alternative ways of communicating the version mismatch, as
long as that didn't require the version reporting code to load in the entire
adapter module.

[1]: emberjs#1120 (comment)
  • Loading branch information
wycats authored and patricklx committed Sep 19, 2022
1 parent 17af5f5 commit a8f1764
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 21 deletions.
20 changes: 13 additions & 7 deletions app/adapters/web-extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ export default BasicAdapter.extend({
},

_injectDebugger() {
loadEmberDebug().then((emberDebug) => {
chrome.devtools.inspectedWindow.eval(emberDebug);
loadEmberDebug().then(emberDebug => {
chrome.devtools.inspectedWindow.eval(emberDebug, (success, error) => {
if (success === undefined && error) {
throw error;
}
});
this.onResourceAdded();
});
},
Expand Down Expand Up @@ -96,24 +100,26 @@ export default BasicAdapter.extend({
* @param {String} goToVersion
*/
onVersionMismatch(goToVersion) {
window.location.href = `../panes-${goToVersion.replace(/\./g, '-')}/index.html`;
window.location.href = `../panes-${goToVersion.replace(
/\./g,
'-'
)}/index.html`;
},

/**
We handle the reload here so we can inject
scripts as soon as possible into the new page.
*/
reloadTab() {
loadEmberDebug().then((emberDebug) => {
loadEmberDebug().then(emberDebug => {
chrome.devtools.inspectedWindow.reload({ injectedScript: emberDebug });
});

},

canOpenResource: false,

sendIframes(urls) {
loadEmberDebug().then((emberDebug) => {
loadEmberDebug().then(emberDebug => {
urls.forEach(url => {
chrome.devtools.inspectedWindow.eval(emberDebug, { frameURL: url });
});
Expand All @@ -124,7 +130,7 @@ export default BasicAdapter.extend({
function loadEmberDebug() {
let minimumVersion = config.emberVersionsSupported[0].replace(/\./g, '-');
let xhr;
return new Promise((resolve) => {
return new Promise(resolve => {
if (!emberDebug) {
xhr = new XMLHttpRequest();
xhr.open("GET", chrome.runtime.getURL(`/panes-${minimumVersion}/ember_debug.js`));
Expand Down
93 changes: 87 additions & 6 deletions ember_debug/vendor/startup-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ var EMBER_VERSIONS_SUPPORTED = {{EMBER_VERSIONS_SUPPORTED}};
// @formatter:on

(function(adapter) {
var onReady = requireModule('ember-debug/utils/on-ready').onReady;
var compareVersion = requireModule('ember-debug/utils/version').compareVersion;

onEmberReady(function() {
// global to prevent injection
if (window.NO_EMBER_DEBUG) {
Expand Down Expand Up @@ -219,6 +216,12 @@ var EMBER_VERSIONS_SUPPORTED = {{EMBER_VERSIONS_SUPPORTED}};
});
}

let channel = new MessageChannel();
let port = channel.port1;
window.postMessage('debugger-client', '*', [channel.port2]);

let registeredMiss = false;

/**
* This function is called if the app's Ember version
* is not supported by this version of the inspector.
Expand All @@ -227,16 +230,24 @@ var EMBER_VERSIONS_SUPPORTED = {{EMBER_VERSIONS_SUPPORTED}};
* to an inspector version that supports this Ember version.
*/
function sendVersionMiss() {
var adapter = requireModule('ember-debug/adapters/' + currentAdapter)['default'].create();
adapter.onMessageReceived(function(message) {
if (registeredMiss) {
return;
}

registeredMiss = true;

port.addEventListener('message', message => {
if (message.type === 'check-version') {
sendVersionMismatch();
}
});

sendVersionMismatch();

port.start();

function sendVersionMismatch() {
adapter.sendMessage({
port.postMessage({
name: 'version-mismatch',
version: Ember.VERSION,
from: 'inspectedWindow'
Expand Down Expand Up @@ -277,4 +288,74 @@ var EMBER_VERSIONS_SUPPORTED = {{EMBER_VERSIONS_SUPPORTED}};
return !toVersion || compareVersion(version, toVersion) === -1;
}

function onReady(callback) {
if (document.readyState === 'complete' || document.readyState === 'interactive') {
setTimeout(completed);
} else {
document.addEventListener("DOMContentLoaded", completed, false);
// For some reason DOMContentLoaded doesn't always work
window.addEventListener("load", completed, false);
}

function completed() {
document.removeEventListener("DOMContentLoaded", completed, false);
window.removeEventListener("load", completed, false);
callback();
}
}

/**
* Compares two Ember versions.
*
* Returns:
* `-1` if version1 < version
* 0 if version1 == version2
* 1 if version1 > version2
*
* @param {String} version1
* @param {String} version2
* @return {Boolean} result of the comparison
*/
function compareVersion(version1, version2) {
let compared, i;
version1 = cleanupVersion(version1).split('.');
version2 = cleanupVersion(version2).split('.');
for (i = 0; i < 3; i++) {
compared = compare(+version1[i], +version2[i]);
if (compared !== 0) {
return compared;
}
}
return 0;
}

/**
* Remove -alpha, -beta, etc from versions
*
* @param {String} version
* @return {String} The cleaned up version
*/
function cleanupVersion(version) {
return version.replace(/-.*/g, '');
}

/**
* @method compare
* @param {Number} val
* @param {Number} number
* @return {Number}
* 0: same
* -1: <
* 1: >
*/
function compare(val, number) {
if (val === number) {
return 0;
} else if (val < number) {
return -1;
} else if (val > number) {
return 1;
}
}

}(currentAdapter));
9 changes: 2 additions & 7 deletions skeletons/web-extension/content-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,13 @@
document.documentElement.dataset.emberExtension = 1;
}



// Iframes should not reset the icon so we make sure
// it's the top level window before resetting.
if (window.top === window) {
// Clear a possible previous Ember icon
chrome.runtime.sendMessage({ type: 'resetEmberIcon' });
}


/**
* Inject JS into the page to check for an app on domready. The in-page-script
* is used by all variants of ember-inspector (Chrome, FF, Bookmarklet) to get
Expand All @@ -91,7 +88,7 @@
*/
var iframes = document.getElementsByTagName('iframe');
var urls = [];
for (var i = 0, l = iframes.length; i < l; i ++) {
for (var i = 0, l = iframes.length; i < l; i++) {
urls.push(iframes[i].src);
}

Expand All @@ -103,6 +100,4 @@
setTimeout(function() {
chrome.runtime.sendMessage({type: 'iframes', urls: urls});
}, 500);


}());
})();
2 changes: 1 addition & 1 deletion skeletons/web-extension/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"contextMenus"
],

"content_security_policy": "script-src 'self'; object-src 'self'",
"content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",
"devtools_page": "devtools.html",

"content_scripts": [{
Expand Down

0 comments on commit a8f1764

Please sign in to comment.