w3c / webdriver

Remote control interface that enables introspection and control of user agents.
https://w3c.github.io/webdriver/
Other
681 stars 194 forks source link

Need to wait for a paint before taking a screenshot #895

Open jgraham opened 7 years ago

jgraham commented 7 years ago

https://github.com/jugglinmike/chrome-screenshot-race/issues/1 details an issue where using ChromeDriver to take screenshots on page load produces unreliable behaviour because it isn't waiting for the content to paint before taking the screenshot. The specification should require webdriver implementations to wait for a paint before taking the screenshot, otherwise the implementation isn't reliable.

cc @jugglinmike

gsnedders commented 7 years ago

Related to #893.

jgraham commented 7 years ago

This is related to #893 but somewhat different because it isn't about resources that aren't loaded by load but about the fact that load firing doesn't guarantee a paint has happened.

andreastt commented 7 years ago

Would adding the step

Wait until the user agent event loop has spun enough times to process the DOM events generated by the previous step.

from the Element Click command help?

We are somewhat vague about what that means, but in practice, what Marionette does wait for window.requestAnimationFrame to return:

interaction.flushEventLoop = function* (win) {
  let unloadEv;

  return new Promise((resolve, reject) => {
    if (win.closed) {
      reject();
      return;
    }

    unloadEv = reject;
    win.addEventListener("unload", unloadEv, {once: true});

    win.requestAnimationFrame(resolve);
  }).then(() => {
    win.removeEventListener("unload", unloadEv);
  });
};
jgraham commented 7 years ago

No. In gecko terms we should wait for MozAfterPaint (although I think that rAF is equivalent). But I'm not sure if it's guaranteed that a paint actually happens before that runs. The spec sort of implies that it probably ought to, but maybe in practice it doesn't.

shs96c commented 7 years ago

We now using the same language as requestAnimationFrame, so there shouldn't (ha!) be pending paint events. Waiting for all content to be loaded (including non-critical ones) is out of scope for Level 1. Pushing to level 2.

foolip commented 6 years ago

@Hexcles and I are discussing how to deal with https://crbug.com/507054 and need to understand the exact timing of Take Screenshot to do that.

In gecko terms we should wait for MozAfterPaint (although I think that rAF is equivalent)

@jgraham, this isn't the case I think. rAF is a before layout/paint callback, as can be seen in https://html.spec.whatwg.org/multipage/webappapis.html#update-the-rendering. "run the animation frame callbacks" comes before "update the rendering" with no async-y bits in between, so AFAICT the current WebDriver spec guarantees that the screenshot is right before the next paint, if we take "update the rendering" to mean paint. (@chrishtr, is that right?)

The way that "Take Screenshot" is implemented in ChromeDriver is to force a redraw in paint, here: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?l=1615&rcl=f04536cae9016aabeb9e45eff1364b0e90e90838 https://cs.chromium.org/chromium/src/content/renderer/render_view_impl.cc?l=1244&rcl=2faeeb15178698f9bd50db3c334e1db4df03fc10

So... I think that to match this WebDriver would be to, in the "remote end steps", not say "When the user agent is next to run the animation frame callbacks" but rather have a hook after the last step of the "update the rendering" steps in HTML. And perhaps something to cause those steps to run, since they may never run again otherwise.

@andreastt, would that match what's implemented in GeckoDriver and Marionette, or what timing do you have?

andreastt commented 6 years ago

Gecko doesn’t currently have any synchronisation, i.e. does not conform to WebDriver here.

foolip commented 6 years ago

Will it just grab whatever was last painted then, so not forcing a new layout or paint? Do you think that's a better behavior?