Commit 29c12a26 authored by hiroshige's avatar hiroshige Committed by Commit bot

Fix ImageResourceContent::Create()'s GetContentStatus()

Follow-up of a regression bug caused by [1]:
Previously, ImageResourceContent::Create() returned a content with
kCached, while after [1] it returns a content with kNotStarted.
This causes behavior changes if Create() is called with non-null
argument.

[1] https://codereview.chromium.org/2746343002/

This CL fixes this issue by renaming Create() with a non-null argument
to ImageResourceContent::CreateLoaded() that sets the status to kCached.
The callsites of CreateLoaded() were affected by the bug, i.e.
a callsite in HTMLImageElement and others in tests.

Create() with the nullptr argument is renamed to CreateNotStarted(),
which is only called from ImageResource and is not affected by the bug.

BUG=667641, 382170, 690480

Review-Url: https://codereview.chromium.org/2869733004
Cr-Commit-Position: refs/heads/master@{#471113}
parent 5f60ca09
......@@ -106,8 +106,8 @@ TEST_F(ImageBitmapTest, ImageResourceConsistency) {
const ImageBitmapOptions default_options;
HTMLImageElement* image_element =
HTMLImageElement::Create(*Document::Create());
ImageResourceContent* image =
ImageResourceContent::Create(StaticBitmapImage::Create(image_).Get());
ImageResourceContent* image = ImageResourceContent::CreateLoaded(
StaticBitmapImage::Create(image_).Get());
image_element->SetImageResource(image);
Optional<IntRect> crop_rect =
......@@ -149,7 +149,8 @@ TEST_F(ImageBitmapTest, ImageResourceConsistency) {
TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
HTMLImageElement* image = HTMLImageElement::Create(*Document::Create());
ImageResourceContent* original_image_resource =
ImageResourceContent::Create(StaticBitmapImage::Create(image_).Get());
ImageResourceContent::CreateLoaded(
StaticBitmapImage::Create(image_).Get());
image->SetImageResource(original_image_resource);
const ImageBitmapOptions default_options;
......@@ -163,8 +164,8 @@ TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
ASSERT_NE(image_bitmap->BitmapImage()->ImageForCurrentFrame(),
original_image_resource->GetImage()->ImageForCurrentFrame());
ImageResourceContent* new_image_resource =
ImageResourceContent::Create(StaticBitmapImage::Create(image2_).Get());
ImageResourceContent* new_image_resource = ImageResourceContent::CreateLoaded(
StaticBitmapImage::Create(image2_).Get());
image->SetImageResource(new_image_resource);
{
......@@ -241,7 +242,8 @@ TEST_F(ImageBitmapTest, ImageBitmapColorSpaceConversionHTMLImageElement) {
image->width() * raster_image_info.bytesPerPixel(), 5, 5);
ImageResourceContent* original_image_resource =
ImageResourceContent::Create(StaticBitmapImage::Create(image).Get());
ImageResourceContent::CreateLoaded(
StaticBitmapImage::Create(image).Get());
image_element->SetImageResource(original_image_resource);
Optional<IntRect> crop_rect = IntRect(0, 0, image->width(), image->height());
......@@ -352,7 +354,8 @@ TEST_F(ImageBitmapTest, ImageBitmapColorSpaceConversionImageBitmap) {
image->width() * raster_image_info.bytesPerPixel(), 5, 5);
ImageResourceContent* source_image_resource =
ImageResourceContent::Create(StaticBitmapImage::Create(image).Get());
ImageResourceContent::CreateLoaded(
StaticBitmapImage::Create(image).Get());
image_element->SetImageResource(source_image_resource);
Optional<IntRect> crop_rect = IntRect(0, 0, image->width(), image->height());
......
......@@ -372,8 +372,9 @@ void HTMLImageElement::AttachLayoutTree(const AttachContext& context) {
blink::DeviceScaleFactorDeprecated(layout_image->GetFrame());
std::pair<Image*, float> broken_image_and_image_scale_factor =
ImageResourceContent::BrokenImage(device_scale_factor);
ImageResourceContent* new_image_resource = ImageResourceContent::Create(
broken_image_and_image_scale_factor.first);
ImageResourceContent* new_image_resource =
ImageResourceContent::CreateLoaded(
broken_image_and_image_scale_factor.first);
layout_image->ImageResource()->SetImageResource(new_image_resource);
}
if (layout_image_resource->HasImage())
......
......@@ -134,7 +134,8 @@ class ImageResource::ImageResourceFactory : public ResourceFactory {
Resource* Create(const ResourceRequest& request,
const ResourceLoaderOptions& options,
const String&) const override {
return new ImageResource(request, options, ImageResourceContent::Create(),
return new ImageResource(request, options,
ImageResourceContent::CreateNotStarted(),
fetch_params_->GetPlaceholderImageRequestType() ==
FetchParameters::kAllowPlaceholder);
}
......@@ -183,7 +184,7 @@ bool ImageResource::CanReuse(const FetchParameters& params) const {
ImageResource* ImageResource::Create(const ResourceRequest& request) {
return new ImageResource(request, ResourceLoaderOptions(),
ImageResourceContent::Create(), false);
ImageResourceContent::CreateNotStarted(), false);
}
ImageResource::ImageResource(const ResourceRequest& resource_request,
......
......@@ -74,6 +74,14 @@ ImageResourceContent::ImageResourceContent(PassRefPtr<blink::Image> image)
info_ = &null_info;
}
ImageResourceContent* ImageResourceContent::CreateLoaded(
PassRefPtr<blink::Image> image) {
DCHECK(image);
ImageResourceContent* content = new ImageResourceContent(std::move(image));
content->content_status_ = ResourceStatus::kCached;
return content;
}
ImageResourceContent* ImageResourceContent::Fetch(FetchParameters& params,
ResourceFetcher* fetcher) {
// TODO(hiroshige): Remove direct references to ImageResource by making
......
......@@ -46,10 +46,15 @@ class CORE_EXPORT ImageResourceContent final
USING_GARBAGE_COLLECTED_MIXIN(ImageResourceContent);
public:
static ImageResourceContent* Create(
PassRefPtr<blink::Image> image = nullptr) {
return new ImageResourceContent(std::move(image));
// Used for loading.
// Returned content will be associated immediately later with ImageResource.
static ImageResourceContent* CreateNotStarted() {
return new ImageResourceContent(nullptr);
}
// Creates ImageResourceContent from an already loaded image.
static ImageResourceContent* CreateLoaded(PassRefPtr<blink::Image>);
static ImageResourceContent* Fetch(FetchParameters&, ResourceFetcher*);
// Returns the nullImage() if the image is not available yet.
......
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