Opened on 02/11/2019 at 11:56:04 AM
Closed on 03/13/2019 at 10:49:51 AM
Last modified on 07/22/2019 at 08:29:24 AM
#7271 closed change (fixed)
Adjust Chrome API calls to use Promises where possible
| Reported by: | kzar | Assignee: | kzar |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-for-Chrome-Opera-Firefox-next |
| Module: | Platform | Keywords: | manifestv3 |
| Cc: | sebastian, greiner | Blocked By: | |
| Blocking: | #7334 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): |
https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/42 |
||
Description (last modified by kzar)
Background
For version 3 of Chrome's extension manifest format, one planned change is to switch from callbacks to Promises for the Chrome APIs. From the draft spec:
Extension APIs will be promise-based. The older callback version will continue to be supported...
In order to maintain backwards compatibility (and not force developers to rewrite their extension
more than they already have to), providing a callback to an API method will continue to work. If
a callback is provided, a promise will not be returned.
What to change
- Where possible use Promises instead of callbacks when using the browser.* or chrome.* APIs.
- Rewrite the lib/options.js code around promises.
- Remove Page.prototype.sendMessage, replacing usage with browser.tabs.sendMessage.
Tip: Use this patch to find all the calls that need to be fixed.
Hints for testers
Unfortunately a whole bunch of code paths were touched with these changes. As far as possible please smoke test all functionality with Edge, old version of Chrome, new version of Chrome, Firefox on mobile, old version of Firefox desktop and new version of Firefox desktop.
A (not complete) list of functionality that was touched:
- Logic to open options page, both from popup, notifications and subscription links. Including focusing the page if it's already open (even if that's in a different window), logic to add dialogues e.g. when clicking on subscription links. There are quite a few different code paths, so it's important to test thoroughly with the different browsers and version listed above.
- The options page itself.
- "Block element" tool, both from context menu and popup.
- Our developer tools panel.
- Collapsing (hiding) of blocked elements.
- Element hiding.
- WebRTC blocking.
- ABP icon animations (e.g. when a critical notification is received).
- Block counter, including after the preference to show it is disabled then enabled again.
- Options page opening correctly on mobile vs desktop.
Notes
- See #7272 for the equivalent adblockplusui changes.
Attachments (0)
Change History (20)
comment:2 Changed on 02/11/2019 at 12:30:02 PM by greiner
comment:5 Changed on 03/05/2019 at 03:11:49 PM by kzar
- Blocked By 7327 added
comment:8 Changed on 03/05/2019 at 03:42:01 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:9 Changed on 03/06/2019 at 08:30:51 AM by kzar
- Blocking 7334 added
comment:10 Changed on 03/06/2019 at 10:58:55 AM by mjethani
- Description modified (diff)
comment:11 Changed on 03/07/2019 at 05:08:24 PM by kzar
- Description modified (diff)
comment:12 Changed on 03/07/2019 at 05:09:57 PM by kzar
- Description modified (diff)
comment:13 Changed on 03/11/2019 at 11:50:04 AM by kzar
- Description modified (diff)
comment:14 Changed on 03/11/2019 at 11:50:54 AM by kzar
- Description modified (diff)
comment:15 Changed on 03/11/2019 at 11:54:43 AM by kzar
- Blocking 7350 added
comment:16 Changed on 03/12/2019 at 04:55:50 PM by kzar
- Blocking 7350 removed
comment:17 Changed on 03/13/2019 at 10:46:53 AM by abpbot
Some commits referencing this issue have landed:
comment:18 Changed on 03/13/2019 at 10:49:51 AM by kzar
- Blocked By 7327 removed
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:19 Changed on 03/13/2019 at 01:44:32 PM by abpbot
A commit referencing this issue has landed:
Issue 7271 - Ensure port is provided by showOptions where necessary
comment:20 Changed on 07/22/2019 at 08:29:24 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Does not look to have caused any regressions.
ABP 0.9.15.2339
Microsoft Edge 44.17763.1.0 / Windows 10 1809
ABP 3.5.2.2339
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
For reference, I've created #7272 for doing the same in adblockplusui.