Commit 90ba3c0c authored by Alex Rudenko's avatar Alex Rudenko Committed by Commit Bot

DevTools: create unsupported_image_mime_types lazily to save memory

This CL fixes the memory usage regression (crbug/1144454) by
creating the instances for unsupported_image_mime_types only when
DevTools is enabled and some image types are disabled.

Measurement: memory:chrome:all_processes:reported_by_chrome:blink_gc:effective_size
Pinpoint job: https://pinpoint-dot-chromeperf.appspot.com/job/13be0a31520000
Fixed: 1144454
Bug: 1130556

Change-Id: Iac9472238ca3e05c393c0b6fbd62596e5148a5b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515403Reviewed-by: default avatarPeter Marshall <petermarshall@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824338}
parent 3e95b49a
......@@ -447,9 +447,9 @@ void InspectorEmulationAgent::FrameStartedLoading(LocalFrame*) {
}
AtomicString InspectorEmulationAgent::OverrideAcceptImageHeader(
const HashSet<String>& disabled_image_types) {
const HashSet<String>* disabled_image_types) {
String header(ImageAcceptHeader());
for (String type : disabled_image_types) {
for (String type : *disabled_image_types) {
// The header string is expected to be like
// `image/avif,image/webp,image/apng,image/svg+xml,image/*,*/*;q=0.8` and is
// expected to be always ending with `image/*,*/*;q=xxx`, therefore, to
......@@ -475,12 +475,17 @@ void InspectorEmulationAgent::PrepareRequest(DocumentLoader* loader,
if (resource_type != ResourceType::kImage || disabled_image_types_.IsEmpty())
return;
if (!options.unsupported_image_mime_types) {
options.unsupported_image_mime_types =
base::MakeRefCounted<base::RefCountedData<HashSet<String>>>();
}
for (String type : disabled_image_types_.Keys()) {
options.unsupported_image_mime_types.insert(type);
options.unsupported_image_mime_types->data.insert(type);
}
request.SetHTTPAccept(
OverrideAcceptImageHeader(options.unsupported_image_mime_types));
OverrideAcceptImageHeader(&options.unsupported_image_mime_types->data));
// Bypassing caching to prevent the use of the previously loaded and cached
// images.
request.SetCacheMode(mojom::blink::FetchCacheMode::kBypassCache);
......
......@@ -107,7 +107,7 @@ class CORE_EXPORT InspectorEmulationAgent final
void Trace(Visitor*) const override;
static AtomicString OverrideAcceptImageHeader(const HashSet<String>&);
static AtomicString OverrideAcceptImageHeader(const HashSet<String>*);
private:
WebViewImpl* GetWebViewImpl();
......
......@@ -33,16 +33,16 @@ TEST_F(InspectorEmulationAgentTest, ModifiesAcceptHeader) {
#endif
HashSet<String> disabled_types;
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(disabled_types),
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(&disabled_types),
expected_default);
disabled_types.insert("image/webp");
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(disabled_types),
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(&disabled_types),
expected_no_webp);
disabled_types.insert("image/avif");
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(disabled_types),
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(&disabled_types),
expected_no_webp_and_avif);
disabled_types.erase("image/webp");
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(disabled_types),
EXPECT_EQ(InspectorEmulationAgent::OverrideAcceptImageHeader(&disabled_types),
expected_no_avif);
}
......
......@@ -135,8 +135,10 @@ class ImageResource::ImageResourceInfoImpl final
return resource_->GetResourceRequest().IsAdResource();
}
const HashSet<String>& GetUnsupportedImageMimeTypes() const override {
return resource_->Options().unsupported_image_mime_types;
const HashSet<String>* GetUnsupportedImageMimeTypes() const override {
if (!resource_->Options().unsupported_image_mime_types)
return nullptr;
return &resource_->Options().unsupported_image_mime_types->data;
}
const Member<ImageResource> resource_;
......
......@@ -66,13 +66,12 @@ class NullImageResourceInfo final
bool IsAdResource() const override { return false; }
const HashSet<String>& GetUnsupportedImageMimeTypes() const override {
return unsupported_image_mime_types_;
const HashSet<String>* GetUnsupportedImageMimeTypes() const override {
return nullptr;
}
const KURL url_;
const ResourceResponse response_;
const HashSet<String> unsupported_image_mime_types_;
};
} // namespace
......@@ -423,12 +422,12 @@ ImageResourceContent::UpdateImageResult ImageResourceContent::UpdateImage(
if (size_available_ == Image::kSizeUnavailable && !all_data_received)
return UpdateImageResult::kNoDecodeError;
if (image_) {
if (image_ && info_->GetUnsupportedImageMimeTypes()) {
// Filename extension is set by the image decoder based on the actual
// image content.
String file_extension = image_->FilenameExtension();
if (info_->GetUnsupportedImageMimeTypes().Contains("image/" +
file_extension)) {
if (info_->GetUnsupportedImageMimeTypes()->Contains(
String("image/" + file_extension))) {
return UpdateImageResult::kShouldDecodeError;
}
}
......
......@@ -58,7 +58,7 @@ class CORE_EXPORT ImageResourceInfo : public GarbageCollectedMixin {
virtual bool IsAdResource() const = 0;
virtual const HashSet<String>& GetUnsupportedImageMimeTypes() const = 0;
virtual const HashSet<String>* GetUnsupportedImageMimeTypes() const = 0;
void Trace(Visitor* visitor) const override {}
};
......
......@@ -114,7 +114,8 @@ struct PLATFORM_EXPORT ResourceLoaderOptions {
url_loader_factory;
// Used by DevTools to emulate unsupported image types. See crbug.com/1130556.
HashSet<String> unsupported_image_mime_types;
scoped_refptr<base::RefCountedData<HashSet<String>>>
unsupported_image_mime_types;
};
} // namespace blink
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment