Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc(viewer): support 3.x and legacy (2.x) reports in viewer #5204

Merged
merged 8 commits into from
May 16, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
22 changes: 13 additions & 9 deletions lighthouse-core/report/html/renderer/report-ui-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
* the report.
*/

const VIEWER_ORIGIN = 'https://googlechrome.github.io';

/* globals self URL Blob CustomEvent getFilenamePrefix window */

class ReportUIFeatures {
Expand Down Expand Up @@ -318,7 +320,8 @@ class ReportUIFeatures {
break;
}
case 'open-viewer': {
this.sendJsonReport();
const viewerPath = '/lighthouse/viewer/';
this.sendJsonReport(this.json, viewerPath);
break;
}
case 'save-gist': {
Expand All @@ -344,30 +347,31 @@ class ReportUIFeatures {
/**
* Opens a new tab to the online viewer and sends the local page's JSON results
* to the online viewer using postMessage.
* @param {!ReportRenderer.ReportJSON} reportJson
* @param {string} viewerPath
* @protected
*/
sendJsonReport() {
const VIEWER_ORIGIN = 'https://googlechrome.github.io';
const VIEWER_URL = `${VIEWER_ORIGIN}/lighthouse/viewer/`;

sendJsonReport(reportJson, viewerPath, onMessageCallback = function() { }) {
// Chrome doesn't allow us to immediately postMessage to a popup right
// after it's created. Normally, we could also listen for the popup window's
// load event, however it is cross-domain and won't fire. Instead, listen
// for a message from the target app saying "I'm open".
const json = this.json;
const json = reportJson;
window.addEventListener('message', function msgHandler(/** @type {!Event} */ e) {
const messageEvent = /** @type {!MessageEvent<{opened: boolean}>} */ (e);
if (messageEvent.origin !== VIEWER_ORIGIN) {
return;
}

if (messageEvent.data.opened) {
popup.postMessage({lhresults: json}, VIEWER_ORIGIN);
window.removeEventListener('message', msgHandler);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this because we need to listen for the render message to close? will this cause duplicate handler problems if they click "Open in viewer" twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

good question. @ebidel is that the situation this is handling?

Copy link
Member

Choose a reason for hiding this comment

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

I remember one issue is if you open viewer from a report, then use viewer as a viewer (looking at other reports or something), if you refresh it will send back an opened message back to the original report and load the first report, not whatever you've been doing since.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what @brendankenny described is the sitch. Although I'm not sure why Chrome does that, but you sometimes get another message event when the page refreshes.

}
onMessageCallback(messageEvent.data);
});

const popup = /** @type {!Window} */ (window.open(VIEWER_URL, '_blank'));
// The popup's window.name is keyed by version+url+fetchTime, so we reuse/select tabs correctly
const fetchTime = json.fetchTime || json.generatedTime;
const windowName = `${json.lighthouseVersion}-${json.initialUrl}-${fetchTime}`;
const popup = /** @type {!Window} */ (window.open(`${VIEWER_ORIGIN}${viewerPath}`, windowName));
}

/**
Expand Down
29 changes: 28 additions & 1 deletion lighthouse-viewer/app/src/lighthouse-report-viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ class LighthouseReportViewer {
_replaceReportHtml(json) {
this._validateReportJson(json);

if (!json.lighthouseVersion.startsWith('3')) {
this._loadInLegacyViewerVersion(json);
return;
}

const dom = new DOM(document);
const renderer = new ReportRenderer(dom);

Expand Down Expand Up @@ -164,12 +169,30 @@ class LighthouseReportViewer {
} catch (e) {
throw new Error('Could not parse JSON file.');
}

this._reportIsFromGist = false;
this._replaceReportHtml(json);
}).catch(err => logger.error(err.message));
}

/**
* Opens new tab with compatible report viewer version
* @param {!ReportRenderer.ReportJSON} reportJson
* @private
*/
_loadInLegacyViewerVersion(json) {
const warnMsg = `Version mismatch between viewer and JSON.
Opening new tab with compatible viewer.`;
const features = new ViewerUIFeatures(new DOM(document));
const viewerPath = '/lighthouse/viewer2x/';

logger.log(warnMsg, false);
features.sendJsonReport(json, viewerPath, onMessage);
Copy link
Collaborator

Choose a reason for hiding this comment

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

impl is a bit confusing that we create a temporary instance of the ViewerUIfeatures to trigger the render in viewer, feels a bit hacky

I think it's mostly just the fake flexibility of sendJsonReport that confuses me. WDYT of making this a bit more explicit with a sendJsonReportToV2 or something that both sure the same underlying logic?

they could just resolve a promise when they get confirmation that the report has been rendered and nix the onMessage piece

function onMessage(data) {
if (data.rendered) window.close();
logger.log(`${warnMsg} You can close this tab.`, false);
}
}

/**
* Reads a file and returns its content as a string.
* @param {!File} file
Expand Down Expand Up @@ -296,6 +319,10 @@ class LighthouseReportViewer {
if (e.source === self.opener && e.data.lhresults) {
this._reportIsFromGist = false;
this._replaceReportHtml(e.data.lhresults);

if (self.opener && !self.opener.closed) {
self.opener.postMessage({rendered: true}, '*');
}
if (window.ga) {
window.ga('send', 'event', 'report', 'open in viewer');
}
Expand Down
7 changes: 0 additions & 7 deletions lighthouse-viewer/app/src/viewer-ui-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,6 @@ class ViewerUIFeatures extends ReportUIFeatures {
return ReportGenerator.generateReportHtml(this.json);
}

/**
* @override
*/
sendJsonReport() {
throw new Error('Cannot send JSON to Viewer from Viewer.');
}

/**
* @override
*/
Expand Down