Make about:debugging webextension popup test able to run with oop extension mode enabled

RESOLVED FIXED in Firefox 57

Status

enhancement
RESOLVED FIXED
2 years ago
10 months ago

People

(Reporter: rpl, Assigned: rpl)

Tracking

unspecified
Firefox 57

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
As described in Bug 1382968 Comment 54, we need to rewrite the about:debugging webextension popup debugging test so that it doesn't use the console messages as a trick to coordinate the test steps happening in the processes involved in the test (main process, browser toolbox process and extension process) and it can work correctly when the extension is running oop even after Bug 1382968 has been landed.
(Assignee)

Updated

2 years ago
Blocks: 1382968
(Assignee)

Updated

2 years ago
Attachment #8895372 - Flags: review?(poirot.alex)
(Assignee)

Updated

2 years ago
Assignee: nobody → lgreco
Status: NEW → ASSIGNED

Comment 2

2 years ago
mozreview-review
Comment on attachment 8895372 [details]
Bug 1388737 - Make about:debugging webextension popup test able to run with oop extension mode enabled.

https://reviewboard.mozilla.org/r/166562/#review171702

Thanks a lot for improving this test so quickly!
r+, assuming you also got a green try without bug 1382968's patches.

::: devtools/client/aboutdebugging/test/browser_addons_debug_webextension_popup.js:76
(Diff revision 1)
> +    onPopupCustomMessage = waitForExtensionTestMessage("popupPageFunctionCalled");
>    });
>  
> +  let {
> +    tab, document, debugBtn,
> +  } = yield setupTestAboutDebuggingWebExtension(ADDON_NAME, ADDON_MANIFEST_PATH);

I was wondering if we should ensure Management's "startup" event fired before proceeding. But I imagine setupTestAboutDebuggingWebExtension may alreay wait correctly for full addon installation?

::: devtools/client/aboutdebugging/test/browser_addons_debug_webextension_popup.js:114
(Diff revision 1)
> -        return waitForFrameListUpdate;
> +          waitForFrameListUpdate,
> -      })
> -      .then((console) => {
> -        // Wait the new frame update (once the extension popup has been opened).
> +          // Wait the new frame update (once the extension popup has been opened).
> -        return popupFramePromise;
> -      })
> +          popupFramePromise,
> +        ]);

nit: I'm wondering if the following is better?
  await waitForFrameListUpdate;
  await popupFramePromise;
Attachment #8895372 - Flags: review?(poirot.alex) → review+
(Assignee)

Comment 3

2 years ago
mozreview-review-reply
Comment on attachment 8895372 [details]
Bug 1388737 - Make about:debugging webextension popup test able to run with oop extension mode enabled.

https://reviewboard.mozilla.org/r/166562/#review171702

> I was wondering if we should ensure Management's "startup" event fired before proceeding. But I imagine setupTestAboutDebuggingWebExtension may alreay wait correctly for full addon installation?

yeah, setupTestAboutDebuggingWebExtension uses this event internally to be able to wait for the full extension startup (http://searchfox.org/mozilla-central/rev/0f16d437cce97733c6678d29982a6bcad49f817b/devtools/client/aboutdebugging/test/head.js#173-182).

(in a followup it would be nice to extract a reusable helper for this and share it with the other tests).

> nit: I'm wondering if the following is better?
>   await waitForFrameListUpdate;
>   await popupFramePromise;

In this case it should be basically the same thing (especially given that both the Promises have been created above), I've no problem with changing it into two explicit await statements, though.

Comment 4

2 years ago
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/5794894b2ef7
Make about:debugging webextension popup test able to run with oop extension mode enabled. r=ochameau

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5794894b2ef7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.