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

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

Issue 2527353002: Phase II Step 3: Reload LoFi/placeholder images via new ImageResource
Patch Set: reloadLoFiImages test Created 4 years 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
Index: third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
diff --git a/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp b/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
index beafff04f6fcd1482b844c2e1f6fbbda4fc7d554..19ebc6c25799f5c3b3203d2a92e318bd911897ec 100644
--- a/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
@@ -35,6 +35,7 @@
#include "platform/SharedBuffer.h"
#include "platform/tracing/TraceEvent.h"
#include "public/platform/Platform.h"
+#include "public/platform/WebCachePolicy.h"
#include "wtf/CurrentTime.h"
#include "wtf/StdLibExtras.h"
#include <memory>
@@ -64,9 +65,6 @@ class ImageResource::ImageResourceInfoImpl final
private:
const KURL& url() const override { return m_resource->url(); }
- bool isSchedulingReload() const override {
- return m_resource->m_isSchedulingReload;
- }
bool hasDevicePixelRatioHeaderValue() const override {
return m_resource->m_hasDevicePixelRatioHeaderValue;
}
@@ -83,10 +81,6 @@ class ImageResource::ImageResourceInfoImpl final
bool isCacheValidator() const override {
return m_resource->isCacheValidator();
}
- bool schedulingReloadOrShouldReloadBrokenPlaceholder() const override {
- return m_resource->m_isSchedulingReload ||
- m_resource->shouldReloadBrokenPlaceholder();
- }
bool isAccessAllowed(
SecurityOrigin* securityOrigin,
DoesCurrentFrameHaveSingleSecurityOrigin
@@ -100,6 +94,7 @@ class ImageResource::ImageResourceInfoImpl final
const ResourceError& resourceError() const override {
return m_resource->resourceError();
}
+ const ImageResource* resourceForTest() const override { return m_resource; }
void decodeError(bool allDataReceived) override {
m_resource->decodeError(allDataReceived);
@@ -121,6 +116,11 @@ class ImageResource::ImageResourceInfoImpl final
WebURLRequest::RequestContextImage,
initiatorName);
}
+ bool reloadIfLoFiOrPlaceholderIfNeeded(
+ ResourceFetcher* fetcherForReload) override {
+ return m_resource->reloadIfLoFiOrPlaceholderImage(fetcherForReload,
+ kReloadIfNeeded);
+ }
const Member<ImageResource> m_resource;
};
@@ -129,20 +129,26 @@ class ImageResource::ImageResourceFactory : public ResourceFactory {
STACK_ALLOCATED();
public:
- ImageResourceFactory(const FetchRequest& fetchRequest)
- : ResourceFactory(Resource::Image), m_fetchRequest(&fetchRequest) {}
+ ImageResourceFactory(const FetchRequest& fetchRequest,
+ ImageResourceContent* content)
+ : ResourceFactory(Resource::Image),
+ m_fetchRequest(&fetchRequest),
+ m_content(content) {}
Resource* create(const ResourceRequest& request,
const ResourceLoaderOptions& options,
const String&) const override {
- return new ImageResource(request, options, ImageResourceContent::create(),
- m_fetchRequest->placeholderImageRequestType() ==
- FetchRequest::AllowPlaceholder);
+ return new ImageResource(
+ request, options,
+ m_content ? m_content.get() : ImageResourceContent::create(),
+ m_fetchRequest->placeholderImageRequestType() ==
+ FetchRequest::AllowPlaceholder);
}
private:
// Weak, unowned pointer. Must outlive |this|.
const FetchRequest* m_fetchRequest;
+ Persistent<ImageResourceContent> m_content;
};
ImageResource* ImageResource::fetch(FetchRequest& request,
@@ -165,16 +171,18 @@ ImageResource* ImageResource::fetch(FetchRequest& request,
return nullptr;
}
- ImageResource* resource = toImageResource(
- fetcher->requestResource(request, ImageResourceFactory(request)));
+ ImageResource* resource = toImageResource(fetcher->requestResource(
+ request, ImageResourceFactory(request, nullptr)));
if (resource &&
request.placeholderImageRequestType() != FetchRequest::AllowPlaceholder &&
Nate Chapin 2016/12/28 00:14:57 This also seems like a case where we could have Im
hiroshige 2016/12/28 01:04:21 Sounds good, I created a separate CL: https://code
resource->m_isPlaceholder) {
// If the image is a placeholder, but this fetch doesn't allow a
// placeholder, then load the original image. Note that the cache is not
// bypassed here - it should be fine to use a cached copy if possible.
- resource->reloadIfLoFiOrPlaceholderImage(
+ ImageResource* reloadingResource = resource->reloadIfLoFiOrPlaceholderImage(
fetcher, kReloadAlwaysWithExistingCachePolicy);
+ if (reloadingResource)
+ resource = reloadingResource;
}
return resource;
}
@@ -192,7 +200,6 @@ ImageResource::ImageResource(const ResourceRequest& resourceRequest,
m_content(content),
m_devicePixelRatioHeaderValue(1.0),
m_hasDevicePixelRatioHeaderValue(false),
- m_isSchedulingReload(false),
m_isPlaceholder(isPlaceholder),
m_flushTimer(this, &ImageResource::flushImageIfNeeded) {
DCHECK(getContent());
@@ -211,28 +218,14 @@ DEFINE_TRACE(ImageResource) {
MultipartImageResourceParser::Client::trace(visitor);
}
-void ImageResource::checkNotify() {
- // Don't notify clients of completion if this ImageResource is
- // about to be reloaded.
- if (m_isSchedulingReload || shouldReloadBrokenPlaceholder())
- return;
-
- Resource::checkNotify();
-}
-
bool ImageResource::hasClientsOrObservers() const {
- return Resource::hasClientsOrObservers() || getContent()->hasObservers();
+ return Resource::hasClientsOrObservers() ||
+ (getContent() && getContent()->hasObservers());
}
void ImageResource::didAddClient(ResourceClient* client) {
DCHECK((m_multipartParser && isLoading()) || !data() ||
- getContent()->hasImage());
-
- // Don't notify observers and clients of completion if this ImageResource is
- // about to be reloaded.
- if (m_isSchedulingReload || shouldReloadBrokenPlaceholder())
- return;
-
+ (getContent() && getContent()->hasImage()));
Resource::didAddClient(client);
}
@@ -244,6 +237,8 @@ void ImageResource::destroyDecodedDataForFailedRevalidation() {
}
void ImageResource::destroyDecodedDataIfPossible() {
+ if (!getContent())
+ return;
getContent()->destroyDecodedData();
if (getContent()->hasImage() && !isPreloaded() &&
getContent()->isRefetchableDataFromDiskCache()) {
@@ -253,16 +248,18 @@ void ImageResource::destroyDecodedDataIfPossible() {
}
void ImageResource::allClientsAndObserversRemoved() {
- CHECK(!getContent()->hasImage() || !errorOccurred());
+ CHECK(!getContent() || !getContent()->hasImage() || !errorOccurred());
// If possible, delay the resetting until back at the event loop. Doing so
// after a conservative GC prevents resetAnimation() from upsetting ongoing
// animation updates (crbug.com/613709)
- if (!ThreadHeap::willObjectBeLazilySwept(this)) {
- Platform::current()->currentThread()->getWebTaskRunner()->postTask(
- BLINK_FROM_HERE, WTF::bind(&ImageResourceContent::doResetAnimation,
- wrapWeakPersistent(getContent())));
- } else {
- getContent()->doResetAnimation();
+ if (getContent()) {
+ if (!ThreadHeap::willObjectBeLazilySwept(this)) {
+ Platform::current()->currentThread()->getWebTaskRunner()->postTask(
+ BLINK_FROM_HERE, WTF::bind(&ImageResourceContent::doResetAnimation,
+ wrapWeakPersistent(getContent())));
+ } else {
+ getContent()->doResetAnimation();
+ }
}
if (m_multipartParser)
m_multipartParser->cancel();
@@ -272,6 +269,8 @@ void ImageResource::allClientsAndObserversRemoved() {
PassRefPtr<const SharedBuffer> ImageResource::resourceBuffer() const {
if (data())
return data();
+ if (!getContent())
+ return nullptr;
return getContent()->resourceBuffer();
}
@@ -282,6 +281,9 @@ void ImageResource::appendData(const char* data, size_t length) {
} else {
Resource::appendData(data, length);
+ if (!getContent())
+ return;
+
// Update the image immediately if needed.
if (getContent()->shouldUpdateImageImmediately()) {
updateImage(this->data(), ImageResourceContent::UpdateImage, false);
@@ -341,13 +343,15 @@ void ImageResource::updateImageAndClearBuffer() {
clearData();
}
-void ImageResource::finish(double loadFinishTime) {
+void ImageResource::finish(double loadFinishTime,
+ ResourceFetcher* fetcherForReload) {
if (m_multipartParser) {
m_multipartParser->finish();
if (data())
updateImageAndClearBuffer();
} else {
- updateImage(data(), ImageResourceContent::UpdateImage, true);
+ updateImage(data(), ImageResourceContent::UpdateImage, true,
+ fetcherForReload);
// As encoded image data can be created from m_image (see
// ImageResource::resourceBuffer(), we don't have to keep m_data. Let's
// clear this. As for the lifetimes of m_image and m_data, see this
@@ -355,17 +359,19 @@ void ImageResource::finish(double loadFinishTime) {
// https://docs.google.com/document/d/1v0yTAZ6wkqX2U_M6BNIGUJpM1s0TIw1VsqpxoL7aciY/edit?usp=sharing
clearData();
}
- Resource::finish(loadFinishTime);
+ Resource::finish(loadFinishTime, fetcherForReload);
}
-void ImageResource::error(const ResourceError& error) {
+void ImageResource::error(const ResourceError& error,
+ ResourceFetcher* fetcherForReload) {
if (m_multipartParser)
m_multipartParser->cancel();
// TODO(hiroshige): Move setEncodedSize() call to Resource::error() if it
// is really needed, or remove it otherwise.
setEncodedSize(0);
- Resource::error(error);
- updateImage(nullptr, ImageResourceContent::ClearImageOnly, true);
+ Resource::error(error, fetcherForReload);
+ updateImage(nullptr, ImageResourceContent::ClearImageOnly, true,
+ fetcherForReload);
}
void ImageResource::responseReceived(
@@ -392,6 +398,17 @@ void ImageResource::responseReceived(
}
}
+void ImageResource::detachContent() {
+ bool hasObservers = getContent() && getContent()->hasObservers();
+
+ m_content = nullptr;
+ clearData();
+ memoryCache()->remove(this);
+
+ if (hasObservers)
+ didRemoveClientOrObserver();
+}
+
static bool isLoFiImage(const ImageResource& resource) {
if (resource.resourceRequest().loFiState() != WebURLRequest::LoFiOn)
return false;
@@ -401,48 +418,44 @@ static bool isLoFiImage(const ImageResource& resource) {
.contains("empty-image");
}
-void ImageResource::reloadIfLoFiOrPlaceholderImage(
+ImageResource* ImageResource::reloadIfLoFiOrPlaceholderImage(
ResourceFetcher* fetcher,
ReloadLoFiOrPlaceholderPolicy policy) {
+ if (!fetcher)
+ return nullptr;
if (policy == kReloadIfNeeded && !shouldReloadBrokenPlaceholder())
- return;
+ return nullptr;
if (!m_isPlaceholder && !isLoFiImage(*this))
- return;
-
- // Prevent clients and observers from being notified of completion while the
- // reload is being scheduled, so that e.g. canceling an existing load in
- // progress doesn't cause clients and observers to be notified of completion
- // prematurely.
- DCHECK(!m_isSchedulingReload);
- m_isSchedulingReload = true;
-
- if (policy != kReloadAlwaysWithExistingCachePolicy)
- setCachePolicyBypassingCache();
- setLoFiStateOff();
-
- if (m_isPlaceholder) {
- m_isPlaceholder = false;
- clearRangeRequestHeader();
- }
-
- if (isLoading()) {
- loader()->cancel();
- // Canceling the loader causes error() to be called, which in turn calls
- // clear() and notifyObservers(), so there's no need to call these again
- // here.
- } else {
- clearData();
- setEncodedSize(0);
- updateImage(nullptr, ImageResourceContent::ClearImageOnly, false);
- }
+ return nullptr;
- setStatus(NotStarted);
+ ImageResourceContent* content = getContent();
+ if (!content)
+ return nullptr;
- DCHECK(m_isSchedulingReload);
- m_isSchedulingReload = false;
+ // Creates request/options for new ImageResource for reloading.
+ ResourceRequest reloadingRequest = resourceRequest();
+ ResourceLoaderOptions reloadingOptions = options();
- fetcher->startLoad(this);
+ if (policy != kReloadAlwaysWithExistingCachePolicy)
+ reloadingRequest.setCachePolicy(WebCachePolicy::BypassingCache);
+ reloadingRequest.setLoFiState(WebURLRequest::LoFiOff);
+ if (m_isPlaceholder)
+ reloadingRequest.clearHTTPHeaderField("range");
+
+ detachContent();
+ // Do not touch |this| after this point.
+
+ FetchRequest fetchRequest(
+ reloadingRequest, reloadingOptions.initiatorInfo.name, reloadingOptions);
+ DCHECK_EQ(FetchRequest::DisallowPlaceholder,
+ fetchRequest.placeholderImageRequestType());
+ fetchRequest.setEnforceNewResource();
+
+ ImageResource* reloadingResource = toImageResource(fetcher->requestResource(
+ fetchRequest, ImageResourceFactory(fetchRequest, content)));
+ DCHECK(!reloadingResource || content == reloadingResource->getContent());
+ return reloadingResource;
}
void ImageResource::onePartInMultipartReceived(
@@ -504,19 +517,24 @@ const ImageResourceContent* ImageResource::getContent() const {
}
ResourcePriority ImageResource::priorityFromObservers() {
+ if (!getContent())
+ return ResourcePriority();
return getContent()->priorityFromObservers();
}
void ImageResource::updateImage(
PassRefPtr<SharedBuffer> sharedBuffer,
ImageResourceContent::UpdateImageOption updateImageOption,
- bool allDataReceived) {
+ bool allDataReceived,
+ ResourceFetcher* fetcherForReload) {
+ if (!getContent())
+ return;
if (!m_isUpdateImageCalled &&
updateImageOption == ImageResourceContent::UpdateImage)
updateImageOption = ImageResourceContent::ClearAndUpdateImage;
m_isUpdateImageCalled = true;
getContent()->updateImage(std::move(sharedBuffer), updateImageOption,
- allDataReceived);
+ allDataReceived, fetcherForReload);
}
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698