Bug 1568831 - [devtools] Make eyedropper fission compatible. r=ladybenko.

The current implementation of the eyedropper was using drawWindow, which does
not handle remote frames.
To make the tool fission compatible, we re-use the screenshot-content actor which
do support remote frames. We had to do a few modification on it though so the
eyedropper would work the same as it does at the moment:
- add a `disableFlash` property to the screenshot options, so we don't play the
  flash animation for the eyedropper
- retrieve the page current zoom level from the screenshot-content actor so we
  can adjust the size of the resulting image
- split the `dpr` screenshot option into 2 distinct ones:
  - `snapshotScale`, which will be the scale passed to `drawSnapshot`. By default,
    this will be the dpr of the window, so the screenshot is as crisp as it can be.
  - `fileScale`, which will be the scale of the resulting file.
- we control the fileScale that is sent to the server with the `ignoreDprForFileScale`
  on the client. This is needed by the eyedropper so it doesn't draw images that are
  too big when the dpr is > 1.

With all those changes, browser_inspector_highlighter-eyedropper-frames.js is passing
with fission enabled, so we can remove the `fail-if` annotation it had.

Differential Revision: https://phabricator.services.mozilla.com/D103879
This commit is contained in:
Nicolas Chevobbe 2021-02-08 07:02:02 +00:00
Родитель 08409aaf66
Коммит 0d3a7b32b4
6 изменённых файлов: 180 добавлений и 80 удалений

Просмотреть файл

