mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
chore: cherry-pick 4 changes from Release-0-M118 (#40210)
* chore: [25-x-y] cherry-pick 4 changes from Release-0-M118 * f218b4f37018 from chromium * d756d71a652c from chromium * chore: update patches --------- Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
@@ -143,3 +143,7 @@ cherry-pick-26175b0903d8.patch
|
||||
fix_use_delegated_generic_capturer_when_available.patch
|
||||
cherry-pick-b11e7d07a6f4.patch
|
||||
cherry-pick-309b604c4e88.patch
|
||||
cherry-pick-f218b4f37018.patch
|
||||
cherry-pick-d756d71a652c.patch
|
||||
parameterize_axtreeserializer_by_vector_type.patch
|
||||
avoid_allocating_recordid_objects_in_elementtiming_and_lcp.patch
|
||||
|
||||
@@ -0,0 +1,655 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Yoav Weiss <yoavweiss@chromium.org>
|
||||
Date: Thu, 31 Aug 2023 10:11:57 +0000
|
||||
Subject: Avoid allocating RecordId objects in ElementTiming and LCP
|
||||
|
||||
RecordId objects in current code keep around references to LayoutObject
|
||||
and ImageResourceContent, both GCed objects.
|
||||
Turn out that most of these references are not needed and are only used
|
||||
as hashmap keys that would be better served with an actual hash. The
|
||||
ones that are needed don't need extensive lifetimes and can be stack
|
||||
allocated.
|
||||
|
||||
Bug=1472365,1472366
|
||||
|
||||
Change-Id: I3fd77bed9899932d5bfadc2a8e6403a8e434235f
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4821128
|
||||
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
|
||||
Reviewed-by: Omer Katz <omerkatz@chromium.org>
|
||||
Cr-Commit-Position: refs/heads/main@{#1190644}
|
||||
|
||||
diff --git a/third_party/blink/renderer/core/paint/DEPS b/third_party/blink/renderer/core/paint/DEPS
|
||||
index 7b2b644ed89b254e97c59116d0a41f6c137f3b17..38d0a0b6b9c1c3598d2ae5a782d405bf90bf3733 100644
|
||||
--- a/third_party/blink/renderer/core/paint/DEPS
|
||||
+++ b/third_party/blink/renderer/core/paint/DEPS
|
||||
@@ -5,6 +5,8 @@ include_rules = [
|
||||
# For DCHECK.
|
||||
"+base/logging.h",
|
||||
"+components/shared_highlighting/core/common",
|
||||
+ # Hash function access
|
||||
+ "+base/hash/hash.h",
|
||||
]
|
||||
|
||||
specific_include_rules = {
|
||||
diff --git a/third_party/blink/renderer/core/paint/build.gni b/third_party/blink/renderer/core/paint/build.gni
|
||||
index 87df076817994e16e4a67331a9b3ce9dc1406c82..7dacd09f9a63eead72ad75d73039e76d1f7809c6 100644
|
||||
--- a/third_party/blink/renderer/core/paint/build.gni
|
||||
+++ b/third_party/blink/renderer/core/paint/build.gni
|
||||
@@ -196,6 +196,8 @@ blink_core_sources_paint = [
|
||||
"timing/image_paint_timing_detector.h",
|
||||
"timing/largest_contentful_paint_calculator.cc",
|
||||
"timing/largest_contentful_paint_calculator.h",
|
||||
+ "timing/media_record_id.cc",
|
||||
+ "timing/media_record_id.h",
|
||||
"timing/paint_timing.cc",
|
||||
"timing/paint_timing_detector.cc",
|
||||
"timing/paint_timing_detector.h",
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/image_element_timing.cc b/third_party/blink/renderer/core/paint/timing/image_element_timing.cc
|
||||
index a8501cfcc91e57b3348e3db8ff11256c4d7176b2..21f7a0ac38732e5381da6b82544044b8e6da3a11 100644
|
||||
--- a/third_party/blink/renderer/core/paint/timing/image_element_timing.cc
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/image_element_timing.cc
|
||||
@@ -68,7 +68,7 @@ void ImageElementTiming::NotifyImageFinished(
|
||||
return;
|
||||
|
||||
const auto& insertion_result = images_notified_.insert(
|
||||
- std::make_pair(&layout_object, cached_image), ImageInfo());
|
||||
+ MediaRecordId::GenerateHash(&layout_object, cached_image), ImageInfo());
|
||||
if (insertion_result.is_new_entry)
|
||||
insertion_result.stored_value->value.load_time_ = base::TimeTicks::Now();
|
||||
}
|
||||
@@ -97,8 +97,8 @@ void ImageElementTiming::NotifyImagePainted(
|
||||
if (!internal::IsExplicitlyRegisteredForTiming(layout_object))
|
||||
return;
|
||||
|
||||
- auto it =
|
||||
- images_notified_.find(std::make_pair(&layout_object, &cached_image));
|
||||
+ auto it = images_notified_.find(
|
||||
+ MediaRecordId::GenerateHash(&layout_object, &cached_image));
|
||||
// It is possible that the pair is not in |images_notified_|. See
|
||||
// https://crbug.com/1027948
|
||||
if (it != images_notified_.end() && !it->value.is_painted_) {
|
||||
@@ -218,7 +218,8 @@ void ImageElementTiming::NotifyBackgroundImagePainted(
|
||||
|
||||
ImageInfo& info =
|
||||
images_notified_
|
||||
- .insert(std::make_pair(layout_object, cached_image), ImageInfo())
|
||||
+ .insert(MediaRecordId::GenerateHash(layout_object, cached_image),
|
||||
+ ImageInfo())
|
||||
.stored_value->value;
|
||||
if (!info.is_painted_) {
|
||||
info.is_painted_ = true;
|
||||
@@ -246,7 +247,7 @@ void ImageElementTiming::ReportImagePaintPresentationTime(
|
||||
|
||||
void ImageElementTiming::NotifyImageRemoved(const LayoutObject* layout_object,
|
||||
const ImageResourceContent* image) {
|
||||
- images_notified_.erase(std::make_pair(layout_object, image));
|
||||
+ images_notified_.erase(MediaRecordId::GenerateHash(layout_object, image));
|
||||
}
|
||||
|
||||
void ImageElementTiming::Trace(Visitor* visitor) const {
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/image_element_timing.h b/third_party/blink/renderer/core/paint/timing/image_element_timing.h
|
||||
index 0a152210de91ab69c255be8caa951616afb3df06..7d3f5dd7c9137b277543e52d4d1e327d75b01e84 100644
|
||||
--- a/third_party/blink/renderer/core/paint/timing/image_element_timing.h
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/image_element_timing.h
|
||||
@@ -5,12 +5,11 @@
|
||||
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_TIMING_IMAGE_ELEMENT_TIMING_H_
|
||||
#define THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_TIMING_IMAGE_ELEMENT_TIMING_H_
|
||||
|
||||
-#include <utility>
|
||||
-
|
||||
#include "base/time/time.h"
|
||||
#include "third_party/blink/renderer/core/core_export.h"
|
||||
#include "third_party/blink/renderer/core/dom/element.h"
|
||||
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
|
||||
+#include "third_party/blink/renderer/core/paint/timing/media_record_id.h"
|
||||
#include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_map.h"
|
||||
#include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h"
|
||||
#include "third_party/blink/renderer/platform/supplementable.h"
|
||||
@@ -123,13 +122,12 @@ class CORE_EXPORT ImageElementTiming final
|
||||
|
||||
DISALLOW_NEW();
|
||||
};
|
||||
- typedef std::pair<const LayoutObject*, const ImageResourceContent*> RecordId;
|
||||
// Hashmap of pairs of elements, LayoutObjects (for the elements) and
|
||||
// ImageResourceContent (for the src) which correspond to either images or
|
||||
// background images whose paint has been observed. For background images,
|
||||
// only the |is_painted_| bit is used, as the timestamp needs to be tracked by
|
||||
// |background_image_timestamps_|.
|
||||
- WTF::HashMap<RecordId, ImageInfo> images_notified_;
|
||||
+ WTF::HashMap<MediaRecordIdHash, ImageInfo> images_notified_;
|
||||
|
||||
// Hashmap of background images which contain information about the load time
|
||||
// of the background image.
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/image_element_timing_test.cc b/third_party/blink/renderer/core/paint/timing/image_element_timing_test.cc
|
||||
index f1aee5cdd179398a2552e8af2de273479522a623..3643ad57403d46d5bc341478ebb51db6ccc9f175 100644
|
||||
--- a/third_party/blink/renderer/core/paint/timing/image_element_timing_test.cc
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/image_element_timing_test.cc
|
||||
@@ -9,6 +9,7 @@
|
||||
#include "third_party/blink/renderer/core/layout/layout_image.h"
|
||||
#include "third_party/blink/renderer/core/layout/svg/layout_svg_image.h"
|
||||
#include "third_party/blink/renderer/core/loader/resource/image_resource_content.h"
|
||||
+#include "third_party/blink/renderer/core/paint/timing/media_record_id.h"
|
||||
#include "third_party/blink/renderer/platform/graphics/unaccelerated_static_bitmap_image.h"
|
||||
#include "third_party/blink/renderer/platform/testing/paint_test_configurations.h"
|
||||
#include "third_party/blink/renderer/platform/testing/url_test_helpers.h"
|
||||
@@ -60,11 +61,9 @@ class ImageElementTimingTest : public testing::Test,
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
- bool ImagesNotifiedContains(
|
||||
- const std::pair<const LayoutObject*, const ImageResourceContent*>&
|
||||
- record_id) {
|
||||
+ bool ImagesNotifiedContains(MediaRecordIdHash record_id_hash) {
|
||||
return ImageElementTiming::From(*GetDoc()->domWindow())
|
||||
- .images_notified_.Contains(record_id);
|
||||
+ .images_notified_.Contains(record_id_hash);
|
||||
}
|
||||
|
||||
unsigned ImagesNotifiedSize() {
|
||||
@@ -159,7 +158,7 @@ TEST_P(ImageElementTimingTest, IgnoresUnmarkedElement) {
|
||||
ASSERT_TRUE(layout_image);
|
||||
UpdateAllLifecyclePhases();
|
||||
EXPECT_FALSE(ImagesNotifiedContains(
|
||||
- std::make_pair(layout_image, layout_image->CachedImage())));
|
||||
+ MediaRecordId::GenerateHash(layout_image, layout_image->CachedImage())));
|
||||
}
|
||||
|
||||
TEST_P(ImageElementTimingTest, ImageInsideSVG) {
|
||||
@@ -179,7 +178,7 @@ TEST_P(ImageElementTimingTest, ImageInsideSVG) {
|
||||
|
||||
// |layout_image| should have had its paint notified to ImageElementTiming.
|
||||
EXPECT_TRUE(ImagesNotifiedContains(
|
||||
- std::make_pair(layout_image, layout_image->CachedImage())));
|
||||
+ MediaRecordId::GenerateHash(layout_image, layout_image->CachedImage())));
|
||||
}
|
||||
|
||||
TEST_P(ImageElementTimingTest, ImageInsideNonRenderedSVG) {
|
||||
@@ -214,7 +213,7 @@ TEST_P(ImageElementTimingTest, ImageRemoved) {
|
||||
ASSERT_TRUE(layout_image);
|
||||
UpdateAllLifecyclePhases();
|
||||
EXPECT_TRUE(ImagesNotifiedContains(
|
||||
- std::make_pair(layout_image, layout_image->CachedImage())));
|
||||
+ MediaRecordId::GenerateHash(layout_image, layout_image->CachedImage())));
|
||||
|
||||
GetDoc()->getElementById("target")->remove();
|
||||
// |layout_image| should no longer be part of |images_notified| since it will
|
||||
@@ -234,7 +233,7 @@ TEST_P(ImageElementTimingTest, SVGImageRemoved) {
|
||||
LayoutSVGImage* layout_image = SetSVGImageResource("target", 5, 5);
|
||||
ASSERT_TRUE(layout_image);
|
||||
UpdateAllLifecyclePhases();
|
||||
- EXPECT_TRUE(ImagesNotifiedContains(std::make_pair(
|
||||
+ EXPECT_TRUE(ImagesNotifiedContains(MediaRecordId::GenerateHash(
|
||||
layout_image, layout_image->ImageResource()->CachedImage())));
|
||||
|
||||
GetDoc()->getElementById("target")->remove();
|
||||
@@ -261,7 +260,8 @@ TEST_P(ImageElementTimingTest, BackgroundImageRemoved) {
|
||||
object->Style()->BackgroundLayers().GetImage()->CachedImage();
|
||||
UpdateAllLifecyclePhases();
|
||||
EXPECT_EQ(ImagesNotifiedSize(), 1u);
|
||||
- EXPECT_TRUE(ImagesNotifiedContains(std::make_pair(object, content)));
|
||||
+ EXPECT_TRUE(
|
||||
+ ImagesNotifiedContains(MediaRecordId::GenerateHash(object, content)));
|
||||
|
||||
GetDoc()->getElementById("target")->remove();
|
||||
EXPECT_EQ(ImagesNotifiedSize(), 0u);
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.cc b/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.cc
|
||||
index b4ed09ccd893b39d4497fc9a237c1ab00bda91fa..e516c2a15a362fa71fcb1d644e8eb61ea2934b1e 100644
|
||||
--- a/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.cc
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.cc
|
||||
@@ -231,8 +231,8 @@ void ImagePaintTimingDetector::OnPaintFinished() {
|
||||
void ImagePaintTimingDetector::NotifyImageRemoved(
|
||||
const LayoutObject& object,
|
||||
const MediaTiming* media_timing) {
|
||||
- RecordId record_id = std::make_pair(&object, media_timing);
|
||||
- records_manager_.RemoveRecord(record_id);
|
||||
+ records_manager_.RemoveRecord(
|
||||
+ MediaRecordId::GenerateHash(&object, media_timing));
|
||||
}
|
||||
|
||||
void ImagePaintTimingDetector::StopRecordEntries() {
|
||||
@@ -270,7 +270,7 @@ void ImageRecordsManager::AssignPaintTimeToRegisteredQueuedRecords(
|
||||
unsigned last_queued_frame_index) {
|
||||
while (!images_queued_for_paint_time_.empty()) {
|
||||
const base::WeakPtr<ImageRecord>& record =
|
||||
- images_queued_for_paint_time_.front().first;
|
||||
+ images_queued_for_paint_time_.front().image_record;
|
||||
if (!record) {
|
||||
images_queued_for_paint_time_.pop_front();
|
||||
continue;
|
||||
@@ -282,8 +282,8 @@ void ImageRecordsManager::AssignPaintTimeToRegisteredQueuedRecords(
|
||||
record->first_animated_frame_time = timestamp;
|
||||
record->queue_animated_paint = false;
|
||||
}
|
||||
- auto it =
|
||||
- pending_images_.find(images_queued_for_paint_time_.front().second);
|
||||
+ auto it = pending_images_.find(
|
||||
+ images_queued_for_paint_time_.front().record_id_hash);
|
||||
images_queued_for_paint_time_.pop_front();
|
||||
// A record may be in |images_queued_for_paint_time_| twice, for instance if
|
||||
// is already loaded by the time of its first paint.
|
||||
@@ -319,7 +319,8 @@ bool ImagePaintTimingDetector::RecordImage(
|
||||
if (image_border.IsEmpty())
|
||||
return false;
|
||||
|
||||
- RecordId record_id = std::make_pair(&object, &media_timing);
|
||||
+ MediaRecordId record_id(&object, &media_timing);
|
||||
+ MediaRecordIdHash record_id_hash = record_id.GetHash();
|
||||
|
||||
if (int depth = IgnorePaintTimingScope::IgnoreDepth()) {
|
||||
// Record the largest loaded image that is hidden due to documentElement
|
||||
@@ -340,17 +341,18 @@ bool ImagePaintTimingDetector::RecordImage(
|
||||
return false;
|
||||
}
|
||||
|
||||
- if (records_manager_.IsRecordedImage(record_id)) {
|
||||
+ if (records_manager_.IsRecordedImage(record_id_hash)) {
|
||||
base::WeakPtr<ImageRecord> record =
|
||||
- records_manager_.GetPendingImage(record_id);
|
||||
+ records_manager_.GetPendingImage(record_id_hash);
|
||||
if (!record)
|
||||
return false;
|
||||
if (ShouldReportAnimatedImages() && media_timing.IsPaintedFirstFrame()) {
|
||||
added_entry_in_latest_frame_ |=
|
||||
- records_manager_.OnFirstAnimatedFramePainted(record_id, frame_index_);
|
||||
+ records_manager_.OnFirstAnimatedFramePainted(record_id_hash,
|
||||
+ frame_index_);
|
||||
}
|
||||
if (!record->loaded && media_timing.IsSufficientContentLoadedForPaint()) {
|
||||
- records_manager_.OnImageLoaded(record_id, frame_index_, style_image);
|
||||
+ records_manager_.OnImageLoaded(record_id_hash, frame_index_, style_image);
|
||||
added_entry_in_latest_frame_ = true;
|
||||
if (absl::optional<PaintTimingVisualizer>& visualizer =
|
||||
frame_view_->GetPaintTimingDetector().Visualizer()) {
|
||||
@@ -386,10 +388,11 @@ bool ImagePaintTimingDetector::RecordImage(
|
||||
|
||||
if (ShouldReportAnimatedImages() && media_timing.IsPaintedFirstFrame()) {
|
||||
added_entry_in_latest_frame_ |=
|
||||
- records_manager_.OnFirstAnimatedFramePainted(record_id, frame_index_);
|
||||
+ records_manager_.OnFirstAnimatedFramePainted(record_id_hash,
|
||||
+ frame_index_);
|
||||
}
|
||||
if (media_timing.IsSufficientContentLoadedForPaint()) {
|
||||
- records_manager_.OnImageLoaded(record_id, frame_index_, style_image);
|
||||
+ records_manager_.OnImageLoaded(record_id_hash, frame_index_, style_image);
|
||||
added_entry_in_latest_frame_ = true;
|
||||
return true;
|
||||
}
|
||||
@@ -445,8 +448,8 @@ uint64_t ImagePaintTimingDetector::ComputeImageRectSize(
|
||||
void ImagePaintTimingDetector::NotifyImageFinished(
|
||||
const LayoutObject& object,
|
||||
const MediaTiming* media_timing) {
|
||||
- RecordId record_id = std::make_pair(&object, media_timing);
|
||||
- records_manager_.NotifyImageFinished(record_id);
|
||||
+ records_manager_.NotifyImageFinished(
|
||||
+ MediaRecordId::GenerateHash(&object, media_timing));
|
||||
}
|
||||
|
||||
void ImagePaintTimingDetector::ReportLargestIgnoredImage() {
|
||||
@@ -458,9 +461,9 @@ ImageRecordsManager::ImageRecordsManager(LocalFrameView* frame_view)
|
||||
: size_ordered_set_(&LargeImageFirst), frame_view_(frame_view) {}
|
||||
|
||||
bool ImageRecordsManager::OnFirstAnimatedFramePainted(
|
||||
- const RecordId& record_id,
|
||||
+ MediaRecordIdHash record_id_hash,
|
||||
unsigned current_frame_index) {
|
||||
- base::WeakPtr<ImageRecord> record = GetPendingImage(record_id);
|
||||
+ base::WeakPtr<ImageRecord> record = GetPendingImage(record_id_hash);
|
||||
DCHECK(record);
|
||||
if (record->media_timing &&
|
||||
!record->media_timing->GetFirstVideoFrameTime().is_null()) {
|
||||
@@ -473,19 +476,19 @@ bool ImageRecordsManager::OnFirstAnimatedFramePainted(
|
||||
// Otherwise, this is an animated images, and so we should wait for the
|
||||
// presentation callback to fire to set the first frame presentation time.
|
||||
record->queue_animated_paint = true;
|
||||
- QueueToMeasurePaintTime(record_id, record, current_frame_index);
|
||||
+ QueueToMeasurePaintTime(record_id_hash, record, current_frame_index);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
-void ImageRecordsManager::OnImageLoaded(const RecordId& record_id,
|
||||
+void ImageRecordsManager::OnImageLoaded(MediaRecordIdHash record_id_hash,
|
||||
unsigned current_frame_index,
|
||||
const StyleFetchedImage* style_image) {
|
||||
- base::WeakPtr<ImageRecord> record = GetPendingImage(record_id);
|
||||
+ base::WeakPtr<ImageRecord> record = GetPendingImage(record_id_hash);
|
||||
DCHECK(record);
|
||||
if (!style_image) {
|
||||
- auto it = image_finished_times_.find(record_id);
|
||||
+ auto it = image_finished_times_.find(record_id_hash);
|
||||
if (it != image_finished_times_.end()) {
|
||||
record->load_time = it->value;
|
||||
DCHECK(!record->load_time.is_null());
|
||||
@@ -498,7 +501,7 @@ void ImageRecordsManager::OnImageLoaded(const RecordId& record_id,
|
||||
record->origin_clean = style_image->IsOriginClean();
|
||||
}
|
||||
}
|
||||
- OnImageLoadedInternal(record_id, record, current_frame_index);
|
||||
+ OnImageLoadedInternal(record_id_hash, record, current_frame_index);
|
||||
}
|
||||
|
||||
void ImageRecordsManager::ReportLargestIgnoredImage(
|
||||
@@ -518,25 +521,25 @@ void ImageRecordsManager::ReportLargestIgnoredImage(
|
||||
DCHECK(document);
|
||||
PaintTiming::From(*document).MarkFirstContentfulPaint();
|
||||
|
||||
- RecordId record_id = std::make_pair(node->GetLayoutObject(),
|
||||
- largest_ignored_image_->media_timing);
|
||||
- recorded_images_.insert(record_id);
|
||||
+ MediaRecordIdHash record_id_hash = MediaRecordId::GenerateHash(
|
||||
+ node->GetLayoutObject(), largest_ignored_image_->media_timing);
|
||||
+ recorded_images_.insert(record_id_hash);
|
||||
base::WeakPtr<ImageRecord> record = largest_ignored_image_->AsWeakPtr();
|
||||
size_ordered_set_.insert(record);
|
||||
- pending_images_.insert(record_id, std::move(largest_ignored_image_));
|
||||
- OnImageLoadedInternal(record_id, record, current_frame_index);
|
||||
+ pending_images_.insert(record_id_hash, std::move(largest_ignored_image_));
|
||||
+ OnImageLoadedInternal(record_id_hash, record, current_frame_index);
|
||||
}
|
||||
|
||||
void ImageRecordsManager::OnImageLoadedInternal(
|
||||
- const RecordId& record_id,
|
||||
+ MediaRecordIdHash record_id_hash,
|
||||
base::WeakPtr<ImageRecord>& record,
|
||||
unsigned current_frame_index) {
|
||||
SetLoaded(record);
|
||||
- QueueToMeasurePaintTime(record_id, record, current_frame_index);
|
||||
+ QueueToMeasurePaintTime(record_id_hash, record, current_frame_index);
|
||||
}
|
||||
|
||||
void ImageRecordsManager::MaybeUpdateLargestIgnoredImage(
|
||||
- const RecordId& record_id,
|
||||
+ const MediaRecordId& record_id,
|
||||
const uint64_t& visual_size,
|
||||
const gfx::Rect& frame_visual_rect,
|
||||
const gfx::RectF& root_visual_rect,
|
||||
@@ -544,14 +547,14 @@ void ImageRecordsManager::MaybeUpdateLargestIgnoredImage(
|
||||
if (visual_size && (!largest_ignored_image_ ||
|
||||
visual_size > largest_ignored_image_->recorded_size)) {
|
||||
largest_ignored_image_ = CreateImageRecord(
|
||||
- *record_id.first, record_id.second, visual_size, frame_visual_rect,
|
||||
- root_visual_rect, is_loaded_after_mouseover);
|
||||
+ *record_id.GetLayoutObject(), record_id.GetMediaTiming(), visual_size,
|
||||
+ frame_visual_rect, root_visual_rect, is_loaded_after_mouseover);
|
||||
largest_ignored_image_->load_time = base::TimeTicks::Now();
|
||||
}
|
||||
}
|
||||
|
||||
bool ImageRecordsManager::RecordFirstPaintAndReturnIsPending(
|
||||
- const RecordId& record_id,
|
||||
+ const MediaRecordId& record_id,
|
||||
const uint64_t& visual_size,
|
||||
const gfx::Rect& frame_visual_rect,
|
||||
const gfx::RectF& root_visual_rect,
|
||||
@@ -562,7 +565,7 @@ bool ImageRecordsManager::RecordFirstPaintAndReturnIsPending(
|
||||
if (visual_size == 0u) {
|
||||
return false;
|
||||
}
|
||||
- recorded_images_.insert(record_id);
|
||||
+ recorded_images_.insert(record_id.GetHash());
|
||||
// If this cannot become an LCP candidate, no need to do anything else.
|
||||
if (visual_size == 0u ||
|
||||
(largest_painted_image_ &&
|
||||
@@ -588,10 +591,10 @@ bool ImageRecordsManager::RecordFirstPaintAndReturnIsPending(
|
||||
}
|
||||
|
||||
std::unique_ptr<ImageRecord> record = CreateImageRecord(
|
||||
- *record_id.first, record_id.second, visual_size, frame_visual_rect,
|
||||
- root_visual_rect, is_loaded_after_mouseover);
|
||||
+ *record_id.GetLayoutObject(), record_id.GetMediaTiming(), visual_size,
|
||||
+ frame_visual_rect, root_visual_rect, is_loaded_after_mouseover);
|
||||
size_ordered_set_.insert(record->AsWeakPtr());
|
||||
- pending_images_.insert(record_id, std::move(record));
|
||||
+ pending_images_.insert(record_id.GetHash(), std::move(record));
|
||||
return true;
|
||||
}
|
||||
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.h b/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.h
|
||||
index 413176e992e110db2818d7ed2b06ebae232fea1f..5a5ee2a9033de3aa74c5296ce6b1e962524a8fd8 100644
|
||||
--- a/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.h
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/image_paint_timing_detector.h
|
||||
@@ -15,6 +15,7 @@
|
||||
#include "third_party/blink/renderer/core/core_export.h"
|
||||
#include "third_party/blink/renderer/core/dom/dom_node_ids.h"
|
||||
#include "third_party/blink/renderer/core/loader/resource/image_resource_content.h"
|
||||
+#include "third_party/blink/renderer/core/paint/timing/media_record_id.h"
|
||||
#include "third_party/blink/renderer/core/paint/timing/paint_timing_detector.h"
|
||||
#include "third_party/blink/renderer/platform/allow_discouraged_type.h"
|
||||
#include "third_party/blink/renderer/platform/loader/fetch/media_timing.h"
|
||||
@@ -90,8 +91,6 @@ class ImageRecord : public base::SupportsWeakPtr<ImageRecord> {
|
||||
bool is_loaded_after_mouseover = false;
|
||||
};
|
||||
|
||||
-typedef std::pair<const LayoutObject*, const MediaTiming*> RecordId;
|
||||
-
|
||||
// |ImageRecordsManager| is the manager of all of the images that Largest
|
||||
// Image Paint cares about. Note that an image does not necessarily correspond
|
||||
// to a node; it can also be one of the background images attached to a node.
|
||||
@@ -115,10 +114,10 @@ class CORE_EXPORT ImageRecordsManager {
|
||||
ImageRecordsManager& operator=(const ImageRecordsManager&) = delete;
|
||||
ImageRecord* LargestImage() const;
|
||||
|
||||
- inline void RemoveRecord(const RecordId& record_id) {
|
||||
- recorded_images_.erase(record_id);
|
||||
- image_finished_times_.erase(record_id);
|
||||
- auto it = pending_images_.find(record_id);
|
||||
+ inline void RemoveRecord(MediaRecordIdHash record_id_hash) {
|
||||
+ recorded_images_.erase(record_id_hash);
|
||||
+ image_finished_times_.erase(record_id_hash);
|
||||
+ auto it = pending_images_.find(record_id_hash);
|
||||
if (it != pending_images_.end()) {
|
||||
size_ordered_set_.erase(it->value->AsWeakPtr());
|
||||
pending_images_.erase(it);
|
||||
@@ -128,41 +127,42 @@ class CORE_EXPORT ImageRecordsManager {
|
||||
}
|
||||
}
|
||||
// Returns whether an image was added to |pending_images_|.
|
||||
- bool RecordFirstPaintAndReturnIsPending(const RecordId& record_id,
|
||||
+ bool RecordFirstPaintAndReturnIsPending(const MediaRecordId& record_id,
|
||||
const uint64_t& visual_size,
|
||||
const gfx::Rect& frame_visual_rect,
|
||||
const gfx::RectF& root_visual_rect,
|
||||
double bpp,
|
||||
bool is_loaded_after_mouseover);
|
||||
- bool IsRecordedImage(const RecordId& record_id) const {
|
||||
- return recorded_images_.Contains(record_id);
|
||||
+ bool IsRecordedImage(MediaRecordIdHash record_id_hash) const {
|
||||
+ return recorded_images_.Contains(record_id_hash);
|
||||
}
|
||||
|
||||
- void NotifyImageFinished(const RecordId& record_id) {
|
||||
+ void NotifyImageFinished(MediaRecordIdHash record_id_hash) {
|
||||
// TODO(npm): Ideally NotifyImageFinished() would only be called when the
|
||||
// record has not yet been inserted in |image_finished_times_| but that's
|
||||
// not currently the case. If we plumb some information from
|
||||
// MediaTiming we may be able to ensure that this call does not
|
||||
// require the Contains() check, which would save time.
|
||||
- if (!image_finished_times_.Contains(record_id)) {
|
||||
- image_finished_times_.insert(record_id, base::TimeTicks::Now());
|
||||
+ if (!image_finished_times_.Contains(record_id_hash)) {
|
||||
+ image_finished_times_.insert(record_id_hash, base::TimeTicks::Now());
|
||||
}
|
||||
}
|
||||
|
||||
- inline base::WeakPtr<ImageRecord> GetPendingImage(const RecordId& record_id) {
|
||||
- auto it = pending_images_.find(record_id);
|
||||
+ inline base::WeakPtr<ImageRecord> GetPendingImage(
|
||||
+ MediaRecordIdHash record_id_hash) {
|
||||
+ auto it = pending_images_.find(record_id_hash);
|
||||
return it == pending_images_.end() ? nullptr : it->value->AsWeakPtr();
|
||||
}
|
||||
- bool OnFirstAnimatedFramePainted(const RecordId&,
|
||||
+ bool OnFirstAnimatedFramePainted(MediaRecordIdHash,
|
||||
unsigned current_frame_index);
|
||||
- void OnImageLoaded(const RecordId&,
|
||||
+ void OnImageLoaded(MediaRecordIdHash,
|
||||
unsigned current_frame_index,
|
||||
const StyleFetchedImage*);
|
||||
|
||||
// Receives a candidate image painted under opacity 0 but without nested
|
||||
// opacity. May update |largest_ignored_image_| if the new candidate has a
|
||||
// larger size.
|
||||
- void MaybeUpdateLargestIgnoredImage(const RecordId&,
|
||||
+ void MaybeUpdateLargestIgnoredImage(const MediaRecordId&,
|
||||
const uint64_t& visual_size,
|
||||
const gfx::Rect& frame_visual_rect,
|
||||
const gfx::RectF& root_visual_rect,
|
||||
@@ -186,19 +186,31 @@ class CORE_EXPORT ImageRecordsManager {
|
||||
const gfx::Rect& frame_visual_rect,
|
||||
const gfx::RectF& root_visual_rect,
|
||||
bool is_loaded_after_mouseover);
|
||||
- inline void QueueToMeasurePaintTime(const RecordId& record_id,
|
||||
+ inline void QueueToMeasurePaintTime(MediaRecordIdHash record_id_hash,
|
||||
base::WeakPtr<ImageRecord>& record,
|
||||
unsigned current_frame_index) {
|
||||
record->frame_index = current_frame_index;
|
||||
- images_queued_for_paint_time_.push_back(std::make_pair(record, record_id));
|
||||
+ images_queued_for_paint_time_.push_back(
|
||||
+ ImageRecordAndHashPair(record, record_id_hash));
|
||||
}
|
||||
inline void SetLoaded(base::WeakPtr<ImageRecord>& record) {
|
||||
record->loaded = true;
|
||||
}
|
||||
- void OnImageLoadedInternal(const RecordId&,
|
||||
+ void OnImageLoadedInternal(MediaRecordIdHash,
|
||||
base::WeakPtr<ImageRecord>&,
|
||||
unsigned current_frame_index);
|
||||
|
||||
+ struct ImageRecordAndHashPair {
|
||||
+ ImageRecordAndHashPair(base::WeakPtr<ImageRecord>& record,
|
||||
+ MediaRecordIdHash id_hash) {
|
||||
+ image_record = record;
|
||||
+ record_id_hash = id_hash;
|
||||
+ }
|
||||
+
|
||||
+ base::WeakPtr<ImageRecord> image_record;
|
||||
+ MediaRecordIdHash record_id_hash;
|
||||
+ };
|
||||
+
|
||||
// The ImageRecord corresponding to the largest image that has been loaded and
|
||||
// painted.
|
||||
std::unique_ptr<ImageRecord> largest_painted_image_;
|
||||
@@ -207,24 +219,23 @@ class CORE_EXPORT ImageRecordsManager {
|
||||
// timestamp, ordered by size.
|
||||
ImageRecordSet size_ordered_set_;
|
||||
|
||||
- // RecordId for images for which we have seen a first paint. A RecordId is
|
||||
- // added to this set regardless of whether the image could be an LCP
|
||||
- // candidate.
|
||||
- HashSet<RecordId> recorded_images_;
|
||||
+ // MediaRecordId for images for which we have seen a first paint. A
|
||||
+ // MediaRecordId is added to this set regardless of whether the image could be
|
||||
+ // an LCP candidate.
|
||||
+ HashSet<MediaRecordIdHash> recorded_images_;
|
||||
|
||||
- // Map of RecordId to ImageRecord for images for which the first paint has
|
||||
- // been seen but which do not have the paint time set yet. This may contain
|
||||
- // only images which are potential LCP candidates.
|
||||
- HashMap<RecordId, std::unique_ptr<ImageRecord>> pending_images_;
|
||||
+ // Map of MediaRecordId to ImageRecord for images for which the first paint
|
||||
+ // has been seen but which do not have the paint time set yet. This may
|
||||
+ // contain only images which are potential LCP candidates.
|
||||
+ HashMap<MediaRecordIdHash, std::unique_ptr<ImageRecord>> pending_images_;
|
||||
|
||||
// |ImageRecord|s waiting for paint time are stored in this map
|
||||
// until they get a presentation time.
|
||||
- Deque<std::pair<base::WeakPtr<ImageRecord>, RecordId>>
|
||||
- images_queued_for_paint_time_;
|
||||
+ Deque<ImageRecordAndHashPair> images_queued_for_paint_time_;
|
||||
|
||||
// Map containing timestamps of when LayoutObject::ImageNotifyFinished is
|
||||
// first called.
|
||||
- HashMap<RecordId, base::TimeTicks> image_finished_times_;
|
||||
+ HashMap<MediaRecordIdHash, base::TimeTicks> image_finished_times_;
|
||||
|
||||
Member<LocalFrameView> frame_view_;
|
||||
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/media_record_id.cc b/third_party/blink/renderer/core/paint/timing/media_record_id.cc
|
||||
new file mode 100644
|
||||
index 0000000000000000000000000000000000000000..e49c898d1111015d80a71ddc11bd791bb2a0dca1
|
||||
--- /dev/null
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/media_record_id.cc
|
||||
@@ -0,0 +1,27 @@
|
||||
+// Copyright 2023 The Chromium Authors
|
||||
+// Use of this source code is governed by a BSD-style license that can be
|
||||
+// found in the LICENSE file.
|
||||
+
|
||||
+#include "third_party/blink/renderer/core/paint/timing/media_record_id.h"
|
||||
+
|
||||
+#include "base/hash/hash.h"
|
||||
+
|
||||
+namespace blink {
|
||||
+
|
||||
+MediaRecordId::MediaRecordId(const LayoutObject* layout,
|
||||
+ const MediaTiming* media)
|
||||
+ : layout_object_(layout),
|
||||
+ media_timing_(media),
|
||||
+ hash_(GenerateHash(layout, media)) {}
|
||||
+
|
||||
+// This hash is used as a key where previously MediaRecordId was used directly.
|
||||
+// That helps us avoid storing references to the GCed LayoutObject and
|
||||
+// MediaTiming, as that can be unsafe when using regular WTF containers. It also
|
||||
+// helps us avoid needlessly allocating MediaRecordId on the heap.
|
||||
+MediaRecordIdHash MediaRecordId::GenerateHash(const LayoutObject* layout,
|
||||
+ const MediaTiming* media) {
|
||||
+ return base::HashInts(reinterpret_cast<MediaRecordIdHash>(layout),
|
||||
+ reinterpret_cast<MediaRecordIdHash>(media));
|
||||
+}
|
||||
+
|
||||
+} // namespace blink
|
||||
diff --git a/third_party/blink/renderer/core/paint/timing/media_record_id.h b/third_party/blink/renderer/core/paint/timing/media_record_id.h
|
||||
new file mode 100644
|
||||
index 0000000000000000000000000000000000000000..32952101e8463e31617d1b5f67c36abf04303c4a
|
||||
--- /dev/null
|
||||
+++ b/third_party/blink/renderer/core/paint/timing/media_record_id.h
|
||||
@@ -0,0 +1,37 @@
|
||||
+// Copyright 2023 The Chromium Authors
|
||||
+// Use of this source code is governed by a BSD-style license that can be
|
||||
+// found in the LICENSE file.
|
||||
+
|
||||
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_TIMING_MEDIA_RECORD_ID_H_
|
||||
+#define THIRD_PARTY_BLINK_RENDERER_CORE_PAINT_TIMING_MEDIA_RECORD_ID_H_
|
||||
+
|
||||
+#include "third_party/blink/renderer/core/core_export.h"
|
||||
+#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
|
||||
+
|
||||
+namespace blink {
|
||||
+class LayoutObject;
|
||||
+class MediaTiming;
|
||||
+
|
||||
+using MediaRecordIdHash = size_t;
|
||||
+
|
||||
+class MediaRecordId {
|
||||
+ STACK_ALLOCATED();
|
||||
+
|
||||
+ public:
|
||||
+ static MediaRecordIdHash CORE_EXPORT GenerateHash(const LayoutObject* layout,
|
||||
+ const MediaTiming* media);
|
||||
+
|
||||
+ MediaRecordId(const LayoutObject* layout, const MediaTiming* media);
|
||||
+
|
||||
+ MediaRecordIdHash GetHash() const { return hash_; }
|
||||
+ const LayoutObject* GetLayoutObject() const { return layout_object_; }
|
||||
+ const MediaTiming* GetMediaTiming() const { return media_timing_; }
|
||||
+
|
||||
+ private:
|
||||
+ const LayoutObject* const layout_object_;
|
||||
+ const MediaTiming* const media_timing_;
|
||||
+ const MediaRecordIdHash hash_;
|
||||
+};
|
||||
+
|
||||
+} // namespace blink
|
||||
+#endif
|
||||
diff --git a/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py b/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
|
||||
index 569a2e9dcc37aaad1368d2daa742775ce12ecdd9..7197616a7be18333d51c61a256f1d712a52b8256 100755
|
||||
--- a/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
|
||||
+++ b/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
|
||||
@@ -68,6 +68,7 @@ _CONFIG = [
|
||||
'base::DefaultTickClock',
|
||||
'base::ElapsedTimer',
|
||||
'base::EnumSet',
|
||||
+ 'base::HashInts',
|
||||
'base::JobDelegate',
|
||||
'base::JobHandle',
|
||||
'base::PostJob',
|
||||
201
patches/chromium/cherry-pick-d756d71a652c.patch
Normal file
201
patches/chromium/cherry-pick-d756d71a652c.patch
Normal file
@@ -0,0 +1,201 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Alex Moshchuk <alexmos@chromium.org>
|
||||
Date: Mon, 9 Oct 2023 17:14:13 +0000
|
||||
Subject: Fix RFHI::pending_navigate_ cleanup after crashes and early RFH
|
||||
swaps.
|
||||
|
||||
When resuming a navigation that had been saved into
|
||||
RenderFrameHostImpl::pending_navigate_, we need to account for the
|
||||
fact that OnBeginNavigation() calls GetFrameHostForNavigation() which
|
||||
may perform an early RenderFrameHost swap and synchronously destroy
|
||||
the old RFH.
|
||||
|
||||
There's also no need to keep a pending_navigate_ around after the
|
||||
corresponding renderer process crashes, so this CL also adds logic to
|
||||
clear it. Resuming such a navigation would require additional work,
|
||||
since the NavigationClient stashed in pending_navigate_ is no longer
|
||||
usable and would just immediately call the disconnect handler and
|
||||
cancel the navigation. But there isn't really any benefit to adding
|
||||
that complexity, and we already cancel the RFH's other ongoing
|
||||
navigations when its renderer process dies.
|
||||
|
||||
This CL also tweaks the logic in RenderWidgetHostImpl to allow the
|
||||
resuming logic (ResumeLoadingCreatedWebContents) to work without
|
||||
hitting DCHECKs, if it's called after a renderer process crash. This
|
||||
case never worked cleanly before, but is supported now (and allows the
|
||||
new test to work without crashing).
|
||||
|
||||
(cherry picked from commit 093daae65d50511c2027d01f9188681749b5a1be)
|
||||
|
||||
Bug: 1487110
|
||||
Change-Id: Icd6a55002e52729e6ee966210efba1a5ce23eb55
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908270
|
||||
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
|
||||
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
|
||||
Cr-Original-Commit-Position: refs/heads/main@{#1205927}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4923011
|
||||
Owners-Override: Krishna Govind <govind@chromium.org>
|
||||
Reviewed-by: Krishna Govind <govind@chromium.org>
|
||||
Commit-Queue: Krishna Govind <govind@chromium.org>
|
||||
Cr-Commit-Position: refs/branch-heads/5993@{#1208}
|
||||
Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594}
|
||||
|
||||
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
|
||||
index 820afdd8c73b6dbcf3aceba38af2cf0e202bd49a..17954b8b5796d02eeeb291d93aff4353c1ec1411 100644
|
||||
--- a/content/browser/renderer_host/render_frame_host_impl.cc
|
||||
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
|
||||
@@ -3133,6 +3133,13 @@ void RenderFrameHostImpl::RenderProcessGone(
|
||||
ResetOwnedNavigationRequests(NavigationDiscardReason::kRenderProcessGone);
|
||||
ResetLoadingState();
|
||||
|
||||
+ // Also, clear any pending navigations that have been blocked while the
|
||||
+ // embedder is processing window.open() requests. This is consistent
|
||||
+ // with clearing NavigationRequests and loading state above, and it also
|
||||
+ // makes sense because certain parts of `pending_navigate_`, like the
|
||||
+ // NavigationClient remote interface, can no longer be used.
|
||||
+ pending_navigate_.reset();
|
||||
+
|
||||
// Any future UpdateState or UpdateTitle messages from this or a recreated
|
||||
// process should be ignored until the next commit.
|
||||
set_nav_entry_id(0);
|
||||
@@ -3537,14 +3544,25 @@ void RenderFrameHostImpl::Init() {
|
||||
});
|
||||
|
||||
if (pending_navigate_) {
|
||||
+
|
||||
+ // Transfer `pending_navigate_` to a local variable, to avoid resetting it
|
||||
+ // after OnBeginNavigation since `this` might already be destroyed (see
|
||||
+ // below).
|
||||
+ //
|
||||
+ // This shouldn't matter for early RFH swaps out of crashed frames, since
|
||||
+ // `pending_navigate_` is cleared when the renderer process dies, but it
|
||||
+ // may matter for other current/future use cases of the early RFH swap.
|
||||
+ std::unique_ptr<PendingNavigation> pending_navigation =
|
||||
+ std::move(pending_navigate_);
|
||||
frame_tree_node()->navigator().OnBeginNavigation(
|
||||
- frame_tree_node(), std::move(pending_navigate_->common_params),
|
||||
- std::move(pending_navigate_->begin_navigation_params),
|
||||
- std::move(pending_navigate_->blob_url_loader_factory),
|
||||
- std::move(pending_navigate_->navigation_client),
|
||||
+ frame_tree_node(), std::move(pending_navigation->common_params),
|
||||
+ std::move(pending_navigation->begin_navigation_params),
|
||||
+ std::move(pending_navigation->blob_url_loader_factory),
|
||||
+ std::move(pending_navigation->navigation_client),
|
||||
EnsurePrefetchedSignedExchangeCache(),
|
||||
- std::move(pending_navigate_->renderer_cancellation_listener));
|
||||
- pending_navigate_.reset();
|
||||
+ std::move(pending_navigation->renderer_cancellation_listener));
|
||||
+ // DO NOT ADD CODE after this, as `this` might be deleted if an early
|
||||
+ // RenderFrameHost swap was performed when starting the navigation above.
|
||||
}
|
||||
}
|
||||
|
||||
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
|
||||
index 07b6f899132ab4b94fb9474d441461c8fcf53705..4df864783b69bad7c578341ccf995c4a7ef62b62 100644
|
||||
--- a/content/browser/renderer_host/render_widget_host_impl.cc
|
||||
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
|
||||
@@ -756,9 +756,15 @@ void RenderWidgetHostImpl::RendererWidgetCreated(bool for_frame_widget) {
|
||||
}
|
||||
|
||||
void RenderWidgetHostImpl::Init() {
|
||||
- DCHECK(renderer_widget_created_);
|
||||
- DCHECK(waiting_for_init_);
|
||||
+ // Note that this may be called after a renderer crash. In this case, we can
|
||||
+ // just exit early, as there is nothing else to do. Note that
|
||||
+ // `waiting_for_init_` should've already been reset to false in that case.
|
||||
+ if (!renderer_widget_created_) {
|
||||
+ DCHECK(!waiting_for_init_);
|
||||
+ return;
|
||||
+ }
|
||||
|
||||
+ DCHECK(waiting_for_init_);
|
||||
waiting_for_init_ = false;
|
||||
|
||||
// These two methods avoid running while we are `waiting_for_init_`, so we
|
||||
@@ -2215,6 +2221,10 @@ void RenderWidgetHostImpl::RendererExited() {
|
||||
|
||||
blink_widget_.reset();
|
||||
|
||||
+ // No need to perform a deferred show after the renderer crashes, and this
|
||||
+ // wouldn't work anyway as it requires a valid `blink_widget_`.
|
||||
+ pending_show_params_.reset();
|
||||
+
|
||||
// After the renderer crashes, the view is destroyed and so the
|
||||
// RenderWidgetHost cannot track its visibility anymore. We assume such
|
||||
// RenderWidgetHost to be invisible for the sake of internal accounting - be
|
||||
diff --git a/content/browser/web_contents/web_contents_impl_browsertest.cc b/content/browser/web_contents/web_contents_impl_browsertest.cc
|
||||
index e008d0adb0e643b7796adf13946b19ab6cfaba56..b48b8fc0fc1e56e327663f9230ea599a86b1eb87 100644
|
||||
--- a/content/browser/web_contents/web_contents_impl_browsertest.cc
|
||||
+++ b/content/browser/web_contents/web_contents_impl_browsertest.cc
|
||||
@@ -81,6 +81,7 @@
|
||||
#include "content/public/test/fenced_frame_test_util.h"
|
||||
#include "content/public/test/mock_client_hints_controller_delegate.h"
|
||||
#include "content/public/test/mock_web_contents_observer.h"
|
||||
+#include "content/public/test/navigation_handle_observer.h"
|
||||
#include "content/public/test/no_renderer_crashes_assertion.h"
|
||||
#include "content/public/test/prerender_test_util.h"
|
||||
#include "content/public/test/test_navigation_observer.h"
|
||||
@@ -5575,6 +5576,63 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
|
||||
EXPECT_FALSE(shell()->web_contents()->IsCrashed());
|
||||
}
|
||||
|
||||
+// Check that there's no crash if a new window is set to defer navigations (for
|
||||
+// example, this is done on Android Webview and for <webview> guests), then the
|
||||
+// renderer process crashes while there's a deferred new window navigation in
|
||||
+// place, and then navigations are resumed. Prior to fixing
|
||||
+// https://crbug.com/1487110, the deferred navigation was allowed to proceed,
|
||||
+// performing an early RenderFrameHost swap and hitting a bug while clearing
|
||||
+// the deferred navigation state. Now, the deferred navigation should be
|
||||
+// canceled when the renderer process dies.
|
||||
+IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
|
||||
+ DeferredWindowOpenNavigationIsResumedWithEarlySwap) {
|
||||
+ // Force WebContents in a new Shell to defer new navigations until the
|
||||
+ // delegate is set.
|
||||
+ shell()->set_delay_popup_contents_delegate_for_testing(true);
|
||||
+
|
||||
+ // Load an initial page.
|
||||
+ ASSERT_TRUE(embedded_test_server()->Start());
|
||||
+ GURL url(embedded_test_server()->GetURL("/title1.html"));
|
||||
+ EXPECT_TRUE(NavigateToURL(shell(), url));
|
||||
+
|
||||
+ // Open a popup to a same-site URL via window.open.
|
||||
+ ShellAddedObserver new_shell_observer;
|
||||
+ EXPECT_TRUE(ExecJs(shell(), JsReplace("window.open($1);", url)));
|
||||
+ Shell* new_shell = new_shell_observer.GetShell();
|
||||
+ WebContents* new_contents = new_shell->web_contents();
|
||||
+
|
||||
+ // The navigation in the new popup should be deferred.
|
||||
+ EXPECT_TRUE(WaitForLoadStop(new_contents));
|
||||
+ EXPECT_TRUE(new_contents->GetController().IsInitialBlankNavigation());
|
||||
+ EXPECT_TRUE(new_contents->GetLastCommittedURL().is_empty());
|
||||
+
|
||||
+ // Set the new shell's delegate now. This doesn't resume the navigation just
|
||||
+ // yet.
|
||||
+ EXPECT_FALSE(new_contents->GetDelegate());
|
||||
+ new_contents->SetDelegate(new_shell);
|
||||
+
|
||||
+ // Crash the renderer process. This should clear the deferred navigation
|
||||
+ // state. If this wasn't done due to a bug, it would also force the resumed
|
||||
+ // navigation to use the early RenderFrameHost swap.
|
||||
+ {
|
||||
+ RenderProcessHost* popup_process =
|
||||
+ new_contents->GetPrimaryMainFrame()->GetProcess();
|
||||
+ RenderProcessHostWatcher crash_observer(
|
||||
+ popup_process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
|
||||
+ EXPECT_TRUE(popup_process->Shutdown(0));
|
||||
+ crash_observer.Wait();
|
||||
+ }
|
||||
+
|
||||
+ // Resume the navigation and verify that it gets canceled. Ensure this
|
||||
+ // doesn't crash.
|
||||
+ NavigationHandleObserver handle_observer(new_contents, url);
|
||||
+ new_contents->ResumeLoadingCreatedWebContents();
|
||||
+ EXPECT_TRUE(WaitForLoadStop(new_contents));
|
||||
+ EXPECT_FALSE(handle_observer.has_committed());
|
||||
+ EXPECT_TRUE(new_contents->GetController().IsInitialBlankNavigation());
|
||||
+ EXPECT_TRUE(new_contents->GetLastCommittedURL().is_empty());
|
||||
+}
|
||||
+
|
||||
namespace {
|
||||
|
||||
class MediaWaiter : public WebContentsObserver {
|
||||
56
patches/chromium/cherry-pick-f218b4f37018.patch
Normal file
56
patches/chromium/cherry-pick-f218b4f37018.patch
Normal file
@@ -0,0 +1,56 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Joshua Pawlicki <waffles@chromium.org>
|
||||
Date: Fri, 29 Sep 2023 22:25:20 +0000
|
||||
Subject: update_client: Check string length before calling front().
|
||||
|
||||
(cherry picked from commit 6581c6b7c4b7b627b32b09cdbe9e84d2dd9ec018)
|
||||
|
||||
Fixed: 1486316
|
||||
Change-Id: I7be04ea0c8e040b5a67364925fc06d4ee9167242
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4892838
|
||||
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
|
||||
Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
|
||||
Reviewed-by: Sorin Jianu <sorin@chromium.org>
|
||||
Cr-Original-Commit-Position: refs/heads/main@{#1201617}
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4902697
|
||||
Cr-Commit-Position: refs/branch-heads/5993@{#978}
|
||||
Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594}
|
||||
|
||||
diff --git a/components/update_client/protocol_parser_json.cc b/components/update_client/protocol_parser_json.cc
|
||||
index 9858ba06290675ae39d2606fbe4aea34f2efe655..ba1aeba747d990bc97977bebb60f1141e3981517 100644
|
||||
--- a/components/update_client/protocol_parser_json.cc
|
||||
+++ b/components/update_client/protocol_parser_json.cc
|
||||
@@ -196,7 +196,7 @@ bool ParseUpdateCheck(const base::Value& updatecheck_node_val,
|
||||
const base::Value::Dict& updatecheck_node = updatecheck_node_val.GetDict();
|
||||
|
||||
for (auto kv : updatecheck_node) {
|
||||
- if (kv.first.front() == '_' && kv.second.is_string()) {
|
||||
+ if (!kv.first.empty() && kv.first.front() == '_' && kv.second.is_string()) {
|
||||
result->custom_attributes[kv.first] = kv.second.GetString();
|
||||
}
|
||||
}
|
||||
diff --git a/components/update_client/protocol_parser_json_unittest.cc b/components/update_client/protocol_parser_json_unittest.cc
|
||||
index 606c6ddc8f906e4bcd0722828ab34f5c77e73e5d..c59bf8ab7b1f7751c2e78396712f0de283c3b00d 100644
|
||||
--- a/components/update_client/protocol_parser_json_unittest.cc
|
||||
+++ b/components/update_client/protocol_parser_json_unittest.cc
|
||||
@@ -393,6 +393,10 @@ const char* kJSONCustomAttributes = R"()]}'
|
||||
]
|
||||
}})";
|
||||
|
||||
+const char* kBadJSONBadAppIdNoNewlinesBadUCKey =
|
||||
+ R"()]}'{"response":{"app":[{"appid":";","updatecheck":{"":1}}],)"
|
||||
+ R"("protocol":"3.1"}})";
|
||||
+
|
||||
TEST(UpdateClientProtocolParserJSONTest, Parse) {
|
||||
const auto parser = std::make_unique<ProtocolParserJSON>();
|
||||
|
||||
@@ -610,4 +614,9 @@ TEST(UpdateClientProtocolParserJSONTest, ParseAttrs) {
|
||||
}
|
||||
}
|
||||
|
||||
+TEST(UpdateClientProtocolParserJSONTest, ParseBadJSONNoCrash) {
|
||||
+ const auto parser = std::make_unique<ProtocolParserJSON>();
|
||||
+ EXPECT_TRUE(parser->Parse(kBadJSONBadAppIdNoNewlinesBadUCKey));
|
||||
+}
|
||||
+
|
||||
} // namespace update_client
|
||||
@@ -0,0 +1,511 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Chris Harrelson <chrishtr@chromium.org>
|
||||
Date: Fri, 25 Aug 2023 17:55:16 +0000
|
||||
Subject: Parameterize AXTreeSerializer by vector type
|
||||
|
||||
This enables us to use HeapVector in Blink, which is necessary because
|
||||
AXObject is a garbage-collected type.
|
||||
|
||||
Bug: 1472368
|
||||
|
||||
Change-Id: Idf6fee18827f2914452be49f61de0b6a36b19678
|
||||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4811206
|
||||
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
|
||||
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
|
||||
Reviewed-by: Mark Schillaci <mschillaci@google.com>
|
||||
Cr-Commit-Position: refs/heads/main@{#1188419}
|
||||
|
||||
diff --git a/chrome/browser/ash/arc/accessibility/ax_tree_source_arc.h b/chrome/browser/ash/arc/accessibility/ax_tree_source_arc.h
|
||||
index b7e0e6dea25fff3deae082f30a765df43756d3df..02c15515f119c48e4d48ffca15ce0049a2b40c16 100644
|
||||
--- a/chrome/browser/ash/arc/accessibility/ax_tree_source_arc.h
|
||||
+++ b/chrome/browser/ash/arc/accessibility/ax_tree_source_arc.h
|
||||
@@ -31,7 +31,9 @@ class Window;
|
||||
namespace arc {
|
||||
class AXTreeSourceArcTest;
|
||||
|
||||
-using AXTreeArcSerializer = ui::AXTreeSerializer<AccessibilityInfoDataWrapper*>;
|
||||
+using AXTreeArcSerializer =
|
||||
+ ui::AXTreeSerializer<AccessibilityInfoDataWrapper*,
|
||||
+ std::vector<AccessibilityInfoDataWrapper*>>;
|
||||
|
||||
// This class represents the accessibility tree from the focused ARC window.
|
||||
class AXTreeSourceArc : public ui::AXTreeSource<AccessibilityInfoDataWrapper*>,
|
||||
diff --git a/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc b/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc
|
||||
index f973c8c0c2c4b1f96a4ab7e22a924b3d85b44f35..e26827761686946669e21ba6459b24b571bdb4c0 100644
|
||||
--- a/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc
|
||||
+++ b/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc
|
||||
@@ -33,7 +33,9 @@ using views::Textfield;
|
||||
using views::View;
|
||||
using views::Widget;
|
||||
|
||||
-using AuraAXTreeSerializer = ui::AXTreeSerializer<views::AXAuraObjWrapper*>;
|
||||
+using AuraAXTreeSerializer =
|
||||
+ ui::AXTreeSerializer<views::AXAuraObjWrapper*,
|
||||
+ std::vector<views::AXAuraObjWrapper*>>;
|
||||
|
||||
// Helper to count the number of nodes in a tree.
|
||||
size_t GetSize(AXAuraObjWrapper* tree) {
|
||||
diff --git a/chrome/browser/ui/aura/accessibility/automation_manager_aura.h b/chrome/browser/ui/aura/accessibility/automation_manager_aura.h
|
||||
index 498488f27e7fc273a0797063b6388576dfc5cd93..4d0ae2b291ce5bd607117288b0bfc1d3ce8e1c13 100644
|
||||
--- a/chrome/browser/ui/aura/accessibility/automation_manager_aura.h
|
||||
+++ b/chrome/browser/ui/aura/accessibility/automation_manager_aura.h
|
||||
@@ -33,7 +33,9 @@ class AXAuraObjWrapper;
|
||||
class View;
|
||||
} // namespace views
|
||||
|
||||
-using AuraAXTreeSerializer = ui::AXTreeSerializer<views::AXAuraObjWrapper*>;
|
||||
+using AuraAXTreeSerializer =
|
||||
+ ui::AXTreeSerializer<views::AXAuraObjWrapper*,
|
||||
+ std::vector<views::AXAuraObjWrapper*>>;
|
||||
|
||||
// Manages a tree of automation nodes backed by aura constructs.
|
||||
class AutomationManagerAura : public ui::AXActionHandler,
|
||||
diff --git a/chrome/renderer/accessibility/read_anything_app_controller.cc b/chrome/renderer/accessibility/read_anything_app_controller.cc
|
||||
index a30a4fb78d837d98a7c9532f2643ceac742ac5e5..89e45eae934f417c571f2eae8dfa76a977ead825 100644
|
||||
--- a/chrome/renderer/accessibility/read_anything_app_controller.cc
|
||||
+++ b/chrome/renderer/accessibility/read_anything_app_controller.cc
|
||||
@@ -401,7 +401,8 @@ void ReadAnythingAppController::Distill() {
|
||||
model_.GetTreeFromId(model_.active_tree_id()).get();
|
||||
std::unique_ptr<ui::AXTreeSource<const ui::AXNode*>> tree_source(
|
||||
tree->CreateTreeSource());
|
||||
- ui::AXTreeSerializer<const ui::AXNode*> serializer(tree_source.get());
|
||||
+ ui::AXTreeSerializer<const ui::AXNode*, std::vector<const ui::AXNode*>>
|
||||
+ serializer(tree_source.get());
|
||||
ui::AXTreeUpdate snapshot;
|
||||
CHECK(serializer.SerializeChanges(tree->root(), &snapshot));
|
||||
model_.SetDistillationInProgress(true);
|
||||
diff --git a/components/services/screen_ai/screen_ai_ax_tree_serializer.cc b/components/services/screen_ai/screen_ai_ax_tree_serializer.cc
|
||||
index ecd818228f31dfefdc436302be9dcb47415b684c..6f921bd47e8dfb585da81cefdbdcf870f420b305 100644
|
||||
--- a/components/services/screen_ai/screen_ai_ax_tree_serializer.cc
|
||||
+++ b/components/services/screen_ai/screen_ai_ax_tree_serializer.cc
|
||||
@@ -43,7 +43,8 @@ ScreenAIAXTreeSerializer::ScreenAIAXTreeSerializer(
|
||||
tree_source_ = base::WrapUnique<ui::AXTreeSource<const ui::AXNode*>>(
|
||||
tree_->CreateTreeSource());
|
||||
DCHECK(tree_source_);
|
||||
- serializer_ = std::make_unique<ui::AXTreeSerializer<const ui::AXNode*>>(
|
||||
+ serializer_ = std::make_unique<
|
||||
+ ui::AXTreeSerializer<const ui::AXNode*, std::vector<const ui::AXNode*>>>(
|
||||
tree_source_.get(), /* crash_on_error */ true);
|
||||
}
|
||||
|
||||
diff --git a/components/services/screen_ai/screen_ai_ax_tree_serializer.h b/components/services/screen_ai/screen_ai_ax_tree_serializer.h
|
||||
index e522698819c4fe80bfd9da0b49934def0d2abb66..38fd5e2391a4a899f0f46069b42d6056d42f1e01 100644
|
||||
--- a/components/services/screen_ai/screen_ai_ax_tree_serializer.h
|
||||
+++ b/components/services/screen_ai/screen_ai_ax_tree_serializer.h
|
||||
@@ -41,7 +41,9 @@ class ScreenAIAXTreeSerializer final {
|
||||
private:
|
||||
const std::unique_ptr<ui::AXSerializableTree> tree_;
|
||||
std::unique_ptr<ui::AXTreeSource<const ui::AXNode*>> tree_source_;
|
||||
- mutable std::unique_ptr<ui::AXTreeSerializer<const ui::AXNode*>> serializer_;
|
||||
+ mutable std::unique_ptr<
|
||||
+ ui::AXTreeSerializer<const ui::AXNode*, std::vector<const ui::AXNode*>>>
|
||||
+ serializer_;
|
||||
};
|
||||
|
||||
} // namespace screen_ai
|
||||
diff --git a/content/browser/accessibility/browser_accessibility_manager.cc b/content/browser/accessibility/browser_accessibility_manager.cc
|
||||
index 482267f40346c0b0bb4cde27e8a581ec18bef0fd..727907e7d3b804c98625bbc90f9a4f15d3349638 100644
|
||||
--- a/content/browser/accessibility/browser_accessibility_manager.cc
|
||||
+++ b/content/browser/accessibility/browser_accessibility_manager.cc
|
||||
@@ -1649,7 +1649,8 @@ bool BrowserAccessibilityManager::IsRootFrameManager() const {
|
||||
ui::AXTreeUpdate BrowserAccessibilityManager::SnapshotAXTreeForTesting() {
|
||||
std::unique_ptr<ui::AXTreeSource<const ui::AXNode*>> tree_source(
|
||||
ax_serializable_tree()->CreateTreeSource());
|
||||
- ui::AXTreeSerializer<const ui::AXNode*> serializer(tree_source.get());
|
||||
+ ui::AXTreeSerializer<const ui::AXNode*, std::vector<const ui::AXNode*>>
|
||||
+ serializer(tree_source.get());
|
||||
ui::AXTreeUpdate update;
|
||||
serializer.SerializeChanges(GetRoot(), &update);
|
||||
return update;
|
||||
diff --git a/content/renderer/accessibility/render_accessibility_impl.h b/content/renderer/accessibility/render_accessibility_impl.h
|
||||
index ddd8273fb0384d326f5ee2c78de23e3e03afe520..088edf542ef06c17e0e149ba5927fae16ee38817 100644
|
||||
--- a/content/renderer/accessibility/render_accessibility_impl.h
|
||||
+++ b/content/renderer/accessibility/render_accessibility_impl.h
|
||||
@@ -278,7 +278,8 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
|
||||
// Manages the automatic image annotations, if enabled.
|
||||
std::unique_ptr<AXImageAnnotator> ax_image_annotator_;
|
||||
|
||||
- using PluginAXTreeSerializer = ui::AXTreeSerializer<const ui::AXNode*>;
|
||||
+ using PluginAXTreeSerializer =
|
||||
+ ui::AXTreeSerializer<const ui::AXNode*, std::vector<const ui::AXNode*>>;
|
||||
std::unique_ptr<PluginAXTreeSerializer> plugin_serializer_;
|
||||
PluginAXTreeSource* plugin_tree_source_;
|
||||
|
||||
diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
|
||||
index 0de34e2c2327120ce357fbbc9594bb6ffb9a6d6c..b0d37309e7dbd3b9e6061d2269e01e193fef5932 100644
|
||||
--- a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
|
||||
+++ b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
|
||||
@@ -691,9 +691,11 @@ AXObjectCacheImpl::AXObjectCacheImpl(Document& document,
|
||||
permission_observer_receiver_(this, document.GetExecutionContext()),
|
||||
render_accessibility_host_(document.GetExecutionContext()),
|
||||
ax_tree_source_(BlinkAXTreeSource::Create(*this)),
|
||||
- ax_tree_serializer_(std::make_unique<ui::AXTreeSerializer<AXObject*>>(
|
||||
- ax_tree_source_,
|
||||
- /*crash_on_error*/ true)) {
|
||||
+ ax_tree_serializer_(
|
||||
+ std::make_unique<
|
||||
+ ui::AXTreeSerializer<AXObject*, HeapVector<AXObject*>>>(
|
||||
+ ax_tree_source_,
|
||||
+ /*crash_on_error*/ true)) {
|
||||
use_ax_menu_list_ = GetSettings()->GetUseAXMenuList();
|
||||
}
|
||||
|
||||
@@ -3987,7 +3989,8 @@ bool AXObjectCacheImpl::SerializeEntireTree(size_t max_node_count,
|
||||
// or a partial accessibility tree. AXTreeSerializer is stateful, but the
|
||||
// first time you serialize from a brand-new tree you're guaranteed to get a
|
||||
// complete tree.
|
||||
- ui::AXTreeSerializer<AXObject*> serializer(tree_source);
|
||||
+ ui::AXTreeSerializer<AXObject*, HeapVector<AXObject*>> serializer(
|
||||
+ tree_source);
|
||||
|
||||
if (max_node_count)
|
||||
serializer.set_max_node_count(max_node_count);
|
||||
diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
|
||||
index eeea077f3bc086065b2dbed1fe38c3ee572e83a1..339eae38265b206e4a99447152503b39e12bf00d 100644
|
||||
--- a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
|
||||
+++ b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
|
||||
@@ -930,7 +930,8 @@ class MODULES_EXPORT AXObjectCacheImpl
|
||||
render_accessibility_host_;
|
||||
|
||||
Member<BlinkAXTreeSource> ax_tree_source_;
|
||||
- std::unique_ptr<ui::AXTreeSerializer<AXObject*>> ax_tree_serializer_;
|
||||
+ std::unique_ptr<ui::AXTreeSerializer<AXObject*, HeapVector<AXObject*>>>
|
||||
+ ax_tree_serializer_;
|
||||
|
||||
HeapDeque<Member<AXDirtyObject>> dirty_objects_;
|
||||
|
||||
diff --git a/ui/accessibility/ax_generated_tree_unittest.cc b/ui/accessibility/ax_generated_tree_unittest.cc
|
||||
index 2e24152f026a1f6962af348c96744218c783561c..cea591f9a9315ec6476ddec5411a04bb04f49df3 100644
|
||||
--- a/ui/accessibility/ax_generated_tree_unittest.cc
|
||||
+++ b/ui/accessibility/ax_generated_tree_unittest.cc
|
||||
@@ -75,7 +75,8 @@ std::string TreeToString(const AXTree& tree) {
|
||||
AXTreeUpdate SerializeEntireTree(AXSerializableTree& tree) {
|
||||
std::unique_ptr<AXTreeSource<const AXNode*>> tree_source(
|
||||
tree.CreateTreeSource());
|
||||
- AXTreeSerializer<const AXNode*> serializer(tree_source.get());
|
||||
+ AXTreeSerializer<const AXNode*, std::vector<const AXNode*>> serializer(
|
||||
+ tree_source.get());
|
||||
AXTreeUpdate update;
|
||||
CHECK(serializer.SerializeChanges(tree.root(), &update));
|
||||
return update;
|
||||
@@ -270,7 +271,8 @@ TEST_P(SerializeGeneratedTreesTest, SerializeGeneratedTrees) {
|
||||
// empty tree |dst_tree|.
|
||||
std::unique_ptr<AXTreeSource<const AXNode*>> tree0_source(
|
||||
tree0.CreateTreeSource());
|
||||
- AXTreeSerializer<const AXNode*> serializer(tree0_source.get());
|
||||
+ AXTreeSerializer<const AXNode*, std::vector<const AXNode*>>
|
||||
+ serializer(tree0_source.get());
|
||||
AXTreeUpdate update0;
|
||||
ASSERT_TRUE(serializer.SerializeChanges(tree0.root(), &update0));
|
||||
|
||||
diff --git a/ui/accessibility/ax_tree_serializer.h b/ui/accessibility/ax_tree_serializer.h
|
||||
index 0fb9303c39c979ab80c572769c2344701478f7e3..7874a5575823489aef2afa03b7f325e56ab689d1 100644
|
||||
--- a/ui/accessibility/ax_tree_serializer.h
|
||||
+++ b/ui/accessibility/ax_tree_serializer.h
|
||||
@@ -64,7 +64,7 @@ struct ClientTreeNode;
|
||||
// because AXTreeSerializer always keeps track of what updates it's sent,
|
||||
// it will never send an invalid update and the client tree will not break,
|
||||
// it just may not contain all of the changes.
|
||||
-template <typename AXSourceNode>
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
class AXTreeSerializer {
|
||||
public:
|
||||
explicit AXTreeSerializer(AXTreeSource<AXSourceNode>* tree,
|
||||
@@ -260,14 +260,14 @@ struct AX_EXPORT ClientTreeNode {
|
||||
bool in_dirty_subtree;
|
||||
};
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-AXTreeSerializer<AXSourceNode>::AXTreeSerializer(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::AXTreeSerializer(
|
||||
AXTreeSource<AXSourceNode>* tree,
|
||||
bool crash_on_error)
|
||||
: tree_(tree), crash_on_error_(crash_on_error) {}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-AXTreeSerializer<AXSourceNode>::~AXTreeSerializer() {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::~AXTreeSerializer() {
|
||||
// Clear |tree_| to prevent any additional calls to the tree source
|
||||
// during teardown.
|
||||
// TODO(accessibility) How would that happen?
|
||||
@@ -276,14 +276,14 @@ AXTreeSerializer<AXSourceNode>::~AXTreeSerializer() {
|
||||
Reset();
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::Reset() {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::Reset() {
|
||||
InternalReset();
|
||||
did_reset_ = true;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::InternalReset() {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::InternalReset() {
|
||||
client_tree_data_ = AXTreeData();
|
||||
|
||||
// Normally we use DeleteClientSubtree to remove nodes from the tree,
|
||||
@@ -296,26 +296,28 @@ void AXTreeSerializer<AXSourceNode>::InternalReset() {
|
||||
client_root_ = nullptr;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::ChangeTreeSourceForTesting(
|
||||
- AXTreeSource<AXSourceNode>* new_tree) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::
|
||||
+ ChangeTreeSourceForTesting(AXTreeSource<AXSourceNode>* new_tree) {
|
||||
tree_ = new_tree;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-size_t AXTreeSerializer<AXSourceNode>::ClientTreeNodeCount() const {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+size_t AXTreeSerializer<AXSourceNode,
|
||||
+ AXSourceNodeVectorType>::ClientTreeNodeCount() const {
|
||||
return client_id_map_.size();
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-AXSourceNode AXTreeSerializer<AXSourceNode>::LeastCommonAncestor(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+AXSourceNode
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::LeastCommonAncestor(
|
||||
AXSourceNode node,
|
||||
ClientTreeNode* client_node) {
|
||||
if (!node || client_node == nullptr) {
|
||||
return tree_->GetNull();
|
||||
}
|
||||
|
||||
- std::vector<AXSourceNode> ancestors;
|
||||
+ AXSourceNodeVectorType ancestors;
|
||||
while (node) {
|
||||
ancestors.push_back(node);
|
||||
node = tree_->GetParent(node);
|
||||
@@ -334,17 +336,18 @@ AXSourceNode AXTreeSerializer<AXSourceNode>::LeastCommonAncestor(
|
||||
for (size_t source_index = ancestors.size(),
|
||||
client_index = client_ancestors.size();
|
||||
source_index > 0 && client_index > 0; --source_index, --client_index) {
|
||||
- if (tree_->GetId(ancestors[source_index - 1]) !=
|
||||
+ if (tree_->GetId(ancestors[(unsigned int)(source_index - 1)]) !=
|
||||
client_ancestors[client_index - 1]->id) {
|
||||
return lca;
|
||||
}
|
||||
- lca = ancestors[source_index - 1];
|
||||
+ lca = ancestors[(unsigned int)(source_index - 1)];
|
||||
}
|
||||
return lca;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-AXSourceNode AXTreeSerializer<AXSourceNode>::LeastCommonAncestor(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+AXSourceNode
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::LeastCommonAncestor(
|
||||
AXSourceNode node) {
|
||||
// Walk up the tree until the source node's id also exists in the
|
||||
// client tree, whose parent is not invalid, then call LeastCommonAncestor
|
||||
@@ -371,10 +374,9 @@ AXSourceNode AXTreeSerializer<AXSourceNode>::LeastCommonAncestor(
|
||||
return LeastCommonAncestor(node, client_node);
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-bool AXTreeSerializer<AXSourceNode>::AnyDescendantWasReparented(
|
||||
- AXSourceNode node,
|
||||
- AXSourceNode* out_lca) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+bool AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::
|
||||
+ AnyDescendantWasReparented(AXSourceNode node, AXSourceNode* out_lca) {
|
||||
bool result = false;
|
||||
int id = tree_->GetId(node);
|
||||
tree_->CacheChildrenIfNeeded(node);
|
||||
@@ -422,8 +424,9 @@ bool AXTreeSerializer<AXSourceNode>::AnyDescendantWasReparented(
|
||||
return result;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-ClientTreeNode* AXTreeSerializer<AXSourceNode>::ClientTreeNodeById(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+ClientTreeNode*
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::ClientTreeNodeById(
|
||||
AXNodeID id) {
|
||||
std::map<AXNodeID, ClientTreeNode*>::iterator iter = client_id_map_.find(id);
|
||||
if (iter != client_id_map_.end())
|
||||
@@ -431,8 +434,9 @@ ClientTreeNode* AXTreeSerializer<AXSourceNode>::ClientTreeNodeById(
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-ClientTreeNode* AXTreeSerializer<AXSourceNode>::GetClientTreeNodeParent(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+ClientTreeNode*
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::GetClientTreeNodeParent(
|
||||
ClientTreeNode* obj) {
|
||||
ClientTreeNode* parent = obj->parent;
|
||||
if (!parent)
|
||||
@@ -457,8 +461,8 @@ ClientTreeNode* AXTreeSerializer<AXSourceNode>::GetClientTreeNodeParent(
|
||||
return parent;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-bool AXTreeSerializer<AXSourceNode>::SerializeChanges(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+bool AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::SerializeChanges(
|
||||
AXSourceNode node,
|
||||
AXTreeUpdate* out_update) {
|
||||
if (!timeout_.is_zero())
|
||||
@@ -540,33 +544,37 @@ bool AXTreeSerializer<AXSourceNode>::SerializeChanges(
|
||||
return true;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-std::vector<AXNodeID> AXTreeSerializer<AXSourceNode>::GetIncompleteNodeIds() {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+std::vector<AXNodeID>
|
||||
+AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::GetIncompleteNodeIds() {
|
||||
DCHECK(max_node_count_ > 0 || !timeout_.is_zero());
|
||||
return incomplete_node_ids_;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::MarkSubtreeDirty(AXSourceNode node) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::MarkSubtreeDirty(
|
||||
+ AXSourceNode node) {
|
||||
ClientTreeNode* client_node = ClientTreeNodeById(tree_->GetId(node));
|
||||
if (client_node)
|
||||
MarkClientSubtreeDirty(client_node);
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-bool AXTreeSerializer<AXSourceNode>::IsInClientTree(AXSourceNode node) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+bool AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::IsInClientTree(
|
||||
+ AXSourceNode node) {
|
||||
return ClientTreeNodeById(tree_->GetId(node));
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-bool AXTreeSerializer<AXSourceNode>::IsDirty(AXSourceNode node) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+bool AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::IsDirty(
|
||||
+ AXSourceNode node) {
|
||||
ClientTreeNode* client_node = ClientTreeNodeById(tree_->GetId(node));
|
||||
return client_node ? client_node->in_dirty_subtree : false;
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::MarkClientSubtreeDirty(
|
||||
- ClientTreeNode* client_node) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::
|
||||
+ MarkClientSubtreeDirty(ClientTreeNode* client_node) {
|
||||
// Return early if already marked dirty, in order to avoid duplicate work in
|
||||
// subtree, as the only method that marks nodes dirty is this one.
|
||||
if (client_node->in_dirty_subtree) {
|
||||
@@ -578,9 +586,9 @@ void AXTreeSerializer<AXSourceNode>::MarkClientSubtreeDirty(
|
||||
}
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::DeleteClientSubtree(
|
||||
- ClientTreeNode* client_node) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::
|
||||
+ DeleteClientSubtree(ClientTreeNode* client_node) {
|
||||
if (client_node == client_root_) {
|
||||
Reset(); // Do not try to reuse a bad root later.
|
||||
// A heuristic for this condition rather than an explicit Reset() from a
|
||||
@@ -601,18 +609,17 @@ void AXTreeSerializer<AXSourceNode>::DeleteClientSubtree(
|
||||
}
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-void AXTreeSerializer<AXSourceNode>::DeleteDescendants(
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+void AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::DeleteDescendants(
|
||||
ClientTreeNode* client_node) {
|
||||
for (size_t i = 0; i < client_node->children.size(); ++i)
|
||||
DeleteClientSubtree(client_node->children[i]);
|
||||
client_node->children.clear();
|
||||
}
|
||||
|
||||
-template <typename AXSourceNode>
|
||||
-bool AXTreeSerializer<AXSourceNode>::SerializeChangedNodes(
|
||||
- AXSourceNode node,
|
||||
- AXTreeUpdate* out_update) {
|
||||
+template <typename AXSourceNode, typename AXSourceNodeVectorType>
|
||||
+bool AXTreeSerializer<AXSourceNode, AXSourceNodeVectorType>::
|
||||
+ SerializeChangedNodes(AXSourceNode node, AXTreeUpdate* out_update) {
|
||||
// This method has three responsibilities:
|
||||
// 1. Serialize |node| into an AXNodeData, and append it to
|
||||
// the AXTreeUpdate to be sent to the client.
|
||||
diff --git a/ui/accessibility/ax_tree_serializer_unittest.cc b/ui/accessibility/ax_tree_serializer_unittest.cc
|
||||
index 3955a06f31795d0941d76976d8ae0a5c3204eedd..126e43a2bdcb1f9780baac6657201f26b3bd3244 100644
|
||||
--- a/ui/accessibility/ax_tree_serializer_unittest.cc
|
||||
+++ b/ui/accessibility/ax_tree_serializer_unittest.cc
|
||||
@@ -21,7 +21,8 @@ using testing::UnorderedElementsAre;
|
||||
|
||||
namespace ui {
|
||||
|
||||
-using BasicAXTreeSerializer = AXTreeSerializer<const AXNode*>;
|
||||
+using BasicAXTreeSerializer =
|
||||
+ AXTreeSerializer<const AXNode*, std::vector<const AXNode*>>;
|
||||
|
||||
// The framework for these tests is that each test sets up |treedata0_|
|
||||
// and |treedata1_| and then calls GetTreeSerializer, which creates a
|
||||
@@ -535,7 +536,7 @@ TEST_F(AXTreeSerializerTest, TestPartialSerialization) {
|
||||
// The result should be indistinguishable from the source tree.
|
||||
std::unique_ptr<AXTreeSource<const AXNode*>> dst_tree_source(
|
||||
dst_tree.CreateTreeSource());
|
||||
- AXTreeSerializer<const AXNode*> serializer(dst_tree_source.get());
|
||||
+ BasicAXTreeSerializer serializer(dst_tree_source.get());
|
||||
AXTreeUpdate dst_update;
|
||||
CHECK(serializer.SerializeChanges(dst_tree.root(), &dst_update));
|
||||
ASSERT_EQ(treedata1_.ToString(), dst_update.ToString());
|
||||
diff --git a/ui/accessibility/ax_tree_unittest.cc b/ui/accessibility/ax_tree_unittest.cc
|
||||
index 910c03370446e228692aed4e356d06db3d0ec574..bbd9fa9bafc2003204be9ac05c9044e7fc851b1d 100644
|
||||
--- a/ui/accessibility/ax_tree_unittest.cc
|
||||
+++ b/ui/accessibility/ax_tree_unittest.cc
|
||||
@@ -333,7 +333,8 @@ TEST(AXTreeTest, SerializeSimpleAXTree) {
|
||||
|
||||
std::unique_ptr<AXTreeSource<const AXNode*>> tree_source(
|
||||
src_tree.CreateTreeSource());
|
||||
- AXTreeSerializer<const AXNode*> serializer(tree_source.get());
|
||||
+ AXTreeSerializer<const AXNode*, std::vector<const AXNode*>> serializer(
|
||||
+ tree_source.get());
|
||||
AXTreeUpdate update;
|
||||
serializer.SerializeChanges(src_tree.root(), &update);
|
||||
|
||||
diff --git a/ui/views/accessibility/ax_aura_obj_cache_unittest.cc b/ui/views/accessibility/ax_aura_obj_cache_unittest.cc
|
||||
index 42e505a548edab06ebd31c298791959fe1b55867..68f6b64b1e96a18549d78c88a7adff804c9b2da2 100644
|
||||
--- a/ui/views/accessibility/ax_aura_obj_cache_unittest.cc
|
||||
+++ b/ui/views/accessibility/ax_aura_obj_cache_unittest.cc
|
||||
@@ -216,7 +216,8 @@ TEST_F(AXAuraObjCacheTest, ValidTree) {
|
||||
ui::AXTreeID tree_id = ui::AXTreeID::CreateNewAXTreeID();
|
||||
AXTreeSourceViews tree_source(
|
||||
cache.GetOrCreate(parent_widget->GetNativeWindow()), tree_id, &cache);
|
||||
- ui::AXTreeSerializer<AXAuraObjWrapper*> serializer(&tree_source);
|
||||
+ ui::AXTreeSerializer<AXAuraObjWrapper*, std::vector<AXAuraObjWrapper*>>
|
||||
+ serializer(&tree_source);
|
||||
ui::AXTreeUpdate serialized_tree;
|
||||
serializer.SerializeChanges(tree_source.GetRoot(), &serialized_tree);
|
||||
|
||||
diff --git a/ui/views/accessibility/views_ax_tree_manager.h b/ui/views/accessibility/views_ax_tree_manager.h
|
||||
index 0caff17e0568762d61c7f2fdc6b94c01bc3d9e30..00fe01d00e17b51d7d37d099a8f21549669e69a0 100644
|
||||
--- a/ui/views/accessibility/views_ax_tree_manager.h
|
||||
+++ b/ui/views/accessibility/views_ax_tree_manager.h
|
||||
@@ -93,7 +93,8 @@ class VIEWS_EXPORT ViewsAXTreeManager : public ui::AXTreeManager,
|
||||
void OnWidgetDestroyed(Widget* widget) override;
|
||||
|
||||
private:
|
||||
- using ViewsAXTreeSerializer = ui::AXTreeSerializer<AXAuraObjWrapper*>;
|
||||
+ using ViewsAXTreeSerializer =
|
||||
+ ui::AXTreeSerializer<AXAuraObjWrapper*, std::vector<AXAuraObjWrapper*>>;
|
||||
|
||||
void SerializeTreeUpdates();
|
||||
void UnserializeTreeUpdates(const std::vector<ui::AXTreeUpdate>& updates);
|
||||
Reference in New Issue
Block a user