From cf6abb812c07a72a2a800aa635a42a1d17d68b34 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 31 Mar 2022 10:13:03 +0200 Subject: [PATCH 1/2] fix: BrowserView background color in webContents * chore: fix BrowserView background color in webContents * disable screen capture test on linux * spec: fix platform failure condition --- .../browser/api/electron_api_browser_view.cc | 21 ++++++- .../browser/api/electron_api_web_contents.cc | 10 ++-- spec-main/api-browser-view-spec.ts | 55 ++++++++++++++++++- 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/shell/browser/api/electron_api_browser_view.cc b/shell/browser/api/electron_api_browser_view.cc index 7193c00fa1c5e..60009ffedaa6e 100644 --- a/shell/browser/api/electron_api_browser_view.cc +++ b/shell/browser/api/electron_api_browser_view.cc @@ -6,10 +6,13 @@ #include +#include "content/browser/renderer_host/render_widget_host_view_base.h" // nogncheck +#include "content/public/browser/render_widget_host_view.h" #include "shell/browser/api/electron_api_web_contents.h" #include "shell/browser/browser.h" #include "shell/browser/native_browser_view.h" #include "shell/browser/ui/drag_util.h" +#include "shell/browser/web_contents_preferences.h" #include "shell/common/color_util.h" #include "shell/common/gin_converters/gfx_converter.h" #include "shell/common/gin_helper/dictionary.h" @@ -155,11 +158,25 @@ gfx::Rect BrowserView::GetBounds() { } void BrowserView::SetBackgroundColor(const std::string& color_name) { - view_->SetBackgroundColor(ParseHexColor(color_name)); + SkColor color = ParseHexColor(color_name); + view_->SetBackgroundColor(color); if (web_contents()) { auto* wc = web_contents()->web_contents(); - wc->SetPageBaseBackgroundColor(ParseHexColor(color_name)); + wc->SetPageBaseBackgroundColor(color); + + auto* const rwhv = wc->GetRenderWidgetHostView(); + if (rwhv) { + rwhv->SetBackgroundColor(color); + static_cast(rwhv) + ->SetContentBackgroundColor(color); + } + + // Ensure new color is stored in webPreferences, otherwise + // the color will be reset on the next load via HandleNewRenderFrame. + auto* web_preferences = WebContentsPreferences::From(wc); + if (web_preferences) + web_preferences->SetBackgroundColor(color); } } diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 1e4160a370f5d..92889bad8c3ef 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -1463,11 +1463,13 @@ void WebContents::HandleNewRenderFrame( // Set the background color of RenderWidgetHostView. auto* web_preferences = WebContentsPreferences::From(web_contents()); if (web_preferences) { + absl::optional maybe_color = web_preferences->GetBackgroundColor(); + web_contents()->SetPageBaseBackgroundColor(maybe_color); + bool guest = IsGuest() || type_ == Type::kBrowserView; - absl::optional color = - guest ? SK_ColorTRANSPARENT : web_preferences->GetBackgroundColor(); - web_contents()->SetPageBaseBackgroundColor(color); - SetBackgroundColor(rwhv, color.value_or(SK_ColorWHITE)); + SkColor color = + maybe_color.value_or(guest ? SK_ColorTRANSPARENT : SK_ColorWHITE); + SetBackgroundColor(rwhv, color); } if (!background_throttling_) diff --git a/spec-main/api-browser-view-spec.ts b/spec-main/api-browser-view-spec.ts index 8d54b9efe1cae..3352c06e991a4 100644 --- a/spec-main/api-browser-view-spec.ts +++ b/spec-main/api-browser-view-spec.ts @@ -1,9 +1,10 @@ import { expect } from 'chai'; import * as path from 'path'; import { emittedOnce } from './events-helpers'; -import { BrowserView, BrowserWindow, webContents } from 'electron/main'; +import { BrowserView, BrowserWindow, screen, webContents } from 'electron/main'; import { closeWindow } from './window-helpers'; -import { defer, startRemoteControlApp } from './spec-helpers'; +import { defer, ifit, startRemoteControlApp } from './spec-helpers'; +import { areColorsSimilar, captureScreen, getPixelColor } from './screen-helpers'; describe('BrowserView module', () => { const fixtures = path.resolve(__dirname, '..', 'spec', 'fixtures'); @@ -60,6 +61,56 @@ describe('BrowserView module', () => { view.setBackgroundColor(null as any); }).to.throw(/conversion failure/); }); + + // Linux and arm64 platforms (WOA and macOS) do not return any capture sources + ifit(process.platform !== 'linux' && process.arch !== 'arm64')('sets the background color to transparent if none is set', async () => { + const display = screen.getPrimaryDisplay(); + const WINDOW_BACKGROUND_COLOR = '#55ccbb'; + + w.show(); + w.setBounds(display.bounds); + w.setBackgroundColor(WINDOW_BACKGROUND_COLOR); + await w.loadURL('about:blank'); + + view = new BrowserView(); + view.setBounds(display.bounds); + w.setBrowserView(view); + await view.webContents.loadURL('data:text/html,hello there'); + + const screenCapture = await captureScreen(); + const centerColor = getPixelColor(screenCapture, { + x: display.size.width / 2, + y: display.size.height / 2 + }); + + expect(areColorsSimilar(centerColor, WINDOW_BACKGROUND_COLOR)).to.be.true(); + }); + + // Linux and arm64 platforms (WOA and macOS) do not return any capture sources + ifit(process.platform !== 'linux' && process.arch !== 'arm64')('successfully applies the background color', async () => { + const WINDOW_BACKGROUND_COLOR = '#55ccbb'; + const VIEW_BACKGROUND_COLOR = '#ff00ff'; + const display = screen.getPrimaryDisplay(); + + w.show(); + w.setBounds(display.bounds); + w.setBackgroundColor(WINDOW_BACKGROUND_COLOR); + await w.loadURL('about:blank'); + + view = new BrowserView(); + view.setBounds(display.bounds); + w.setBrowserView(view); + w.setBackgroundColor(VIEW_BACKGROUND_COLOR); + await view.webContents.loadURL('data:text/html,hello there'); + + const screenCapture = await captureScreen(); + const centerColor = getPixelColor(screenCapture, { + x: display.size.width / 2, + y: display.size.height / 2 + }); + + expect(areColorsSimilar(centerColor, VIEW_BACKGROUND_COLOR)).to.be.true(); + }); }); describe('BrowserView.setAutoResize()', () => { From 8a03f0d1cf1f5c1c1b3e2096dcd11b85cd4d9fb7 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Thu, 30 Jun 2022 15:59:34 +0200 Subject: [PATCH 2/2] add missing screen-helpers.ts --- spec-main/screen-helpers.ts | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 spec-main/screen-helpers.ts diff --git a/spec-main/screen-helpers.ts b/spec-main/screen-helpers.ts new file mode 100644 index 0000000000000..26fbcb8ea2b08 --- /dev/null +++ b/spec-main/screen-helpers.ts @@ -0,0 +1,87 @@ +import * as path from 'path'; +import * as fs from 'fs'; +import { screen, desktopCapturer, NativeImage } from 'electron'; + +const fixtures = path.resolve(__dirname, '..', 'spec', 'fixtures'); + +/** Chroma key green. */ +export const CHROMA_COLOR_HEX = '#00b140'; + +/** + * Capture the screen at the given point. + * + * NOTE: Not yet supported on Linux in CI due to empty sources list. + */ +export const captureScreen = async (point: Electron.Point = { x: 0, y: 0 }): Promise => { + const display = screen.getDisplayNearestPoint(point); + const sources = await desktopCapturer.getSources({ types: ['screen'], thumbnailSize: display.size }); + // Toggle to save screen captures for debugging. + const DEBUG_CAPTURE = false; + if (DEBUG_CAPTURE) { + for (const source of sources) { + await fs.promises.writeFile(path.join(fixtures, `screenshot_${source.display_id}.png`), source.thumbnail.toPNG()); + } + } + const screenCapture = sources.find(source => source.display_id === `${display.id}`); + // Fails when HDR is enabled on Windows. + // https://bugs.chromium.org/p/chromium/issues/detail?id=1247730 + if (!screenCapture) { + const displayIds = sources.map(source => source.display_id); + throw new Error(`Unable to find screen capture for display '${display.id}'\n\tAvailable displays: ${displayIds.join(', ')}`); + } + return screenCapture.thumbnail; +}; + +const formatHexByte = (val: number): string => { + const str = val.toString(16); + return str.length === 2 ? str : `0${str}`; +}; + +/** + * Get the hex color at the given pixel coordinate in an image. + */ +export const getPixelColor = (image: Electron.NativeImage, point: Electron.Point): string => { + const pixel = image.crop({ ...point, width: 1, height: 1 }); + // TODO(samuelmaddock): NativeImage.toBitmap() should return the raw pixel + // color, but it sometimes differs. Why is that? + const [b, g, r] = pixel.toBitmap(); + return `#${formatHexByte(r)}${formatHexByte(g)}${formatHexByte(b)}`; +}; + +const hexToRgba = (hexColor: string) => { + const match = hexColor.match(/^#([0-9a-fA-F]{6,8})$/); + if (!match) return; + + const colorStr = match[1]; + return [ + parseInt(colorStr.substring(0, 2), 16), + parseInt(colorStr.substring(2, 4), 16), + parseInt(colorStr.substring(4, 6), 16), + parseInt(colorStr.substring(6, 8), 16) || 0xFF + ]; +}; + +/** Calculate euclidian distance between colors. */ +const colorDistance = (hexColorA: string, hexColorB: string) => { + const colorA = hexToRgba(hexColorA); + const colorB = hexToRgba(hexColorB); + if (!colorA || !colorB) return -1; + return Math.sqrt( + Math.pow(colorB[0] - colorA[0], 2) + + Math.pow(colorB[1] - colorA[1], 2) + + Math.pow(colorB[2] - colorA[2], 2) + ); +}; + +/** + * Determine if colors are similar based on distance. This can be useful when + * comparing colors which may differ based on lossy compression. + */ +export const areColorsSimilar = ( + hexColorA: string, + hexColorB: string, + distanceThreshold = 90 +): boolean => { + const distance = colorDistance(hexColorA, hexColorB); + return distance <= distanceThreshold; +};