Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 9bab573a37 from chromium #30100

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -116,3 +116,4 @@ support_runtime_configurable_key_storage_on_linux_os_crypto.patch
make_keychain_service_account_optionally_configurable_at_runtime.patch
don_t_run_pcscan_notifythreadcreated_if_pcscan_is_disabled.patch
cherry-pick-cc20b36a5845.patch
set_svgimage_page_after_document_install.patch
48 changes: 48 additions & 0 deletions patches/chromium/set_svgimage_page_after_document_install.patch
@@ -0,0 +1,48 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20S=C3=B6derqvist?= <fs@opera.com>
Date: Fri, 9 Jul 2021 08:44:55 +0000
Subject: Set SVGImage::page_ after document install
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We can end up having the associated ImageResource call
SVGImage::ResetAnimation() before the Document has been associated with
the SVGImage's LocalFrame, but after the link to the initial Document
was severed, if a GC is triggered within that window and ends up
collecting the last observer of the ImageResource.

By assigning |SVGImage::page_| after the installing the document, we
close this hole since SVGImage::RootElement() (called by
SVGImage::ResetAnimation()) will now observe a null Page and return null
without attempting to dereference the document.

Bug: 1216190
Change-Id: I26e08848e5b9bd52e3377841eee35e4acc03d320
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010140
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#899922}

diff --git a/third_party/blink/renderer/core/svg/graphics/svg_image.cc b/third_party/blink/renderer/core/svg/graphics/svg_image.cc
index 0742d1e2e78b6bb0cd2582afd2133bb5eaf56387..96768e7fed1ba36ef5bba48d540f76e8ce50a608 100644
--- a/third_party/blink/renderer/core/svg/graphics/svg_image.cc
+++ b/third_party/blink/renderer/core/svg/graphics/svg_image.cc
@@ -853,12 +853,15 @@ Image::SizeAvailability SVGImage::DataChanged(bool all_data_received) {
// SVG Images are transparent.
frame->View()->SetBaseBackgroundColor(Color::kTransparent);

- page_ = page;
-
TRACE_EVENT0("blink", "SVGImage::dataChanged::load");

frame->ForceSynchronousDocumentInstall("image/svg+xml", Data());

+ // Set up our Page reference after installing our document. This avoids
+ // tripping on a non-existing (null) Document if a GC is triggered during the
+ // set up and ends up collecting the last owner/observer of this image.
+ page_ = page;
+
// Intrinsic sizing relies on computed style (e.g. font-size and
// writing-mode).
frame->GetDocument()->UpdateStyleAndLayoutTree();