Minimize direct use of ResourceClient::SetResource() and Resource::AddClient() in tests

Bug: 793028
Change-Id: Icff8fce73e745969e4022329a7cb0374ce43698c
Reviewed-on: https://chromium-review.googlesource.com/818197Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523279}

third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp

@@ -1237,14 +1237,10 @@ TEST(ImageResourceTest, FailedRevalidationSvgToSvg) {
 
 // Tests for pruning.
 
-TEST(ImageResourceTest, AddClientAfterPrune) {
+TEST(ImageResourceTest, Prune) {
   KURL url("http://127.0.0.1:8000/foo");
   ImageResource* image_resource = ImageResource::CreateForTest(url);
 
-  // Adds a ResourceClient but not ImageResourceObserver.
-  Persistent<MockResourceClient> client1 =
-      new MockResourceClient(image_resource);
-
   ReceiveResponse(image_resource, url, "image/jpeg",
                   reinterpret_cast<const char*>(kJpegImage),
                   sizeof(kJpegImage));
@@ -1255,9 +1251,6 @@ TEST(ImageResourceTest, AddClientAfterPrune) {
   EXPECT_EQ(kJpegImageWidth, image_resource->GetContent()->GetImage()->width());
   EXPECT_EQ(kJpegImageHeight,
             image_resource->GetContent()->GetImage()->height());
-  EXPECT_TRUE(client1->NotifyFinishedCalled());
-
-  client1->RemoveAsClient();
 
   EXPECT_FALSE(image_resource->IsAlive());
 
@@ -1265,17 +1258,12 @@ TEST(ImageResourceTest, AddClientAfterPrune) {
 
   EXPECT_TRUE(image_resource->GetContent()->HasImage());
 
-  // Re-adds a ResourceClient but not ImageResourceObserver.
-  Persistent<MockResourceClient> client2 =
-      new MockResourceClient(image_resource);
-
   blink::testing::RunPendingTasks();
   ASSERT_TRUE(image_resource->GetContent()->HasImage());
   EXPECT_FALSE(image_resource->GetContent()->GetImage()->IsNull());
   EXPECT_EQ(kJpegImageWidth, image_resource->GetContent()->GetImage()->width());
   EXPECT_EQ(kJpegImageHeight,
             image_resource->GetContent()->GetImage()->height());
-  EXPECT_TRUE(client2->NotifyFinishedCalled());
 }
 
 TEST(ImageResourceTest, CancelOnDecodeError) {

third_party/WebKit/Source/platform/loader/BUILD.gn

@@ -154,7 +154,6 @@ jumbo_source_set("test_support") {
     "testing/MockFetchContext.h",
     "testing/MockResource.cpp",
     "testing/MockResource.h",
-    "testing/MockResourceClient.cpp",
     "testing/MockResourceClient.h",
     "testing/WebURLLoaderFactoryWithMock.cpp",
     "testing/WebURLLoaderFactoryWithMock.h",

third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp

@@ -92,7 +92,7 @@ class MemoryCacheCorrectnessTest : public ::testing::Test {
   MockResource* FetchMockResource() {
     ResourceRequest resource_request{KURL(kResourceURL)};
     FetchParameters fetch_params(resource_request);
-    return MockResource::Fetch(fetch_params, Fetcher());
+    return MockResource::Fetch(fetch_params, Fetcher(), nullptr);
   }
   ResourceFetcher* Fetcher() const { return fetcher_.Get(); }
   void AdvanceClock(double seconds) { platform_->AdvanceClockSeconds(seconds); }

third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp

@@ -167,7 +167,8 @@ TEST(ResourceTest, RevalidationSucceeded) {
   // Simulate a successful revalidation.
   resource->SetRevalidatingRequest(ResourceRequest("data:text/html,"));
 
-  Persistent<MockResourceClient> client = new MockResourceClient(resource);
+  Persistent<MockResourceClient> client = new MockResourceClient;
+  resource->AddClient(client);
 
   ResourceResponse revalidating_response;
   revalidating_response.SetHTTPStatusCode(304);
@@ -179,7 +180,7 @@ TEST(ResourceTest, RevalidationSucceeded) {
             GetMemoryCache()->ResourceForURL(KURL("data:text/html,")));
   GetMemoryCache()->Remove(resource);
 
-  client->RemoveAsClient();
+  resource->RemoveClient(client);
   EXPECT_FALSE(resource->IsAlive());
   EXPECT_FALSE(client->NotifyFinishedCalled());
 }
@@ -197,7 +198,8 @@ TEST(ResourceTest, RevalidationSucceededForResourceWithoutBody) {
   // Simulate a successful revalidation.
   resource->SetRevalidatingRequest(ResourceRequest("data:text/html,"));
 
-  Persistent<MockResourceClient> client = new MockResourceClient(resource);
+  Persistent<MockResourceClient> client = new MockResourceClient;
+  resource->AddClient(client);
 
   ResourceResponse revalidating_response;
   revalidating_response.SetHTTPStatusCode(304);
@@ -209,7 +211,7 @@ TEST(ResourceTest, RevalidationSucceededForResourceWithoutBody) {
             GetMemoryCache()->ResourceForURL(KURL("data:text/html,")));
   GetMemoryCache()->Remove(resource);
 
-  client->RemoveAsClient();
+  resource->RemoveClient(client);
   EXPECT_FALSE(resource->IsAlive());
   EXPECT_FALSE(client->NotifyFinishedCalled());
 }
@@ -249,7 +251,8 @@ TEST(ResourceTest, RevalidationSucceededUpdateHeaders) {
   EXPECT_EQ("custom value",
             resource->GetResponse().HttpHeaderField("x-custom"));
 
-  Persistent<MockResourceClient> client = new MockResourceClient(resource);
+  Persistent<MockResourceClient> client = new MockResourceClient;
+  resource->AddClient(client);
 
   // Perform a revalidation step.
   ResourceResponse revalidating_response;
@@ -282,8 +285,6 @@ TEST(ResourceTest, RevalidationSucceededUpdateHeaders) {
             resource->GetResponse().HttpHeaderField("proxy-connection"));
   EXPECT_EQ("updated", resource->GetResponse().HttpHeaderField("x-custom"));
 
-  client->RemoveAsClient();
-
   resource->RemoveClient(client);
   EXPECT_FALSE(resource->IsAlive());
   EXPECT_FALSE(client->NotifyFinishedCalled());
@@ -317,7 +318,8 @@ TEST(ResourceTest, RedirectDuringRevalidation) {
   EXPECT_EQ("https://example.com/1",
             resource->LastResourceRequest().Url().GetString());
 
-  Persistent<MockResourceClient> client = new MockResourceClient(resource);
+  Persistent<MockResourceClient> client = new MockResourceClient;
+  resource->AddClient(client);
 
   // The revalidating request is redirected.
   ResourceResponse redirect_response;
@@ -355,7 +357,8 @@ TEST(ResourceTest, RedirectDuringRevalidation) {
   EXPECT_TRUE(client->NotifyFinishedCalled());
 
   // Test the case where a client is added after revalidation is completed.
-  Persistent<MockResourceClient> client2 = new MockResourceClient(resource);
+  Persistent<MockResourceClient> client2 = new MockResourceClient;
+  resource->AddClient(client2);
 
   // Because the client is added asynchronously,
   // |runUntilIdle()| is called to make |client2| to be notified.
@@ -365,8 +368,8 @@ TEST(ResourceTest, RedirectDuringRevalidation) {
 
   GetMemoryCache()->Remove(resource);
 
-  client->RemoveAsClient();
-  client2->RemoveAsClient();
+  resource->RemoveClient(client);
+  resource->RemoveClient(client2);
   EXPECT_FALSE(resource->IsAlive());
 }
 

third_party/WebKit/Source/platform/loader/testing/MockResource.cpp

@@ -26,10 +26,11 @@ class MockResourceFactory final : public NonTextResourceFactory {
 
 // static
 MockResource* MockResource::Fetch(FetchParameters& params,
-                                  ResourceFetcher* fetcher) {
+                                  ResourceFetcher* fetcher,
+                                  ResourceClient* client) {
   params.SetRequestContext(WebURLRequest::kRequestContextSubresource);
-  Resource* resource = fetcher->RequestResource(params, MockResourceFactory());
-  return static_cast<MockResource*>(resource);
+  return static_cast<MockResource*>(
+      fetcher->RequestResource(params, MockResourceFactory(), client));
 }
 
 // static

third_party/WebKit/Source/platform/loader/testing/MockResource.h

@@ -20,7 +20,9 @@ struct ResourceLoaderOptions;
 // class to verify classes that consume Resource sub-classes in a simple way.
 class MockResource final : public Resource {
  public:
-  static MockResource* Fetch(FetchParameters&, ResourceFetcher*);
+  static MockResource* Fetch(FetchParameters&,
+                             ResourceFetcher*,
+                             ResourceClient*);
   static MockResource* Create(const ResourceRequest&);
   MockResource(const ResourceRequest&, const ResourceLoaderOptions&);
 };

third_party/WebKit/Source/platform/loader/testing/MockResourceClient.cpp

-// Copyright 2015 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 "platform/loader/testing/MockResourceClient.h"
-
-namespace blink {
-
-MockResourceClient::MockResourceClient(Resource* resource)
-    : resource_(resource),
-      notify_finished_called_(false),
-      encoded_size_on_notify_finished_(0) {
-  resource_->AddClient(this);
-}
-
-MockResourceClient::~MockResourceClient() {}
-
-void MockResourceClient::NotifyFinished(Resource* resource) {
-  CHECK(!notify_finished_called_);
-  notify_finished_called_ = true;
-  encoded_size_on_notify_finished_ = resource->EncodedSize();
-}
-
-void MockResourceClient::RemoveAsClient() {
-  resource_->RemoveClient(this);
-  resource_ = nullptr;
-}
-
-void MockResourceClient::Dispose() {
-  if (resource_) {
-    resource_->RemoveClient(this);
-    resource_ = nullptr;
-  }
-}
-
-void MockResourceClient::Trace(blink::Visitor* visitor) {
-  visitor->Trace(resource_);
-  ResourceClient::Trace(visitor);
-}
-
-}  // namespace blink

third_party/WebKit/Source/platform/loader/testing/MockResourceClient.h

@@ -32,37 +32,28 @@
 #define MockResourceClient_h
 
 #include "platform/heap/Handle.h"
-#include "platform/loader/fetch/Resource.h"
 #include "platform/loader/fetch/ResourceClient.h"
 
 namespace blink {
 
 class MockResourceClient : public GarbageCollectedFinalized<MockResourceClient>,
                            public ResourceClient {
-  USING_PRE_FINALIZER(MockResourceClient, Dispose);
   USING_GARBAGE_COLLECTED_MIXIN(MockResourceClient);
 
  public:
-  explicit MockResourceClient(Resource*);
-  ~MockResourceClient() override;
+  MockResourceClient() {}
+  ~MockResourceClient() override {}
 
-  void NotifyFinished(Resource*) override;
-  String DebugName() const override { return "MockResourceClient"; }
-  virtual bool NotifyFinishedCalled() const { return notify_finished_called_; }
-
-  size_t EncodedSizeOnNotifyFinished() const {
-    return encoded_size_on_notify_finished_;
+  void NotifyFinished(Resource*) override {
+    CHECK(!notify_finished_called_);
+    notify_finished_called_ = true;
   }
-
-  virtual void RemoveAsClient();
-  virtual void Dispose();
-
-  void Trace(blink::Visitor*) override;
+  String DebugName() const override { return "MockResourceClient"; }
+  bool NotifyFinishedCalled() const { return notify_finished_called_; }
+  void RemoveAsClient() { ClearResource(); }
 
  protected:
-  Member<Resource> resource_;
-  bool notify_finished_called_;
-  size_t encoded_size_on_notify_finished_;
+  bool notify_finished_called_ = false;
 };
 
 }  // namespace blink