Verify the order of RawResourceClient callbacks in DocumentThreadableLoader

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

Index: third_party/WebKit/Source/core/fetch/RawResource.h
diff --git a/third_party/WebKit/Source/core/fetch/RawResource.h b/third_party/WebKit/Source/core/fetch/RawResource.h
index 84b702679dbf358c60ba93339aa20ed0e6a83092..69da9671776fd08d574b6fd6270ad58d95a2545c 100644
--- a/third_party/WebKit/Source/core/fetch/RawResource.h
+++ b/third_party/WebKit/Source/core/fetch/RawResource.h
@@ -111,6 +111,23 @@ public:
     static bool isExpectedType(ResourceClient* client) { return client->getResourceClientType() == RawResourceType; }
     ResourceClientType getResourceClientType() const final { return RawResourceType; }
 
+    // The order of the callbacks is as follows:
+    // [Case 1] A successful load:
+    // 0+  redirectReceived() and/or dataSent()
+    // 1   responseReceived()
+    // 0-1 setSerializedCachedMetadata()
+    // 0+  dataReceived() or dataDownloaded(), but never both
+    // 1   notifyFinished() with errorOccurred() = false
+    // [Case 2] When redirect is blocked:
+    // 0+  redirectReceived() and/or dataSent()
+    // 1   redirectBlocked()
+    // 1   notifyFinished() with errorOccurred() = true
+    // [Case 3] Other failures:
+    //     notifyFinished() with errorOccurred() = true is called at any time
+    //     (unless notifyFinished() is already called).
+    // In all cases:
+    //     No callbacks are made after notifyFinished() or
+    //     removeClient() is called.
     virtual void dataSent(Resource*, unsigned long long /* bytesSent */, unsigned long long /* totalBytesToBeSent */) { }
     virtual void responseReceived(Resource*, const ResourceResponse&, PassOwnPtr<WebDataConsumerHandle>) { }
     virtual void setSerializedCachedMetadata(Resource*, const char*, size_t) { }
@@ -124,6 +141,43 @@ private:
     WeakPtrFactory<RawResourceClient> m_weakFactory;
 };
 
+// Checks the sequence of callbacks of RawResourceClient.
+// This can be used only when a RawResourceClient is added as a client to
+// at most one RawResource.
+class CORE_EXPORT RawResourceClientStateChecker {

[yhirano, 2016/06/03 07:25:13]

+final

[hiroshige, 2016/08/03 11:50:34]

Done.

+public:
+    RawResourceClientStateChecker();
+    ~RawResourceClientStateChecker();
+
+    // Call before addClient()/removeClient() is called.
+    void willAddClient();

[yhirano, 2016/06/03 07:25:13]

I'm a bit concerned about the case when DocumentThreadableLoader is a subclass
of ResourceOwner<RawResource>. In that case ResourceOwner::setResource adds the
client to a Resource without notifying DocumentThreadableLoader (Currently no
one outside of the class calls setResource but we will not be able to forbid
it).

Does that mean DocumentThreadableLoader will never be able to be a subclass of
ResourceOwner?

[hiroshige, 2016/06/03 09:58:38]

ResourceOwner::setResource() is |protected|.
Doesn't it prevent setResource() calls from outside DocumentThreadableLoader?

[yhirano, 2016/06/07 05:18:25]

I didn't notice that, thank you.
But ResourceOwner may call removeClient(self) in the prefinalizer step, so can
you remove the assertion from the destructor?

[hiroshige, 2016/08/03 11:50:34]

Done.

+    void willRemoveClient();
+
+    // Call RawResourceClientStateChecker::f() at the beginning of
+    // RawResourceClient::f().
+    void redirectReceived();
+    void redirectBlocked();
+    void dataSent();
+    void responseReceived();
+    void setSerializedCachedMetadata();
+    void dataReceived();
+    void dataDownloaded();
+    void notifyFinished(Resource*);
+
+private:
+    enum State {
+        NotAddedAsClient,
+        Started,
+        RedirectBlocked,
+        ResponseReceived,
+        SetSerializedCachedMetadata,
+        DataReceived,
+        DataDownloaded,
+        NotifyFinished
+    };
+    State m_state;
+};
+
 } // namespace blink
 
 #endif // RawResource_h