ThreadableLoader::cancel should be called before loader destruction

third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/loader/ThreadableLoader.h"
 
 #include "core/dom/CrossThreadTask.h"
 #include "core/fetch/MemoryCache.h"
 #include "core/fetch/ResourceLoaderOptions.h"
 #include "core/loader/DocumentThreadableLoader.h"
@@ skipping 122 matching lines @@
     void onSetUp() override
     {
     }
 
     void onServeRequests() override
     {
     }
 
     void onTearDown() override
     {
-        m_loader.reset();
+        if (m_loader) {
+            m_loader->cancel();
+            m_loader.reset();
+        }
     }
 
 private:
     Document& document() { return m_dummyPageHolder->document(); }
 
     std::unique_ptr<DummyPageHolder> m_dummyPageHolder;
     Checkpoint m_checkpoint;
     std::unique_ptr<DocumentThreadableLoader> m_loader;
 };
 
@@ skipping 294 matching lines @@
     EXPECT_CALL(checkpoint(), Call(2)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::cancelLoader));
     EXPECT_CALL(*client(), didFail(Truly(isCancellation)));
     EXPECT_CALL(checkpoint(), Call(3));
 
     startLoader(successURL());
     callCheckpoint(2);
     callCheckpoint(3);
     serveRequests();
 }
 
-TEST_P(ThreadableLoaderTest, ClearAfterStart)

[hiroshige, 2016/07/19 06:56:14]

How about keeping these Clear.* tests by adding cancel() calls to
DocumentThreadableLoaderTestHelper/WorkerThreadableLoaderTestHelper::clearLoader(),
in order to check that we can safely destruct ThreadableLoader synchronously in
the ThreadableLoaderClient callbacks?

[yhirano, 2016/07/19 08:05:04]

Done.

-{
-    InSequence s;
-    EXPECT_CALL(checkpoint(), Call(1));
-    createLoader();
-    callCheckpoint(1);
-
-    EXPECT_CALL(checkpoint(), Call(2)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::clearLoader));
-    EXPECT_CALL(checkpoint(), Call(3));
-
-    startLoader(successURL());
-    callCheckpoint(2);
-    callCheckpoint(3);
-    serveRequests();
-}
-
 TEST_P(ThreadableLoaderTest, CancelInDidReceiveResponse)
 {
     InSequence s;
     EXPECT_CALL(checkpoint(), Call(1));
     createLoader();
     callCheckpoint(1);
 
     EXPECT_CALL(checkpoint(), Call(2));
     EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::cancelLoader));
     EXPECT_CALL(*client(), didFail(Truly(isCancellation)));
 
     startLoader(successURL());
     callCheckpoint(2);
     serveRequests();
 }
 
-TEST_P(ThreadableLoaderTest, ClearInDidReceiveResponse)
-{
-    InSequence s;
-    EXPECT_CALL(checkpoint(), Call(1));
-    createLoader();
-    callCheckpoint(1);
-
-    EXPECT_CALL(checkpoint(), Call(2));
-    EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::clearLoader));
-
-    startLoader(successURL());
-    callCheckpoint(2);
-    serveRequests();
-}
-
 TEST_P(ThreadableLoaderTest, CancelInDidReceiveData)
 {
     InSequence s;
     EXPECT_CALL(checkpoint(), Call(1));
     createLoader();
     callCheckpoint(1);
 
     EXPECT_CALL(checkpoint(), Call(2));
     EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _));
     EXPECT_CALL(*client(), didReceiveData(_, _)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::cancelLoader));
     EXPECT_CALL(*client(), didFail(Truly(isCancellation)));
 
     startLoader(successURL());
     callCheckpoint(2);
     serveRequests();
 }
 
-TEST_P(ThreadableLoaderTest, ClearInDidReceiveData)
-{
-    InSequence s;
-    EXPECT_CALL(checkpoint(), Call(1));
-    createLoader();
-    callCheckpoint(1);
-
-    EXPECT_CALL(checkpoint(), Call(2));
-    EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _));
-    EXPECT_CALL(*client(), didReceiveData(_, _)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::clearLoader));
-
-    startLoader(successURL());
-    callCheckpoint(2);
-    serveRequests();
-}
-
 TEST_P(ThreadableLoaderTest, DidFinishLoading)
 {
     InSequence s;
     EXPECT_CALL(checkpoint(), Call(1));
     createLoader();
     callCheckpoint(1);
 
     EXPECT_CALL(checkpoint(), Call(2));
     EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _));
     EXPECT_CALL(*client(), didReceiveData(StrEq("fox"), 4));
@@ skipping 16 matching lines @@
     EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _));
     EXPECT_CALL(*client(), didReceiveData(_, _));
     EXPECT_CALL(*client(), didReceiveResourceTiming(_));
     EXPECT_CALL(*client(), didFinishLoading(_, _)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::cancelLoader));
 
     startLoader(successURL());
     callCheckpoint(2);
     serveRequests();
 }
 
-TEST_P(ThreadableLoaderTest, ClearInDidFinishLoading)
-{
-    InSequence s;
-    EXPECT_CALL(checkpoint(), Call(1));
-    createLoader();
-    callCheckpoint(1);
-
-    EXPECT_CALL(checkpoint(), Call(2));
-    EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _));
-    EXPECT_CALL(*client(), didReceiveData(_, _));
-    EXPECT_CALL(*client(), didReceiveResourceTiming(_));
-    EXPECT_CALL(*client(), didFinishLoading(_, _)).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::clearLoader));
-
-    startLoader(successURL());
-    callCheckpoint(2);
-    serveRequests();
-}
-
 TEST_P(ThreadableLoaderTest, DidFail)
 {
     InSequence s;
     EXPECT_CALL(checkpoint(), Call(1));
     createLoader();
     callCheckpoint(1);
 
     EXPECT_CALL(checkpoint(), Call(2));
     EXPECT_CALL(*client(), didReceiveResponseMock(_, _, _));
     EXPECT_CALL(*client(), didFail(Truly(isNotCancellation)));
@@ skipping 220 matching lines @@
     EXPECT_CALL(*client(), didFailRedirectCheck()).WillOnce(InvokeWithoutArgs(this, &ThreadableLoaderTest::clearLoader));
 
     startLoader(redirectLoopURL());
     callCheckpoint(2);
     serveRequests();
 }
 
 } // namespace
 
 } // namespace blink