Resource: helper class to prohibit {add,remove}Client() calls in scope

third_party/WebKit/Source/core/fetch/ResourceTest.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 "core/fetch/Resource.h"
 
 #include "core/fetch/MemoryCache.h"
 #include "platform/SharedBuffer.h"
 #include "platform/network/ResourceRequest.h"
 #include "platform/network/ResourceResponse.h"
@@ skipping 33 matching lines @@
 void createTestResourceAndSetCachedMetadata(const ResourceResponse& response) {
   const char testData[] = "test data";
   Resource* resource =
       Resource::create(ResourceRequest(response.url()), Resource::Raw);
   resource->setResponse(response);
   resource->cacheHandler()->setCachedMetadata(
       100, testData, sizeof(testData), CachedMetadataHandler::SendToPlatform);
   return;
 }
 
+class TestProhibitAddRemoveClientResource : public Resource {

[hiroshige, 2016/10/05 08:37:25]

optional: I'd like to avoid creating new test-only subclass of Resource, but
this is still OK.
(Alternatively, we might skip this test and add a new test when we add actual
use of ProhibitAddRemoveClientInScope).

[hiroshige, 2016/10/05 08:37:25]

Add "final".

[Shao-Chuan Lee, 2016/10/05 09:04:53]

Done.

+ public:
+  static TestProhibitAddRemoveClientResource* create() {
+    return new TestProhibitAddRemoveClientResource(
+        ResourceRequest(), Resource::Raw, ResourceLoaderOptions());

[kouhei (in TOK), 2016/10/05 08:29:42]

We can just pass these to Resource ctor in line 70. and remove ctor args from
line 67.

[Shao-Chuan Lee, 2016/10/05 09:04:53]

Done.

+  }
+  void test() {

[kouhei (in TOK), 2016/10/05 08:29:42]

We should have this logic in line 112 func.

[Shao-Chuan Lee, 2016/10/05 09:04:53]

Moving EXPECT_DEATH()s to test function.

+    ProhibitAddRemoveClientInScope prohibitAddRemoveClient(this);
+    EXPECT_DEATH(addClient(nullptr), "!m_isAddRemoveClientProhibited");

[hiroshige, 2016/10/05 08:37:25]

addClient(nullptr) is invalid even outside ProhibitAddRemoveClientInScope, so
please use MockResourceClient or something non-null.

[Shao-Chuan Lee, 2016/10/05 08:51:20]

Since MockResourceClient calls addClient in ctor, I tried

EXPECT_DEATH(new MockResourceClient(this), ...)

but the problem is EXPECT_DEATH seems to fork another process for the check,
then it crashes in Oilpan code (operator new -> enterGCForbiddenScopeIfNeeded()
-> checkThread()).

To workaround this I'm now checking if stderr matches
"!m_isAddRemoveClientProhibited", or we should modify MockResourceClient or have
another minimalist ResourceClient subclass.

[Shao-Chuan Lee, 2016/10/05 09:04:53]

Done.

+    EXPECT_DEATH(removeClient(nullptr), "!m_isAddRemoveClientProhibited");

[hiroshige, 2016/10/05 08:37:26]

ditto.

[Shao-Chuan Lee, 2016/10/05 09:04:53]

Done.

+  }
+
+ private:
+  TestProhibitAddRemoveClientResource(const ResourceRequest& request,

[hiroshige, 2016/10/05 08:37:25]

We can remove these parameters as they receive only fixed values.

[Shao-Chuan Lee, 2016/10/05 09:04:53]

Done.

+                                      Type type,
+                                      const ResourceLoaderOptions& options)
+      : Resource(request, type, options) {}
+};
+
 }  // anonymous namespace
 
 TEST(ResourceTest, SetCachedMetadata_SendsMetadataToPlatform) {
   MockPlatform mock;
   ResourceResponse response(createTestResourceResponse());
   createTestResourceAndSetCachedMetadata(response);
   EXPECT_EQ(1u, mock.cachedURLs().size());
 }
 
 TEST(
@@ skipping 18 matching lines @@
   // Revalidating with a url that differs by only the fragment
   // shouldn't trigger a securiy check.
   url.setFragmentIdentifier("bar");
   resource->setRevalidatingRequest(ResourceRequest(url));
   ResourceResponse revalidatingResponse;
   revalidatingResponse.setURL(url);
   revalidatingResponse.setHTTPStatusCode(304);
   resource->responseReceived(revalidatingResponse, nullptr);
 }
 
+TEST(ResourceTest, ProhibitAddRemoveClientInScope) {
+  TestProhibitAddRemoveClientResource::create()->test();
+}
+
 }  // namespace blink