Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(558)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp

Issue 2527353002: Phase II Step 3: Reload LoFi/placeholder images via new ImageResource
Patch Set: Use startLoad() again to avoid re-applying modifications to ResourceRequest on reload Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« third_party/WebKit/Source/core/loader/resource/ImageResource.cpp ('K') | « third_party/WebKit/Source/core/loader/resource/ImageResourceInfo.h ('k') | third_party/WebKit/Source/platform/loader/fetch/Resource.h » ('j') | third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
diff --git a/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp b/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
index 962a51741d3c46f07a10a764fc513fce9bb5ec95..7b1dc30199cfdc35d4c7c127b25bfdc122c7984f 100644
--- a/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
+++ b/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
@@ -175,16 +175,22 @@ void receiveResponse(ImageResource* imageResource,
imageResource->finish();
}
-void testThatReloadIsStartedThenServeReload(const KURL& testURL,
- ImageResource* imageResource,
- ImageResourceContent* content,
- MockImageResourceObserver* observer,
- WebCachePolicy policyForReload) {
+void testThatReloadIsStartedThenServeReload(
+ const KURL& testURL,
+ ImageResource* originalImageResource,
+ ImageResource* imageResource,
+ ImageResourceContent* content,
+ MockImageResourceObserver* observer,
+ WebCachePolicy policyForReload) {
const char* data = reinterpret_cast<const char*>(kJpegImage2);
constexpr size_t dataLength = sizeof(kJpegImage2);
constexpr int imageWidth = 50;
constexpr int imageHeight = 50;
+ // Checks that |originalImageResource| is detached.
+ EXPECT_NE(originalImageResource, imageResource);
+ EXPECT_FALSE(originalImageResource->getContent());
+
// Checks that |imageResource| and |content| are ready for non-placeholder
// reloading.
EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
@@ -195,6 +201,7 @@ void testThatReloadIsStartedThenServeReload(const KURL& testURL,
EXPECT_EQ(policyForReload, imageResource->resourceRequest().getCachePolicy());
EXPECT_EQ(content, imageResource->getContent());
EXPECT_FALSE(content->hasImage());
+ EXPECT_EQ(imageResource, memoryCache()->resourceForURL(testURL));
// Checks |observer| before reloading.
const int originalImageChangedCount = observer->imageChangedCount();
@@ -509,6 +516,7 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderAfterFinished) {
ResourceRequest request = ResourceRequest(testURL);
request.setPreviewsState(WebURLRequest::ServerLoFiOn);
ImageResource* imageResource = ImageResource::create(request);
+ ImageResourceContent* content = imageResource->getContent();
imageResource->setStatus(ResourceStatus::Pending);
std::unique_ptr<MockImageResourceObserver> observer =
@@ -526,25 +534,27 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderAfterFinished) {
sizeof(kJpegImage));
imageResource->finish();
EXPECT_FALSE(imageResource->errorOccurred());
- ASSERT_TRUE(imageResource->getContent()->hasImage());
- EXPECT_FALSE(imageResource->getContent()->getImage()->isNull());
+ ASSERT_TRUE(content->hasImage());
+ EXPECT_FALSE(content->getImage()->isNull());
EXPECT_EQ(2, observer->imageChangedCount());
EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnLastImageChanged());
// The observer should have been notified that the image load completed.
EXPECT_TRUE(observer->imageNotifyFinishedCalled());
EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnImageNotifyFinished());
- EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
- EXPECT_EQ(kJpegImageWidth, imageResource->getContent()->getImage()->width());
- EXPECT_EQ(kJpegImageHeight,
- imageResource->getContent()->getImage()->height());
+ EXPECT_TRUE(content->getImage()->isBitmapImage());
+ EXPECT_EQ(kJpegImageWidth, content->getImage()->width());
+ EXPECT_EQ(kJpegImageHeight, content->getImage()->height());
// Call reloadIfLoFiOrPlaceholderImage() after the image has finished loading.
- imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
- Resource::kReloadAlways);
-
+ ImageResource* reloadingResource =
+ imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
+ Resource::kReloadAlways);
EXPECT_EQ(3, observer->imageChangedCount());
+ ASSERT_TRUE(reloadingResource);
+ EXPECT_EQ(reloadingResource, fetcher->cachedResource(testURL));
+
testThatReloadIsStartedThenServeReload(
- testURL, imageResource, imageResource->getContent(), observer.get(),
+ testURL, imageResource, reloadingResource, content, observer.get(),
WebCachePolicy::BypassingCache);
}
@@ -583,9 +593,13 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderViaResourceFetcher) {
EXPECT_EQ(3, observer->imageChangedCount());
- testThatReloadIsStartedThenServeReload(testURL, imageResource, content,
- observer.get(),
- WebCachePolicy::BypassingCache);
+ ImageResource* reloadingResource =
+ toImageResource(fetcher->cachedResource(testURL));
+ ASSERT_TRUE(reloadingResource);
+
+ testThatReloadIsStartedThenServeReload(
+ testURL, imageResource, reloadingResource, content, observer.get(),
+ WebCachePolicy::BypassingCache);
memoryCache()->remove(imageResource);
}
@@ -600,8 +614,9 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderDuringFetch) {
ResourceFetcher* fetcher = createFetcher();
ImageResource* imageResource = ImageResource::fetch(fetchRequest, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
std::unique_ptr<MockImageResourceObserver> observer =
- MockImageResourceObserver::create(imageResource->getContent());
+ MockImageResourceObserver::create(content);
// Send the image response.
ResourceResponse initialResourceResponse(testURL, "image/jpeg",
@@ -614,20 +629,22 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderDuringFetch) {
reinterpret_cast<const char*>(kJpegImage), sizeof(kJpegImage));
EXPECT_FALSE(imageResource->errorOccurred());
- ASSERT_TRUE(imageResource->getContent()->hasImage());
- EXPECT_FALSE(imageResource->getContent()->getImage()->isNull());
+ ASSERT_TRUE(content->hasImage());
+ EXPECT_FALSE(content->getImage()->isNull());
EXPECT_EQ(1, observer->imageChangedCount());
EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnLastImageChanged());
EXPECT_FALSE(observer->imageNotifyFinishedCalled());
- EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
- EXPECT_EQ(kJpegImageWidth, imageResource->getContent()->getImage()->width());
- EXPECT_EQ(kJpegImageHeight,
- imageResource->getContent()->getImage()->height());
+ EXPECT_TRUE(content->getImage()->isBitmapImage());
+ EXPECT_EQ(kJpegImageWidth, content->getImage()->width());
+ EXPECT_EQ(kJpegImageHeight, content->getImage()->height());
// Call reloadIfLoFiOrPlaceholderImage() while the image is still loading.
- imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
- Resource::kReloadAlways);
+ ImageResource* reloadingResource =
+ imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
+ Resource::kReloadAlways);
+ ASSERT_TRUE(reloadingResource);
+ EXPECT_EQ(reloadingResource, fetcher->cachedResource(testURL));
EXPECT_EQ(2, observer->imageChangedCount());
EXPECT_EQ(0, observer->imageWidthOnLastImageChanged());
// The observer should not have been notified of completion yet, since the
@@ -635,8 +652,14 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderDuringFetch) {
EXPECT_FALSE(observer->imageNotifyFinishedCalled());
testThatReloadIsStartedThenServeReload(
- testURL, imageResource, imageResource->getContent(), observer.get(),
+ testURL, imageResource, reloadingResource, content, observer.get(),
WebCachePolicy::BypassingCache);
+
+ // Trigger the cancel timer, ensure the load for the first ImageResource
+ // was cancelled because reloading is started using another ImageResource.
+ EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
+ blink::testing::runPendingTasks();
+ EXPECT_EQ(ResourceStatus::LoadError, imageResource->getStatus());
}
TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderForPlaceholder) {
@@ -647,19 +670,24 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderForPlaceholder) {
FetchRequest request(testURL, FetchInitiatorInfo());
request.setAllowImagePlaceholder();
ImageResource* imageResource = ImageResource::fetch(request, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
+ ASSERT_TRUE(content);
EXPECT_EQ(FetchRequest::AllowPlaceholder,
request.placeholderImageRequestType());
std::unique_ptr<MockImageResourceObserver> observer =
- MockImageResourceObserver::create(imageResource->getContent());
+ MockImageResourceObserver::create(content);
testThatIsPlaceholderRequestAndServeResponse(testURL, imageResource,
observer.get());
- imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
- Resource::kReloadAlways);
+ ImageResource* reloadingResource =
+ imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
+ Resource::kReloadAlways);
+ ASSERT_TRUE(reloadingResource);
+ EXPECT_EQ(reloadingResource, fetcher->cachedResource(testURL));
testThatReloadIsStartedThenServeReload(
- testURL, imageResource, imageResource->getContent(), observer.get(),
+ testURL, imageResource, reloadingResource, content, observer.get(),
WebCachePolicy::BypassingCache);
}
@@ -1116,7 +1144,9 @@ TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessful) {
FetchRequest request(testURL, FetchInitiatorInfo());
request.setAllowImagePlaceholder();
- ImageResource* imageResource = ImageResource::fetch(request, createFetcher());
+ ResourceFetcher* fetcher = createFetcher();
+ ImageResource* imageResource = ImageResource::fetch(request, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
EXPECT_EQ(FetchRequest::AllowPlaceholder,
request.placeholderImageRequestType());
EXPECT_EQ("bytes=0-2047",
@@ -1140,13 +1170,17 @@ TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessful) {
imageResource->loader()->didReceiveData(kBadData, sizeof(kBadData));
+ ImageResource* reloadingResource =
+ toImageResource(fetcher->cachedResource(testURL));
+ ASSERT_TRUE(reloadingResource);
+
// The dimensions could not be extracted, so the full original image should be
// loading.
EXPECT_FALSE(observer->imageNotifyFinishedCalled());
- EXPECT_EQ(2, observer->imageChangedCount());
+ EXPECT_EQ(1, observer->imageChangedCount());
testThatReloadIsStartedThenServeReload(
- testURL, imageResource, imageResource->getContent(), observer.get(),
+ testURL, imageResource, reloadingResource, content, observer.get(),
WebCachePolicy::BypassingCache);
}
@@ -1156,7 +1190,9 @@ TEST(ImageResourceTest, FetchAllowPlaceholderPartialContentWithoutDimensions) {
FetchRequest request(testURL, FetchInitiatorInfo());
request.setAllowImagePlaceholder();
- ImageResource* imageResource = ImageResource::fetch(request, createFetcher());
+ ResourceFetcher* fetcher = createFetcher();
+ ImageResource* imageResource = ImageResource::fetch(request, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
EXPECT_EQ(FetchRequest::AllowPlaceholder,
request.placeholderImageRequestType());
EXPECT_EQ("bytes=0-2047",
@@ -1189,10 +1225,14 @@ TEST(ImageResourceTest, FetchAllowPlaceholderPartialContentWithoutDimensions) {
kJpegImageSubrangeWithoutDimensionsLength);
EXPECT_FALSE(observer->imageNotifyFinishedCalled());
- EXPECT_EQ(2, observer->imageChangedCount());
+ EXPECT_EQ(1, observer->imageChangedCount());
+
+ ImageResource* reloadingResource =
+ toImageResource(fetcher->cachedResource(testURL));
+ ASSERT_TRUE(reloadingResource);
testThatReloadIsStartedThenServeReload(
- testURL, imageResource, imageResource->getContent(), observer.get(),
+ testURL, imageResource, reloadingResource, content, observer.get(),
WebCachePolicy::BypassingCache);
}
@@ -1206,8 +1246,9 @@ TEST(ImageResourceTest, FetchAllowPlaceholderThenDisallowPlaceholder) {
placeholderRequest.setAllowImagePlaceholder();
ImageResource* imageResource =
ImageResource::fetch(placeholderRequest, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
std::unique_ptr<MockImageResourceObserver> observer =
- MockImageResourceObserver::create(imageResource->getContent());
+ MockImageResourceObserver::create(content);
FetchRequest nonPlaceholderRequest(testURL, FetchInitiatorInfo());
ImageResource* imageResource2 =
@@ -1256,8 +1297,9 @@ TEST(ImageResourceTest,
placeholderRequest.setAllowImagePlaceholder();
ImageResource* imageResource =
ImageResource::fetch(placeholderRequest, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
std::unique_ptr<MockImageResourceObserver> observer =
- MockImageResourceObserver::create(imageResource->getContent());
+ MockImageResourceObserver::create(content);
testThatIsPlaceholderRequestAndServeResponse(testURL, imageResource,
observer.get());
@@ -1378,7 +1420,10 @@ TEST(ImageResourceTest,
WrappedResourceResponse(response));
imageResource->loader()->didReceiveData(kBadImageData, test.dataSize);
+ // Reloading does not occur and loading finishes as DecodeError.
EXPECT_EQ(ResourceStatus::DecodeError, imageResource->getStatus());
+ EXPECT_FALSE(imageResource->isLoading());
+ EXPECT_TRUE(imageResource->getContent());
EXPECT_FALSE(imageResource->shouldShowPlaceholder());
}
}
@@ -1392,15 +1437,16 @@ TEST(ImageResourceTest,
FetchRequest request(testURL, FetchInitiatorInfo());
request.setAllowImagePlaceholder();
- ImageResource* imageResource =
- ImageResource::fetch(request, createFetcher());
+ ResourceFetcher* fetcher = createFetcher();
+ ImageResource* imageResource = ImageResource::fetch(request, fetcher);
+ ImageResourceContent* content = imageResource->getContent();
EXPECT_EQ(FetchRequest::AllowPlaceholder,
request.placeholderImageRequestType());
EXPECT_EQ("bytes=0-2047",
imageResource->resourceRequest().httpHeaderField("range"));
EXPECT_TRUE(imageResource->shouldShowPlaceholder());
std::unique_ptr<MockImageResourceObserver> observer =
- MockImageResourceObserver::create(imageResource->getContent());
+ MockImageResourceObserver::create(content);
static const char kBadImageData[] = "bad image data";
@@ -1414,10 +1460,14 @@ TEST(ImageResourceTest,
EXPECT_FALSE(observer->imageNotifyFinishedCalled());
+ ImageResource* reloadingResource =
+ toImageResource(fetcher->cachedResource(testURL));
+ ASSERT_TRUE(reloadingResource);
+
// The dimensions could not be extracted, and the response code was a 4xx
// error, so the full original image should be loading.
testThatReloadIsStartedThenServeReload(
- testURL, imageResource, imageResource->getContent(), observer.get(),
+ testURL, imageResource, reloadingResource, content, observer.get(),
WebCachePolicy::BypassingCache);
}
}
« third_party/WebKit/Source/core/loader/resource/ImageResource.cpp ('K') | « third_party/WebKit/Source/core/loader/resource/ImageResourceInfo.h ('k') | third_party/WebKit/Source/platform/loader/fetch/Resource.h » ('j') | third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp » ('J')

Powered by Google App Engine
This is Rietveld 408576698