Opened 5 months ago

Closed 4 months ago

Last modified 4 months ago

#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: Unknown Verified working: no
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.

Change History (19)

comment:1 Changed 5 months ago by kzar

  • Description modified (diff)

comment:2 Changed 5 months ago by greiner

For reference, I've created #7272 for doing the same in adblockplusui.

comment:3 Changed 5 months ago by kzar

  • Description modified (diff)

comment:4 Changed 5 months ago by kzar

  • Description modified (diff)

comment:5 Changed 4 months ago by kzar

  • Blocked By 7327 added

comment:6 Changed 4 months ago by kzar

  • Description modified (diff)

comment:7 Changed 4 months ago by kzar

  • Description modified (diff)

comment:8 Changed 4 months ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:9 Changed 4 months ago by kzar

  • Blocking 7334 added

comment:10 Changed 4 months ago by mjethani

  • Description modified (diff)

comment:11 Changed 4 months ago by kzar

  • Description modified (diff)

comment:12 Changed 4 months ago by kzar

  • Description modified (diff)

comment:13 Changed 4 months ago by kzar

  • Description modified (diff)

comment:14 Changed 4 months ago by kzar

  • Description modified (diff)

comment:15 Changed 4 months ago by kzar

  • Blocking 7350 added

comment:16 Changed 4 months ago by kzar

  • Blocking 7350 removed

comment:18 Changed 4 months ago 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 4 months ago by abpbot

Note: See TracTickets for help on using tickets.