@ -12,6 +12,13 @@ const {
} = require("devtools/shared/protocol.js");
const { inspectorSpec } = require("devtools/shared/specs/inspector");
loader.lazyRequireGetter(
this,
"captureScreenshot",
"devtools/client/shared/screenshot",
true
);
const TELEMETRY_EYEDROPPER_OPENED = "DEVTOOLS_EYEDROPPER_OPENED_COUNT";
const TELEMETRY_EYEDROPPER_OPENED_MENU =
"DEVTOOLS_MENU_EYEDROPPER_OPENED_COUNT";
@ -156,7 +163,38 @@ class InspectorFront extends FrontClassWithSpec(inspectorSpec) {
}
async pickColorFromPage(options) {
await super.pickColorFromPage(options);
let screenshot = null;
// @backward-compat { version 87 } ScreenshotContentActor was only added in 87.
// When connecting to older server, the eyedropper will use drawWindow
// to retrieve the screenshot of the page (that's a decent fallback,
// even if it doesn't handle remote frames).
if (this.targetFront.hasActor("screenshotContent")) {
try {
// We use the screenshot actors as it can retrieve an image of the current viewport,
// handling remote frame if need be.
const { data } = await captureScreenshot(this.targetFront, {
browsingContextID: this.targetFront.browsingContextID,
disableFlash: true,
ignoreDprForFileScale: true,
});
screenshot = data;
} catch (e) {
// We simply log the error and still call pickColorFromPage as it will default to
// use drawWindow in order to get the screenshot of the page (that's a decent
// fallback, even if it doesn't handle remote frames).
console.error(
"Error occured when taking a screenshot for the eyedropper",
e
);
}
}
await super.pickColorFromPage({
...options,
screenshot,
});
if (options?.fromMenu) {
telemetry.getHistogramById(TELEMETRY_EYEDROPPER_OPENED_MENU).add(true);
} else {

Просмотреть файл

@ -120,7 +120,6 @@ skip-if = (os == 'linux' && bits == 32 && debug) # bug 1328915, disable linux32
[browser_inspector_highlighter-eyedropper-events.js]
skip-if = os == 'win' # bug 1413442
[browser_inspector_highlighter-eyedropper-frames.js]
fail-if = fission
[browser_inspector_highlighter-eyedropper-image.js]
[browser_inspector_highlighter-eyedropper-label.js]
[browser_inspector_highlighter-eyedropper-show-hide.js]

Просмотреть файл

@ -32,6 +32,12 @@ const L10N = new LocalizationHelper(STRINGS_URI);
* @param {Number} args.delay: Number of seconds to wait before taking the screenshot
* @param {Boolean} args.help: Set to true to receive a message with the screenshot command
* documentation.
* @param {Boolean} args.disableFlash: Set to true to disable the flash animation when the
* screenshot is taken.
* @param {Boolean} args.ignoreDprForFileScale: Set to true to if the resulting screenshot
* file size shouldn't be impacted by the dpr. Note that the dpr will still
* be taken into account when taking the screenshot, only the size of the
* file will be different.
* @returns {Array<Object{text, level}>} An array of object representing the different
* messages emitted throught the process, that should be displayed to the user.
*/
@ -41,58 +47,7 @@ async function captureAndSaveScreenshot(targetFront, window, args = {}) {
return [{ text: getFormattedHelpData() }];
}
// @backward-compat { version 87 } The screenshot-content actor was introduced in 87,
// so we can always use it once 87 reaches release.
const supportsContentScreenshot = targetFront.hasActor("screenshotContent");
let captureResponse;
if (supportsContentScreenshot) {
if (args.delay > 0) {
await new Promise(res => setTimeout(res, args.delay * 1000));
}
const screenshotContentFront = await targetFront.getFront(
"screenshot-content"
);
// Call the content-process on the server to retrieve informations that will be needed
// by the parent process.
const {
rect,
windowDpr,
messages,
error,
} = await screenshotContentFront.prepareCapture(args);
if (error) {
return messages;
}
if (rect) {
args.rect = rect;
}
if (!args.dpr) {
args.dpr = windowDpr;
}
args.browsingContextID = targetFront.browsingContextID;
// We can now call the parent process which will take the screenshot via
// the drawSnapshot API
const rootFront = targetFront.client.mainRoot;
const parentProcessScreenshotFront = await rootFront.getFront("screenshot");
captureResponse = await parentProcessScreenshotFront.capture(args);
messages.push(...(captureResponse.messages || []));
// Reset the page to the state it was in before taking the screenshot (e.g. set the
// scroll position as it was before). This is a oneway method so there's nothing to
// wait for.
screenshotContentFront.captureDone();
} else {
const screenshotFront = await targetFront.getFront("screenshot");
captureResponse = await screenshotFront.capture(args);
}
const captureResponse = await captureScreenshot(targetFront, args);
if (captureResponse.error) {
return captureResponse.messages || [];
@ -102,6 +57,72 @@ async function captureAndSaveScreenshot(targetFront, window, args = {}) {
return (captureResponse.messages || []).concat(saveMessages);
}
/**
* Take a screenshot of a browser element matching the passed target
* @param {TargetFront} targetFront: The targetFront of the frame we want to take a screenshot of.
* @param {Object} args: See args param in captureAndSaveScreenshot
*/
async function captureScreenshot(targetFront, args) {
// @backward-compat { version 87 } The screenshot-content actor was introduced in 87,
// so we can always use it once 87 reaches release.
const supportsContentScreenshot = targetFront.hasActor("screenshotContent");
if (!supportsContentScreenshot) {
const screenshotFront = await targetFront.getFront("screenshot");
return screenshotFront.capture(args);
}
if (args.delay > 0) {
await new Promise(res => setTimeout(res, args.delay * 1000));
}
const screenshotContentFront = await targetFront.getFront(
"screenshot-content"
);
// Call the content-process on the server to retrieve informations that will be needed
// by the parent process.
const {
rect,
windowDpr,
windowZoom,
messages,
error,
} = await screenshotContentFront.prepareCapture(args);
if (error) {
return { error, messages };
}
if (rect) {
args.rect = rect;
}
args.dpr ||= windowDpr;
args.snapshotScale = args.dpr * windowZoom;
if (args.ignoreDprForFileScale) {
args.fileScale = windowZoom;
}
args.browsingContextID = targetFront.browsingContextID;
// We can now call the parent process which will take the screenshot via
// the drawSnapshot API
const rootFront = targetFront.client.mainRoot;
const parentProcessScreenshotFront = await rootFront.getFront("screenshot");
const captureResponse = await parentProcessScreenshotFront.capture(args);
// Reset the page to the state it was in before taking the screenshot (e.g. set the
// scroll position as it was before). This is a oneway method so there's nothing to
// wait for.
screenshotContentFront.captureDone();
return {
...captureResponse,
messages: (messages || []).concat(captureResponse.messages || []),
};
}
const screenshotDescription = L10N.getStr("screenshotDesc");
const screenshotGroupOptions = L10N.getStr("screenshotGroupOptions");
const screenshotCommandParams = [
@ -362,4 +383,8 @@ async function saveToFile(image) {
}
}
module.exports = { captureAndSaveScreenshot, saveScreenshot };
module.exports = {
captureAndSaveScreenshot,
captureScreenshot,
saveScreenshot,
};

Просмотреть файл

@ -134,9 +134,13 @@ class EyeDropper {
/**
* Show the eye-dropper highlighter.
*
* @param {DOMNode} node The node which document the highlighter should be inserted in.
* @param {Object} options The options object may contain the following properties:
* - {Boolean} copyOnSelect Whether selecting a color should copy it to the clipboard.
* - {Boolean} copyOnSelect: Whether selecting a color should copy it to the clipboard.
* - {String|null} screenshot: a dataURL representation of the page screenshot. If null,
* the eyedropper will use `drawWindow` to get the the screenshot
* ( but it won't handle remote frames).
*/
show(node, options = {}) {
if (this.highlighterEnv.isXUL) {
@ -152,7 +156,7 @@ class EyeDropper {
// eyedropper UI will appear in the screenshot itself (since the UI is injected as
// native anonymous content in the page).
// Once the screenshot is ready, the magnified area will be drawn.
this.prepareImageCapture();
this.prepareImageCapture(options.screenshot);
// Start listening for user events.
const { pageListenerTarget } = this.highlighterEnv;
@ -216,22 +220,41 @@ class EyeDropper {
this.win.document.setSuppressedEventListener(null);
}
prepareImageCapture() {
// Get the image data from the content window.
const imageData = getWindowAsImageData(this.win);
/**
* Create an image bitmap from the page screenshot, draw the eyedropper and set the
* "drawn" attribute on the "root" element once it's done.
*
* @params {String|null} screenshot: a dataURL representation of the page screenshot.
* If null, we'll use `drawWindow` to get the the page screenshot
* ( but it won't handle remote frames).
*/
async prepareImageCapture(screenshot) {
let imgData;
if (screenshot) {
// If a screenshot data URL was passed, we create an image tag with it which we
// use to create an image bitmap.
imgData = this.win.document.createElement("img");
const onImgLoaded = new Promise(resolve =>
imgData.addEventListener("load", resolve, { once: true })
);
imgData.src = screenshot;
await onImgLoaded;
} else {
imgData = getWindowAsImageData(this.win);
}
// We need to transform imageData to something drawWindow will consume. An ImageBitmap
// works well. We could have used an Image, but doing so results in errors if the page
// defines CSP headers.
this.win.createImageBitmap(imageData).then(image => {
this.pageImage = image;
// We likely haven't drawn anything yet (no mousemove events yet), so start now.
this.draw();
const image = await this.win.createImageBitmap(imgData);
// Set an attribute on the root element to be able to run tests after the first draw
// was done.
this.getElement("root").setAttribute("drawn", "true");
});
this.pageImage = image;
// We likely haven't drawn anything yet (no mousemove events yet), so start now.
this.draw();
// Set an attribute on the root element to be able to run tests after the first draw
// was done.
this.getElement("root").setAttribute("drawn", "true");
}
/**

Просмотреть файл

@ -11,7 +11,12 @@ const {
const { LocalizationHelper } = require("devtools/shared/l10n");
const STRINGS_URI = "devtools/shared/locales/screenshot.properties";
const L10N = new LocalizationHelper(STRINGS_URI);
loader.lazyRequireGetter(this, "getRect", "devtools/shared/layout/utils", true);
loader.lazyRequireGetter(
this,
["getCurrentZoom", "getRect"],
"devtools/shared/layout/utils",
true
);
exports.ScreenshotContentActor = ActorClassWithSpec(screenshotContentSpec, {
initialize: function(conn, targetActor) {
@ -41,6 +46,7 @@ exports.ScreenshotContentActor = ActorClassWithSpec(screenshotContentSpec, {
* - messages {Array<Object{text, level}>}: An array of objects representing
* the messages emitted throught the process and their level.
* - windowDpr {Number}: Value of window.devicePixelRatio
* - windowZoom {Number}: The page current zoom level
* - rect {Object}: Object with left, top, width and height properties
* representing the rect **inside the browser element** that should be rendered.
* For screenshot of the current viewport, we return null, as expected by the
@ -51,6 +57,7 @@ exports.ScreenshotContentActor = ActorClassWithSpec(screenshotContentSpec, {
const { window } = this.targetActor;
const windowDpr = window.devicePixelRatio;
const windowZoom = getCurrentZoom(window);
const messages = [];
// If we're going to take the current view of the page, we don't need to compute a rect,
@ -60,6 +67,7 @@ exports.ScreenshotContentActor = ActorClassWithSpec(screenshotContentSpec, {
rect: null,
messages,
windowDpr,
windowZoom,
};
}
@ -127,6 +135,7 @@ exports.ScreenshotContentActor = ActorClassWithSpec(screenshotContentSpec, {
return {
windowDpr,
windowZoom,
rect: { left, top, width, height },
messages,
};

Просмотреть файл

@ -57,9 +57,13 @@ function simulateCameraFlash(browsingContext) {
* be rendered. If null, the current viewport of the element will be rendered.
* @param {Boolean} args.fullpage: Should the screenshot be the height of the whole page
* @param {String} args.filename: Expected filename for the screenshot
* @param {Number} args.dpr: Scale of the screenshot. Note that the scale might be
* decreased if the resulting image would be too big to draw safely.
* Warning message will be returned if that's the case.
* @param {Number} args.snapshotScale: Scale that will be used by `drawSnapshot` to take the screenshot.
* Note that the scale might be decreased if the resulting image would
* be too big to draw safely. A warning message will be returned if that's
* the case.
* @param {Number} args.fileScale: Scale of the exported file. Defaults to args.snapshotScale.
* @param {Boolean} args.disableFlash: Set to true to disable the flash animation when the
* screenshot is taken.
* @param {BrowsingContext} browsingContext
* @returns {Object} object with the following properties:
* - data {String}: The dataURL representing the screenshot
@ -100,8 +104,6 @@ async function captureScreenshot(args, browsingContext) {
);
}
const ratio = args.dpr ? args.dpr : 1;
const document = browsingContext.topChromeWindow.document;
const canvas = document.createElementNS(
"http://www.w3.org/1999/xhtml",
@ -119,12 +121,15 @@ async function captureScreenshot(args, browsingContext) {
"rgb(255,255,255)"
);
canvas.width = snapshot.width;
canvas.height = snapshot.height;
width = snapshot.width;
height = snapshot.height;
const fileScale = args.fileScale || actualRatio;
const renderingWidth = (snapshot.width / actualRatio) * fileScale;
const renderingHeight = (snapshot.height / actualRatio) * fileScale;
canvas.width = renderingWidth;
canvas.height = renderingHeight;
width = renderingWidth;
height = renderingHeight;
const ctx = canvas.getContext("2d");
ctx.drawImage(snapshot, 0, 0);
ctx.drawImage(snapshot, 0, 0, renderingWidth, renderingHeight);
// Bug 1574935 - Huge dimensions can trigger an OOM because multiple copies
// of the bitmap will exist in memory. Force the removal of the snapshot
@ -137,6 +142,7 @@ async function captureScreenshot(args, browsingContext) {
}
};
const ratio = args.snapshotScale;
let data = await drawToCanvas(ratio);
if (!data && ratio > 1.0) {
// If the user provided DPR or the window.devicePixelRatio was higher than 1,
@ -154,7 +160,7 @@ async function captureScreenshot(args, browsingContext) {
});
}
if (data) {
if (data && args.disableFlash !== true) {
simulateCameraFlash(browsingContext);
}