clients: retire extension; replace with PSI launcher #9193

connorjclark · 2019-06-13T00:37:50Z

Throwing up this draft to get some feedback on the Chrome extension API / viewer interaction. help, extension noob here.

At first, I attempted to do exactly what the "Open in Viewer" feature does - which is, open the viewer, the viewer sends a message to its self.opener, and the original report page responds with the LHR. In Chrome extension land, I figured the equivalent would be to make use of "externally_connectable", which allows the viewer to connect to the extension via chrome.runtime.connect. On connect, the extension could respond with the LHR. This worked up to the connection point, but any message the extension sent back was never received by the viewer (and caused no errors). I couldn't figure out why, so I moved on to the next approach.

What I went with now is: a background script opens the viewer via chrome.tabs.create and sends the LHR via chrome.tabs.sendMessage. The content script (configured to load at document_start) receives this message, and simply posts it to the window, allowing the viewer to receive the LHR. Does this seem like the best approach?

I did try to do the above without any background script (just open the viewer from popup.js), but the message would never reach the content script.

brendankenny · 2019-06-13T00:45:30Z

we bundle the report generator for just about everything now, would it be simpler to just do that for the extension as well and skip all the message passing stuff?

connorjclark · 2019-06-13T00:54:40Z

We could bundle the renderer. If we don't, and lean on the viewer, we get the extra bonus of not needing to update the extension every release. Whatever that's worth.

patrickhulce · 2019-06-13T02:37:35Z

we get the extra bonus of not needing to update the extension every release

Ooooooo, that sounds like it's worth its weight in gold :)

patrickhulce · 2019-06-13T03:06:08Z

It's been awhile since I've used externally_connectable but I feel like this should be the most straightforward. I've definitely done something like this in the past.

I haven't messed with this in particular, but maybe try getting a super simple sendResponse for chrome.runtime.sendMessage working? 🤷‍♂

connorjclark · 2019-06-13T03:14:14Z

I can try once more, but the commented out block of code was my attempt at that. Perhaps I skipped over something the first time. I'll try again.

connorjclark · 2019-06-17T19:55:52Z

@patrickhulce now that I think of it, I'm not convinced externally_connectable is the better option, even if it happens to work, so I'll defer trying again unless convinced.

Seems simpler to open the viewer via chrome.tabs.create (thus providing the necessary connection via the tabId) than to require the viewer to reach out to the extension (which would require ?utm_source=extension so the viewer knows to use chrome.runtime.connect + would need to know the extension id, making local development annoying).

patrickhulce · 2019-06-17T21:06:56Z

Alright, I'm fine with content script approach. Feel free to clean up and make legit for that method then :)

patrickhulce · 2019-06-17T21:03:31Z

clients/extension/scripts/popup.js

@@ -30,9 +30,41 @@ const NON_BUG_ERROR_MESSAGES = {
      ' with http:// or https://.',
 };

-const subpageVisibleClass = 'subpage--visible';
+const PSI_KEY = 'AIzaSyAjcDRNN9CX9dCazhqI4lGR7yyQbkd_oYE';


is this an extension-specific key?

yeah just forgot to add to the query

connorjclark · 2019-06-17T21:35:35Z

actually, let's just simplify this whole thing and have the content script call PSI itself.

connorjclark · 2019-06-17T22:34:08Z

actualllly, we can circumvent the entire message passing / extension stuff if we just bake loading from PSI into the viewer.

connorjclark · 2019-06-17T22:37:37Z

lighthouse-viewer/app/src/lighthouse-report-viewer.js

+    }
+  });
+
+  return fetch(psiUrl.href).then(res => res.json());


Building script doesn't allow for es6, hence no async/await here. It also injects some Promise polyfills (hey ... we don't like that anymore, right?), so it seems like a larger issue for another day.

brendankenny · 2019-06-18T00:47:47Z

The viewer part would be a good candidate for splitting into a separate PR
We don't really have tests of our bundles after the extension goes away. I'm not sure what we should do about that... https://bugs.chromium.org/p/chromium/issues/detail?id=966721 would help, but would only run on rerolls to DevTools. Puppeteer + the LR bundle?

brendankenny · 2019-06-18T00:48:46Z

The viewer part would be a good candidate for splitting into a separate PR

Also, this is getting pretty close to web.dev/measure territory, or https://developers.google.com/speed/pagespeed/insights/ for that matter. Why do we have those again? :D

connorjclark · 2019-06-18T01:02:18Z

The viewer part would be a good candidate for splitting into a separate PR

Also, this is getting pretty close to web.dev/measure territory, or developers.google.com/speed/pagespeed/insights for that matter. Why do we have those again? :D

