[Obsolete] Fix messaging in chrome extension pages (#2846) #2863

enkot · 2020-04-03T10:55:17Z

Fixed problem with not working onMessage handler in chrome extension pages.

googlebot · 2020-04-03T10:55:21Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

enkot · 2020-04-03T11:00:28Z

@googlebot I signed it!

googlebot · 2020-04-03T11:00:31Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

zwu52

Hi Enkot, you were correct that the current SDK isn't working for Chrome Extension.
Thanks for making the fix and tests w/ it 👍

zwu52 · 2020-04-06T21:31:16Z

packages/messaging/src/controllers/sw-controller.test.ts

@@ -218,6 +218,30 @@ describe('SwController', () => {
      expect(postMessageSpy).to.have.been.calledOnceWith(expectedMessage);
    });

+    it('sends a message to chrome extension clients if a chrome extension client is visible', async () => {
+      const client: Writable<WindowClient> = (await self.clients.openWindow(
+        'chrome-extension://aihpiglmnhnhijdnjghpfnlledckkhja/popup.html'


looks like a real extension id. can we have it to be more generic? something like "chrome-extension://testExtensionId/popup.html" or whatever.

zwu52 · 2020-04-06T21:43:07Z

packages/messaging/src/controllers/sw-controller.ts

@@ -297,7 +297,7 @@ function hasVisibleClients(clientList: WindowClient[]): boolean {
      client.visibilityState === 'visible' &&
      // Ignore chrome-extension clients as that matches the background pages
      // of extensions, which are always considered visible for some reason.
-      !client.url.startsWith('chrome-extension://')
+      !(client.url.startsWith('chrome-extension://') && client.url.endsWith("_generated_background_page.html"))


Looks like this will work for auto-gen background.html. The next step is probably to have a more generic solution that would work for all situation (user defined background html) and across browser if applicable 😎

zwu52 · 2020-04-06T21:47:48Z

packages/messaging/src/controllers/sw-controller.test.ts

@@ -241,6 +265,32 @@ describe('SwController', () => {
      });
    });

+    it('does not send a message to chrome extension background pages if background page is visible', async () => {
+      const client: Writable<WindowClient> = (await self.clients.openWindow(
+        'chrome-extension://aihpiglmnhnhijdnjghpfnlledckkhja/_generated_background_page.html'


enkot · 2020-04-25T09:59:10Z

Hi @zwu52, can you please review last changes?

zwu52

Enkot,
apologies for the delay. LGTM. Made some minor nits.

zwu52 · 2020-04-27T16:34:23Z

packages/messaging/src/controllers/sw-controller.ts

+ * @returns If client is the background page of browser extension, this method will
+ * resolve to true, otherwise false.
+ */
+async function isBackgroundClient(client: WindowClient): Promise<boolean> {


nit: how about isBackgroundExtension or isExtensionBackgroundClient. Just to make clear we're talking in the extension context

zwu52 · 2020-04-27T17:03:18Z

packages/messaging/src/controllers/sw-controller.ts

+    try {
+      const backgroundClient = await runtime.getBackgroundClient();
+      return client === backgroundClient;
+    } catch {


Enkot,
Can we have a debug log here on the error and error message so it would be easier for folks to debug if they hit this path and wanted to get information?

enkot · 2020-05-21T09:45:50Z

Hi @zwu52,
Maybe we can somehow boost review process?
That's small fix, but will allow a lot of people to use messaging in their browser extensions :)

zwu52 · 2020-05-21T16:00:02Z

Enkot,
Thanks for the feedback and help on improving the product. Agreed. I have been working on iterating towards a process where git repo issues are getting a high priority in my task stack :)

enkot · 2020-07-28T14:52:51Z

Hi @zwu52,

Do we have any chance to merge this PR?

zwu52 · 2020-09-09T23:34:31Z

Hi Enkot, apologies that I didn't realize the PR wasn't checked in. Thanks for pinging. This PR had some conflicts so I made this based on your PR. Feel free to use it to update your PR and I'll merge your PR.

Fix messaging in chrome extension pages (firebase#2846)

f6d27c9

Feiyang1 assigned zwu52 Apr 3, 2020

zwu52 requested changes Apr 6, 2020

View reviewed changes

enkot added 4 commits April 11, 2020 22:27

Merge branch 'master' into fixMessagingChromeExtension

c49ca5e

background script detection using "getBackgroundClient"

369a29d

[AUTOMATED]: Prettier Code Styling

aa1d476

[AUTOMATED]: License Headers

6b293a8

enkot requested review from Feiyang1, hiranya911 and hsubox76 as code owners April 11, 2020 21:35

"isBrowserExtension" based on "getBrowserExtensionRuntime"

12a5ecd

enkot requested review from alikn, egilmorez, jsdt, schmidt-sebastian and zijianjoy as code owners April 11, 2020 21:49

enkot requested a review from zwu52 April 13, 2020 10:19

zwu52 approved these changes Apr 27, 2020

View reviewed changes

renamed "isBackgroundClient" to "isExtensionBackgroundClient"

4563bab

zwu52 mentioned this pull request Sep 9, 2020

Resolve conflicts introduced since last request of merge #3756

Closed

zwu52 changed the title ~~Fix messaging in chrome extension pages (#2846)~~ [Obsolete] Fix messaging in chrome extension pages (#2846) Apr 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Obsolete] Fix messaging in chrome extension pages (#2846) #2863

[Obsolete] Fix messaging in chrome extension pages (#2846) #2863

enkot commented Apr 3, 2020

googlebot commented Apr 3, 2020

enkot commented Apr 3, 2020

googlebot commented Apr 3, 2020

zwu52 left a comment

zwu52 Apr 6, 2020

zwu52 Apr 6, 2020

zwu52 Apr 6, 2020

enkot commented Apr 25, 2020

zwu52 left a comment

zwu52 Apr 27, 2020

zwu52 Apr 27, 2020

enkot commented May 21, 2020 •

edited

Loading

zwu52 commented May 21, 2020

enkot commented Jul 28, 2020

zwu52 commented Sep 9, 2020

[Obsolete] Fix messaging in chrome extension pages (#2846) #2863

Are you sure you want to change the base?

[Obsolete] Fix messaging in chrome extension pages (#2846) #2863

Conversation

enkot commented Apr 3, 2020

googlebot commented Apr 3, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

enkot commented Apr 3, 2020

googlebot commented Apr 3, 2020

zwu52 left a comment

Choose a reason for hiding this comment

zwu52 Apr 6, 2020

Choose a reason for hiding this comment

zwu52 Apr 6, 2020

Choose a reason for hiding this comment

zwu52 Apr 6, 2020

Choose a reason for hiding this comment

enkot commented Apr 25, 2020

zwu52 left a comment

Choose a reason for hiding this comment

zwu52 Apr 27, 2020

Choose a reason for hiding this comment

zwu52 Apr 27, 2020

Choose a reason for hiding this comment

enkot commented May 21, 2020 • edited Loading

zwu52 commented May 21, 2020

enkot commented Jul 28, 2020

zwu52 commented Sep 9, 2020

enkot commented May 21, 2020 •

edited

Loading