[ImageResource 4] Split ImageResource into Resource and Image parts

third_party/WebKit/Source/core/fetch/ImageResource.h

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller <mueller@kde.org>
     Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
     Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
     version 2 of the License, or (at your option) any later version.
 
     This library is distributed in the hope that it will be useful,
     but WITHOUT ANY WARRANTY; without even the implied warranty of
     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
     Library General Public License for more details.
 
     You should have received a copy of the GNU Library General Public License
     along with this library; see the file COPYING.LIB.  If not, write to
     the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
     Boston, MA 02110-1301, USA.
 */
 
 #ifndef ImageResource_h
 #define ImageResource_h
 
 #include "core/CoreExport.h"
 #include "core/fetch/MultipartImageResourceParser.h"
 #include "core/fetch/Resource.h"

[yhirano, 2016/12/05 09:02:03]

+platform/Timer.h
+platform/heap/Handle.h

[hiroshige, 2016/12/06 09:32:50]

Done.

-#include "platform/geometry/IntRect.h"
-#include "platform/geometry/IntSizeHash.h"
-#include "platform/geometry/LayoutSize.h"
-#include "platform/graphics/Image.h"
-#include "platform/graphics/ImageObserver.h"
-#include "platform/graphics/ImageOrientation.h"
-#include "wtf/HashMap.h"
 #include <memory>
 
 namespace blink {
 
 class FetchRequest;
-class ImageResourceObserver;
-class MemoryCache;
 class ResourceClient;
 class ResourceFetcher;
 class SecurityOrigin;
+class ImageResourceContent;

[yhirano, 2016/12/05 09:02:03]

alphabetical order?

[hiroshige, 2016/12/06 09:32:51]

Done.

 
-// ImageResource class represents an image type resource.
+// ImageResource is the resource-related part of image loading.

[kouhei (in TOK), 2016/12/05 07:13:06]

ImageResource implements blink::Resource interface for loading images.

[hiroshige, 2016/12/05 09:11:19]

Done.

+// Image-related things (blink::Image and ImageResourceObserver) are handled by
+// ImageResourceContent.
+// Most users should use ImageResourceContent instead of ImageResource.
+// https://docs.google.com/document/d/1O-fB83mrE0B_V8gzXNqHgmRLCvstTB4MMi3RnVLr8bE/edit?usp=sharing
 //
-// As for the lifetimes of m_image and m_data, see this document:
+// As for the lifetimes of ImageResourceContent::m_image and m_data, see this
+// document:
 // https://docs.google.com/document/d/1v0yTAZ6wkqX2U_M6BNIGUJpM1s0TIw1VsqpxoL7aciY/edit?usp=sharing
 class CORE_EXPORT ImageResource final
     : public Resource,
-      public ImageObserver,
       public MultipartImageResourceParser::Client {
-  friend class MemoryCache;
   USING_GARBAGE_COLLECTED_MIXIN(ImageResource);
 
  public:
   using ClientType = ResourceClient;
 
+  // Use ImageResourceContent::fetch() unless ImageResource is required.

[kouhei (in TOK), 2016/12/05 07:13:06]

Can we make this private && friend ImageResourceContent?

[hiroshige, 2016/12/05 09:11:19]

Currently No, because this is used from e.g. DocumentLoader.
I'd like to clean up related code so that this can be private in the near
future, but not in this CL.

[kouhei (in TOK), 2016/12/05 09:22:05]

Optional: Please consider adding TODO(hiroshige) to make this private.

[hiroshige, 2016/12/06 09:32:50]

Done.

   static ImageResource* fetch(FetchRequest&, ResourceFetcher*);
 
-  static ImageResource* create(blink::Image* image) {
-    return new ImageResource(image, ResourceLoaderOptions());
-  }
-
-  static ImageResource* create(const ResourceRequest& request) {
-    return new ImageResource(request, ResourceLoaderOptions(), false);
-  }
+  // TODO(hiroshige): Make create() test-only by refactoring ImageDocument.
+  static ImageResource* create(const ResourceRequest&);
 
   ~ImageResource() override;
 
-  blink::Image*
-  getImage();  // Returns the nullImage() if the image is not available yet.
-  bool hasImage() const { return m_image.get(); }
-
-  static std::pair<blink::Image*, float> brokenImage(
-      float deviceScaleFactor);  // Returns an image and the image's resolution
-                                 // scale factor.
-  bool willPaintBrokenImage() const;
-
-  bool usesImageContainerSize() const;
-  bool imageHasRelativeSize() const;
-  // The device pixel ratio we got from the server for this image, or 1.0.
-  float devicePixelRatioHeaderValue() const {
-    return m_devicePixelRatioHeaderValue;
-  }
-  bool hasDevicePixelRatioHeaderValue() const {
-    return m_hasDevicePixelRatioHeaderValue;
-  }
-
-  enum SizeType {
-    // Report the intrinsic size.
-    IntrinsicSize,
-
-    // Report the intrinsic size corrected to account for image density.
-    IntrinsicCorrectedToDPR,
-  };
-
-  // This method takes a zoom multiplier that can be used to increase the
-  // natural size of the image by the zoom.
-  LayoutSize imageSize(
-      RespectImageOrientationEnum shouldRespectImageOrientation,
-      float multiplier,
-      SizeType = IntrinsicSize);
-
-  bool isAccessAllowed(SecurityOrigin*);
-
-  void updateImageAnimationPolicy();
+  ImageResourceContent* getContent() const;
 
   enum class ReloadCachePolicy {
     UseExistingPolicy = 0,  // Don't modify the request's cache policy.
     BypassCache,  // Modify the request so that the reload bypasses the cache.
   };
 
   // If this ImageResource has the Lo-Fi response headers or is a placeholder,
   // reload the full original image with the Lo-Fi state set to off and
   // optionally bypassing the cache.
   void reloadIfLoFiOrPlaceholder(
       ResourceFetcher*,
       ReloadCachePolicy = ReloadCachePolicy::BypassCache);
 
   void didAddClient(ResourceClient*) override;
 
-  void addObserver(ImageResourceObserver*);
-  void removeObserver(ImageResourceObserver*);
-
   ResourcePriority priorityFromObservers() override;
 
   void allClientsAndObserversRemoved() override;
 
   PassRefPtr<const SharedBuffer> resourceBuffer() const override;
   void appendData(const char*, size_t) override;
   void error(const ResourceError&) override;
   void responseReceived(const ResourceResponse&,
                         std::unique_ptr<WebDataConsumerHandle>) override;
   void finish(double finishTime = 0.0) override;
 
   // For compatibility, images keep loading even if there are HTTP errors.
   bool shouldIgnoreHTTPStatusCodeErrors() const override { return true; }
 
   bool isImage() const override { return true; }
 
-  // ImageObserver
-  void decodedSizeChangedTo(const blink::Image*, size_t newSize) override;
-
-  bool shouldPauseAnimation(const blink::Image*) override;
-  void animationAdvanced(const blink::Image*) override;
-  void changedInRect(const blink::Image*, const IntRect&) override;
-
   // MultipartImageResourceParser::Client
   void onePartInMultipartReceived(const ResourceResponse&) final;
   void multipartDataReceived(const char*, size_t) final;
 
   // Used by tests.
   bool isPlaceholder() const { return m_isPlaceholder; }
 
   bool shouldReloadBrokenPlaceholder() const {
     return m_isPlaceholder && willPaintBrokenImage();
   }
 
-  void setNotRefetchableDataFromDiskCache() {
-    m_isRefetchableDataFromDiskCache = false;
-  }
-
   DECLARE_VIRTUAL_TRACE();
 
  private:
-  explicit ImageResource(blink::Image*, const ResourceLoaderOptions&);
+  // Only for ImageResourceInfoImpl.

[yhirano, 2016/12/05 09:02:03]

Can you move these functions declarations below the constructor?

[hiroshige, 2016/12/06 09:32:50]

Done.

+  void decodeError(bool allDataReceived);
+  bool isAccessAllowed(SecurityOrigin*,
+                       bool doesCurrentFrameHasSingleSecurityOrigin) const;
 
   enum class MultipartParsingState : uint8_t {
     WaitingForFirstPart,
     ParsingFirstPart,
     FinishedParsingFirstPart,
   };
 
+  class ImageResourceInfoImpl;
   class ImageResourceFactory;
 
   ImageResource(const ResourceRequest&,
                 const ResourceLoaderOptions&,
+                ImageResourceContent*,
                 bool isPlaceholder);
 
-  bool hasClientsOrObservers() const override {
-    return Resource::hasClientsOrObservers() || !m_observers.isEmpty() ||
-           !m_finishedObservers.isEmpty();
-  }
-  void clear();
+  bool hasClientsOrObservers() const override;
 
-  void createImage();
-  void updateImage(bool allDataReceived);
   void updateImageAndClearBuffer();
-  void clearImage();
-  enum NotifyFinishOption { ShouldNotifyFinish, DoNotNotifyFinish };
-  // If not null, changeRect is the changed part of the image.
-  void notifyObservers(NotifyFinishOption, const IntRect* changeRect = nullptr);
-
-  void ensureImage();
 
   void checkNotify() override;
-  void notifyObserversInternal();
-  void markObserverFinished(ImageResourceObserver*);
-
-  void doResetAnimation();
 
   void destroyDecodedDataIfPossible() override;
   void destroyDecodedDataForFailedRevalidation() override;
 
   void flushImageIfNeeded(TimerBase*);
 
+  bool willPaintBrokenImage() const;
+
+  Member<ImageResourceContent> m_content;

[yhirano, 2016/12/05 09:02:03]

[optional] const Member<ImageResourceContent> m_content;

[hiroshige, 2016/12/06 09:32:51]

I'll keep |m_content| non-const because it will be non-const soon after this CL.

+
   float m_devicePixelRatioHeaderValue;

[yhirano, 2016/12/05 09:02:03]

Is it desirable to move this member variable to ImageResourceContent and update
the value in ImageResource::responseReceived?

[hiroshige, 2016/12/06 09:32:50]

Moving |m_devicePixelRatioHeaderValue| to ImageResourceContent and make
ImageResource to update it is desirable, because it is more natural (the members
looks image-related) and removes the corresponding interfaces in
ImageResourceInfo.

However, I'd like to update it in updateImage(), not in a new
ImageResourceConent method called in responseReceived(), because I'd like to
make updateImage() the single point of state change of ImageResourceContent
caused by ImageResource (we don't have updateImage() call in
responseReceived()), to simplify timing constraints between ImageResource and
its content.
Older Patch Set (e.g. Patch Set 7) does this: updateImage() has
|devicePixelRatioHeaderValue| argument, and ImageResourceContent updates its
|m_devicePixelRatioHeaderValue| in updateImage().
I felt it is over-complex at this time, but I'll re-apply that change if it
looks good. (Added a TODO anyway in the latest PatchSet).

 
   Member<MultipartImageResourceParser> m_multipartParser;
-  RefPtr<blink::Image> m_image;
   MultipartParsingState m_multipartParsingState =
       MultipartParsingState::WaitingForFirstPart;
   bool m_hasDevicePixelRatioHeaderValue;

[yhirano, 2016/12/05 09:02:03]

ditto

[hiroshige, 2016/12/06 09:32:50]

ditto.

-  HashCountedSet<ImageResourceObserver*> m_observers;
-  HashCountedSet<ImageResourceObserver*> m_finishedObservers;
 
   // Indicates if the ImageResource is currently scheduling a reload, e.g.
   // because reloadIfLoFi() was called.
   bool m_isSchedulingReload;
 
   // Indicates if this ImageResource is either attempting to load a placeholder
   // image, or is a (possibly broken) placeholder image.
   bool m_isPlaceholder;
 
   Timer<ImageResource> m_flushTimer;
   double m_lastFlushTime = 0.;
-  Image::SizeAvailability m_sizeAvailable = Image::SizeUnavailable;
-
-  // Indicates if this resource's encoded image data can be purged and refetched
-  // from disk cache to save memory usage. See crbug/664437.
-  bool m_isRefetchableDataFromDiskCache;
 };
 
 DEFINE_RESOURCE_TYPE_CASTS(Image);
 
 }  // namespace blink
 
 #endif