ASSERTION FAILED: m_origin || m_type == CachedResource::MainResource

https://bugs.webkit.org/show_bug.cgi?id=162472
<rdar://problem/28431522>

Patch by Youenn Fablet <[email protected]> on 2016-09-26
Reviewed by Darin Adler.

No change of behavior.

Introducing a new CachedResource constructor for already loaded resources.
Sharing code with the other constructor in the init method.
The main difference with this new constructor is that the resource has no specified origin.
The response tainting remains Basic.

Making some additional code clean-up.

* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::CachedImage): Making use of the new constructor.
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::CachedResource):
(WebCore::CachedResource::finishRequestInitialization):
* loader/cache/CachedResource.h:
(WebCore::CachedResource::type):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@206370 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Source/WebCore/ChangeLog

@@ -1,3 +1,28 @@
+2016-09-26  Youenn Fablet  <[email protected]>
+
+        ASSERTION FAILED: m_origin || m_type == CachedResource::MainResource
+        https://bugs.webkit.org/show_bug.cgi?id=162472
+        <rdar://problem/28431522>
+
+        Reviewed by Darin Adler.
+
+        No change of behavior.
+
+        Introducing a new CachedResource constructor for already loaded resources.
+        Sharing code with the other constructor in the init method.
+        The main difference with this new constructor is that the resource has no specified origin.
+        The response tainting remains Basic.
+
+        Making some additional code clean-up.
+
+        * loader/cache/CachedImage.cpp:
+        (WebCore::CachedImage::CachedImage): Making use of the new constructor.
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource):
+        (WebCore::CachedResource::finishRequestInitialization):
+        * loader/cache/CachedResource.h:
+        (WebCore::CachedResource::type):
+
 2016-09-26  Olivier Blin  <[email protected]>
 
         [GStreamer] Support flipY for GPU-to-GPU copy of video textures to WebGL

Source/WebCore/loader/cache/CachedImage.cpp

@@ -66,23 +66,19 @@ CachedImage::CachedImage(CachedResourceRequest&& request, SessionID sessionID)
 }
 
 CachedImage::CachedImage(Image* image, SessionID sessionID)
-    : CachedResource(CachedResourceRequest(ResourceRequest()), ImageResource, sessionID)
+    : CachedResource(URL(), ImageResource, sessionID)
     , m_image(image)
     , m_isManuallyCached(false)
     , m_shouldPaintBrokenImage(true)
 {
-    setStatus(Cached);
-    setLoading(false);
 }
 
 CachedImage::CachedImage(const URL& url, Image* image, SessionID sessionID)
-    : CachedResource(CachedResourceRequest(ResourceRequest(url)), ImageResource, sessionID)
+    : CachedResource(url, ImageResource, sessionID)
     , m_image(image)
     , m_isManuallyCached(false)
     , m_shouldPaintBrokenImage(true)
 {
-    setStatus(Cached);
-    setLoading(false);
 }
 
 CachedImage::CachedImage(const URL& url, Image* image, CachedImage::CacheBehaviorType type, SessionID sessionID)

Source/WebCore/loader/cache/CachedResource.cpp