close...but not quite yet. web.dev lacks PWA and PSI is only performance, so until one of those changes seems like viewer is a good way to go

patrickhulce

at least wanted to flush these few before I forget for the millionth time

clients/extension/scripts/popup.js

patrickhulce · 2019-10-22T15:49:46Z

I'm getting an error when trying out the extension this time around, is this expected?

connorjclark · 2019-10-23T00:48:56Z

Patrick, I don't know why it's not working for you locally, but it works here: https://googlechrome.github.io/lighthouse/viewer/?psiurl=https://www.example.com&locale=es

I think we may have removed the localhost referral exception. I don't test it with a local viewer anymore.

EDIT: oh you're not using a local viewer. IDK. works for me..

patrickhulce · 2019-10-23T01:09:06Z

@connorjclark yeah I'm not using the local viewer, the message about the referrer seems real. If I visit the URL directly (https://googlechrome.github.io/lighthouse/viewer/?psiurl=http%3A%2F%2Fexample.com%2F&strategy=desktop&category=accessibility&category=seo&category=pwa&utm_source=lh-chrome-ext), it works just fine, but when it gets opened by the extension it fails.

paulirish

NICE

clients/extension/images/lh_logo.svg

clients/extension/popup.html

clients/extension/styles/lighthouse.css

paulirish · 2019-10-23T01:10:52Z

clients/extension/styles/lighthouse.css

