Open Bug 1377335 Opened 2 years ago Updated 7 days ago

Re-enable DRAWWINDOW_DRAW_VIEW flag for Marionette screen captures

Categories

(Testing :: Marionette, enhancement, P3)

Version 3
enhancement

Tracking

(Not tracked)

People

(Reporter: ato, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

As noted in a TODO in testing/marionette/capture.js, we have disabled two flags on the canvas context when taking screenshots:

>   let ctx = canvas.getContext(CONTEXT_2D);
>   if (flags === null) {
>     flags = ctx.DRAWWINDOW_DRAW_CARET;
>     // Disabled in bug 1243415 for webplatform-test
>     // failures due to out of view elements.  Needs
>     // https://github.com/w3c/web-platform-tests/issues/4383 fixed.
>     /*
>     ctx.DRAWWINDOW_DRAW_VIEW;
>     */
>     // Bug 1009762 - Crash in [@ mozilla::gl::ReadPixelsIntoDataSurface]
>     /*
>     ctx.DRAWWINDOW_USE_WIDGET_LAYERS;
>     */
>   }

We want to investigate re-enabling these.
You could try to test in a try push but the underlying crash is not fixed yet, and as such it should still appear.
Priority: -- → P3

Note that with the patch on bug 1559592 landed this bug will become invalid.

Depends on: 1559592

Actually let me fix that bug first, which will help me with the refactoring on bug 1559592 afterward.

Assignee: nobody → hskupin
Blocks: 1559592
Status: NEW → ASSIGNED
No longer depends on: 1559592
Priority: P3 → P1

Note that both flags can be re-enabled now given that all dependencies have been fixed, or are no longer reproducible.

All dependencies are fixed, or no longer reproducible. As such both
flags can be re-enabled. Also reftests won't have to specify the
exactly same flags on its own anymore.

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4dab6e3dc6ed
[marionette] Re-enable DRAWWINDOW_DRAW_VIEW and DRAWWINDOW_USE_WIDGET_LAYERS flags for screen captures. r=webdriver-reviewers,ato
Status: ASSIGNED → RESOLVED
Closed: 20 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

I would suggest to backout this patch given the fact that doing full screenshots are not working with the DRAWWINDOW_DRAW_VIEW flag not being present. There is a strange behavior change in combination with DRAWWINDOW_USE_WIDGET_LAYERS maybe. And given that we shortly change the API to drawSnapshot() we could try to get it working properly there again.

Flags: needinfo?(sheriffs)
Status: RESOLVED → REOPENED
Flags: needinfo?(sheriffs) → needinfo?(hskupin)
Resolution: FIXED → ---
Target Milestone: mozilla70 → ---
No longer regressions: 1571659

We will have to wait for bug 1571341 before we can try to enable the flags again.

Assignee: hskupin → nobody
Status: REOPENED → NEW
Depends on: 1571341
Flags: needinfo?(hskupin)
Priority: P1 → P3

The new API only got the DRAWWINDOW_DRAW_VIEW flag. Using the widget one isn't necessary because drawSnapshot doesn't readback the data from the compositor.

Given that bug 1559592 hasn't been finished yet, and I don't want to cause a regression with its landing, I will most likely have to take care of it now. So lets mark this bug dependent on it instead.

No longer blocks: 1559592
Depends on: 1559592
Summary: Re-enable DRAWWINDOW_DRAW_VIEW and DRAWWINDOW_USE_WIDGET_LAYERS flags for Marionette screen captures → Re-enable DRAWWINDOW_DRAW_VIEW flag for Marionette screen captures
You need to log in before you can comment on or make changes to this bug.