@@ -122,40 +122,37 @@ CachedResource::CachedResource(CachedResourceRequest&& request, Type type, Sessi
     , m_loadPriority(defaultPriorityForResourceType(type))
     , m_responseTimestamp(std::chrono::system_clock::now())
     , m_origin(request.releaseOrigin())
-    , m_lastDecodedAccessTime(0)
-    , m_loadFinishTime(0)
-    , m_encodedSize(0)
-    , m_decodedSize(0)
-    , m_accessCount(0)
-    , m_handleCount(0)
-    , m_preloadCount(0)
-    , m_preloadResult(PreloadNotReferenced)
-    , m_requestedFromNetworkingLayer(false)
-    , m_inCache(false)
-    , m_loading(false)
-    , m_switchingClientsToRevalidatedResource(false)
     , m_type(type)
-    , m_status(Pending)
-#ifndef NDEBUG
-    , m_deleted(false)
-    , m_lruIndex(0)
-#endif
-    , m_owningCachedResourceLoader(nullptr)
-    , m_resourceToRevalidate(nullptr)
-    , m_proxyResource(nullptr)
 {
-    ASSERT(m_type == unsigned(type)); // m_type is a bitfield, so this tests careless updates of the enum.
     ASSERT(sessionID.isValid());
-#ifndef NDEBUG
-    cachedResourceLeakCounter.increment();
-#endif
+    finishRequestInitialization();
+
     // FIXME: We should have a better way of checking for Navigation loads, maybe FetchMode::Options::Navigate.
     ASSERT(m_origin || m_type == CachedResource::MainResource);
 
     if (m_options.mode != FetchOptions::Mode::SameOrigin && m_origin
         && !(m_resourceRequest.url().protocolIsData() && m_options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set)
         && !m_origin->canRequest(m_resourceRequest.url()))
         setCrossOrigin();
+}
+
+CachedResource::CachedResource(const URL& url, Type type, SessionID sessionID)
+    : m_resourceRequest(url)
+    , m_decodedDataDeletionTimer(*this, &CachedResource::destroyDecodedData, deadDecodedDataDeletionIntervalForResourceType(type))
+    , m_sessionID(sessionID)
+    , m_responseTimestamp(std::chrono::system_clock::now())
+    , m_type(type)
+    , m_status(Cached)
+{
+    ASSERT(sessionID.isValid());
+    finishRequestInitialization();
+}
+
+void CachedResource::finishRequestInitialization()
+{
+#ifndef NDEBUG
+    cachedResourceLeakCounter.increment();
+#endif
 
     if (!m_resourceRequest.url().hasFragmentIdentifier())
         return;

Source/WebCore/loader/cache/CachedResource.h

@@ -119,8 +119,8 @@ class CachedResource {
     const String& cachePartition() const { return m_resourceRequest.cachePartition(); }
 #endif
     SessionID sessionID() const { return m_sessionID; }
-    Type type() const { return static_cast<Type>(m_type); }
-    
+    Type type() const { return m_type; }
+
     ResourceLoadPriority loadPriority() const { return m_loadPriority; }
     void setLoadPriority(const Optional<ResourceLoadPriority>&);
 
@@ -278,6 +278,9 @@ class CachedResource {
     static ResourceLoadPriority defaultPriorityForResourceType(Type);
 
 protected:
+    // CachedResource constructor that may be used when the CachedResource can already be filled with response data.
+    CachedResource(const URL&, Type, SessionID);
+
     void setEncodedSize(unsigned);
     void setDecodedSize(unsigned);
     void didAccessDecodedData(double timeStamp);
@@ -298,6 +301,8 @@ class CachedResource {
 private:
     class Callback;
 
+    void finishRequestInitialization();
+
     bool addClientToSet(CachedResourceClient*);
 
     void decodedDataDeletionTimerFired();
@@ -321,42 +326,42 @@ class CachedResource {
     ResourceError m_error;
     RefPtr<SecurityOrigin> m_origin;
 
-    double m_lastDecodedAccessTime; // Used as a "thrash guard" in the cache
-    double m_loadFinishTime;
+    double m_lastDecodedAccessTime { 0 }; // Used as a "thrash guard" in the cache
+    double m_loadFinishTime { 0 };
 
-    unsigned m_encodedSize;
-    unsigned m_decodedSize;
-    unsigned m_accessCount;
-    unsigned m_handleCount;
-    unsigned m_preloadCount;
+    unsigned m_encodedSize { 0 };
+    unsigned m_decodedSize { 0 };
+    unsigned m_accessCount { 0 };
+    unsigned m_handleCount { 0 };
+    unsigned m_preloadCount { 0 };
 
-    unsigned m_preloadResult : 2; // PreloadResult
+    unsigned m_preloadResult { PreloadNotReferenced };
 
-    bool m_requestedFromNetworkingLayer : 1;
+    bool m_requestedFromNetworkingLayer { false };
 
-    bool m_inCache : 1;
-    bool m_loading : 1;
+    bool m_inCache { false };
+    bool m_loading { false };
 
-    bool m_switchingClientsToRevalidatedResource : 1;
+    bool m_switchingClientsToRevalidatedResource { false };
 
-    unsigned m_type : 4; // Type
-    unsigned m_status : 3; // Status
+    Type m_type; // Type
+    unsigned m_status { Pending }; // Status
 
 #ifndef NDEBUG
-    bool m_deleted;
-    unsigned m_lruIndex;
+    bool m_deleted { false };
+    unsigned m_lruIndex { 0 };
 #endif
 
-    CachedResourceLoader* m_owningCachedResourceLoader; // only non-null for resources that are not in the cache
+    CachedResourceLoader* m_owningCachedResourceLoader { nullptr }; // only non-null for resources that are not in the cache
 
     // If this field is non-null we are using the resource as a proxy for checking whether an existing resource is still up to date
     // using HTTP If-Modified-Since/If-None-Match headers. If the response is 304 all clients of this resource are moved
     // to to be clients of m_resourceToRevalidate and the resource is deleted. If not, the field is zeroed and this
     // resources becomes normal resource load.
-    CachedResource* m_resourceToRevalidate;
+    CachedResource* m_resourceToRevalidate { nullptr };
 
     // If this field is non-null, the resource has a proxy for checking whether it is still up to date (see m_resourceToRevalidate).
-    CachedResource* m_proxyResource;
+    CachedResource* m_proxyResource { nullptr };
 
     // These handles will need to be updated to point to the m_resourceToRevalidate in case we get 304 response.
     HashSet<CachedResourceHandleBase*> m_handlesToRevalidate;