-  opacity: 0.74;
-  color: currentColor;
+.button--generate {
+  cursor: pointer;
  font-family: 'Roboto', Arial, sans-serif;


i think this is duplicated by the html,body font already here

button has a default font-family. but should use the variable here.

brendankenny

LGTM 😢

brendankenny · 2019-10-25T15:31:32Z

@connorjclark yeah I'm not using the local viewer, the message about the referrer seems real. If I visit the URL directly (https://googlechrome.github.io/lighthouse/viewer/?psiurl=http%3A%2F%2Fexample.com%2F&strategy=desktop&category=accessibility&category=seo&category=pwa&utm_source=lh-chrome-ext), it works just fine, but when it gets opened by the extension it fails.

we need to make sure this is resolved, though, whether it's by referrer or whatever is going on. A pptr extension test would be handy for that sort of thing :P

brendankenny · 2019-10-25T15:33:09Z

clients/extension/scripts/popup.js

@@ -5,58 +5,10 @@
 */
 'use strict';


FWIW we should probably rename this file to be popup-entry.js or whatever so it's the same clear signal that it's the root of a bundle like the others. I don't feel strongly about it today, though

connorjclark · 2019-10-26T04:10:13Z

@patrickhulce I expected the referrer to be https://googlechrome.github.io/lighthouse/viewer/ but that error message says https://googlechrome.github.io/ is used. I've added the latter to the whitelist. Does it work now?

patrickhulce

Does it work now?

Like budah!

LGTM! farewell extension, you served us well.

brendankenny · 2019-10-26T17:58:55Z

Don't watch

brendankenny · 2019-10-26T18:10:39Z

Huge step here. Great work @connorjclark

googlebot added the cla: yes label Jun 13, 2019

connorjclark force-pushed the simplify-extension branch from 9468ee0 to 92c3baf Compare June 17, 2019 20:35

vercel bot deployed to staging June 17, 2019 20:35 View deployment

patrickhulce reviewed Jun 17, 2019

View reviewed changes

vercel bot deployed to staging June 17, 2019 22:33 View deployment

vercel bot deployed to staging June 17, 2019 22:35 View deployment

vercel bot deployed to staging June 17, 2019 22:36 View deployment

connorjclark commented Jun 17, 2019

View reviewed changes

vercel bot deployed to staging June 17, 2019 23:39 View deployment

vercel bot deployed to staging June 18, 2019 18:25 View deployment

vercel bot deployed to staging June 24, 2019 22:18 View deployment

connorjclark added 7 commits July 22, 2019 15:00

first pass

11eb294

call PSI

a286cb5

working example of PSI -> viewer

a88c839

comment

a3e889f

make extension just a popup, and move psi stuff to viewer

eff5050

remove commit injection

ed3abef

remove listen for changes

c16c326

vercel bot deployed to staging October 22, 2019 00:56 View deployment

gear btn

ea6f913

vercel bot deployed to staging October 22, 2019 01:02 View deployment

optimize

7e3323e

vercel bot deployed to staging October 22, 2019 01:14 View deployment

Merge remote-tracking branch 'origin/master' into simplify-extension

c5c217e

vercel bot deployed to staging October 22, 2019 01:20 View deployment

patrickhulce reviewed Oct 22, 2019

View reviewed changes

clients/extension/scripts/popup.js Outdated Show resolved Hide resolved

clients/extension/scripts/popup.js Outdated Show resolved Hide resolved

typo

fff5c8f

vercel bot deployed to staging October 22, 2019 02:25 View deployment

connorjclark closed this Oct 22, 2019

connorjclark reopened this Oct 22, 2019

paulirish approved these changes Oct 23, 2019

View reviewed changes

pr

088d526

vercel bot deployed to staging October 23, 2019 01:22 View deployment

brendankenny approved these changes Oct 25, 2019

View reviewed changes

brendankenny reviewed Oct 25, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into simplify-extension

aad5cd8

vercel bot deployed to staging October 25, 2019 19:00 View deployment

patrickhulce approved these changes Oct 26, 2019

View reviewed changes

brendankenny merged commit 2953c0c into master Oct 26, 2019

brendankenny deleted the simplify-extension branch October 26, 2019 17:59

connorjclark mentioned this pull request Feb 1, 2021

The install page for the Lighthouse extension in Chrome Web Store has wrong version #12033

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clients: retire extension; replace with PSI launcher #9193

clients: retire extension; replace with PSI launcher #9193

connorjclark commented Jun 13, 2019 •

edited

Loading

brendankenny commented Jun 13, 2019 •

edited

Loading

connorjclark commented Jun 13, 2019

patrickhulce commented Jun 13, 2019

patrickhulce commented Jun 13, 2019

connorjclark commented Jun 13, 2019

connorjclark commented Jun 17, 2019 •

edited

Loading

patrickhulce commented Jun 17, 2019

patrickhulce Jun 17, 2019

connorjclark Jun 17, 2019

connorjclark commented Jun 17, 2019

connorjclark commented Jun 17, 2019

connorjclark Jun 17, 2019

brendankenny commented Jun 18, 2019

brendankenny commented Jun 18, 2019 •

edited

Loading

connorjclark commented Jun 18, 2019 •

edited

Loading

patrickhulce left a comment

patrickhulce commented Oct 22, 2019

connorjclark commented Oct 23, 2019 •

edited

Loading

patrickhulce commented Oct 23, 2019

paulirish left a comment

paulirish Oct 23, 2019

connorjclark Oct 23, 2019

brendankenny left a comment

brendankenny commented Oct 25, 2019

brendankenny Oct 25, 2019

connorjclark commented Oct 26, 2019

patrickhulce left a comment

brendankenny commented Oct 26, 2019

brendankenny commented Oct 26, 2019

clients: retire extension; replace with PSI launcher #9193

clients: retire extension; replace with PSI launcher #9193

Conversation

connorjclark commented Jun 13, 2019 • edited Loading

brendankenny commented Jun 13, 2019 • edited Loading

connorjclark commented Jun 13, 2019

patrickhulce commented Jun 13, 2019

patrickhulce commented Jun 13, 2019

connorjclark commented Jun 13, 2019

connorjclark commented Jun 17, 2019 • edited Loading

patrickhulce commented Jun 17, 2019

patrickhulce Jun 17, 2019

Choose a reason for hiding this comment

connorjclark Jun 17, 2019

Choose a reason for hiding this comment

connorjclark commented Jun 17, 2019

connorjclark commented Jun 17, 2019

connorjclark Jun 17, 2019

Choose a reason for hiding this comment

brendankenny commented Jun 18, 2019

brendankenny commented Jun 18, 2019 • edited Loading

connorjclark commented Jun 18, 2019 • edited Loading

patrickhulce left a comment

Choose a reason for hiding this comment

patrickhulce commented Oct 22, 2019

connorjclark commented Oct 23, 2019 • edited Loading

patrickhulce commented Oct 23, 2019

paulirish left a comment

Choose a reason for hiding this comment

paulirish Oct 23, 2019

Choose a reason for hiding this comment

connorjclark Oct 23, 2019

Choose a reason for hiding this comment

brendankenny left a comment

Choose a reason for hiding this comment

brendankenny commented Oct 25, 2019

brendankenny Oct 25, 2019

Choose a reason for hiding this comment

connorjclark commented Oct 26, 2019

patrickhulce left a comment

Choose a reason for hiding this comment

brendankenny commented Oct 26, 2019

brendankenny commented Oct 26, 2019

connorjclark commented Jun 13, 2019 •

edited

Loading

brendankenny commented Jun 13, 2019 •

edited

Loading

connorjclark commented Jun 17, 2019 •

edited

Loading

brendankenny commented Jun 18, 2019 •

edited

Loading

connorjclark commented Jun 18, 2019 •

edited

Loading

connorjclark commented Oct 23, 2019 •

edited

Loading