Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29730652: Issue 4580 - Stop using ext.onMessage in background page

Created:
March 22, 2018, 7:53 p.m. by Manish Jethani
Modified:
March 22, 2018, 7:56 p.m.
Reviewers:
Sebastian Noack
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Issue 4580 - Stop using ext.onMessage in background page

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -72 lines) Patch
M ext/background.js View 5 chunks +33 lines, -45 lines 1 comment Download
M lib/messaging.js View 2 chunks +19 lines, -27 lines 2 comments Download

Messages

Total messages: 2
Manish Jethani
March 22, 2018, 7:53 p.m. (2018-03-22 19:53:26 UTC) #1
Manish Jethani
March 22, 2018, 7:56 p.m. (2018-03-22 19:56:55 UTC) #2
Patch Set 1

Note: once this is done, I'll make changes for the content script (which is more
work) and elsewhere.

https://codereview.adblockplus.org/29730652/diff/29730653/ext/background.js
File ext/background.js (right):

https://codereview.adblockplus.org/29730652/diff/29730653/ext/background.js#n...
ext/background.js:512: let Frame = ext.Frame = function(tabId, frameId, url)
So there's a Frame object now and it's used consistently everywhere.

https://codereview.adblockplus.org/29730652/diff/29730653/lib/messaging.js
File lib/messaging.js (right):

https://codereview.adblockplus.org/29730652/diff/29730653/lib/messaging.js#ne...
lib/messaging.js:39: // Add "page" and "frame" if the message was sent by a
content script.
This used to be in ext/background.js

https://codereview.adblockplus.org/29730652/diff/29730653/lib/messaging.js#ne...
lib/messaging.js:49: // The message sender receives only the first response.
Note: using Promise.race here to resolve with first resolved value, which is
equivalent to what was happening earlier, but we could also just use Promise.all
and return all resolved values (but this will require more changes elsewhere and
also the response might take longer).

Powered by Google App Engine
This is Rietveld