TestingPlatformSupport: register Platform instance correctly

third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h

Index: third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
diff --git a/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h b/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
index 159c0febae33681c850788625010da4a68eee369..d9f49afecd2abe01dbf9d43f156d20e369e7e929 100644
--- a/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
+++ b/third_party/WebKit/Source/platform/testing/TestingPlatformSupport.h
@@ -37,6 +37,7 @@
 #include "public/platform/WebCompositorSupport.h"
 #include "public/platform/WebScheduler.h"
 #include "public/platform/WebThread.h"
+#include "wtf/Assertions.h"
 #include "wtf/Vector.h"
 #include <memory>
 
@@ -107,6 +108,16 @@ class TestingPlatformMockScheduler : public WebScheduler {
   void removePendingNavigation(WebScheduler::NavigatingFrameType) override {}
 };
 
+// A base class to override Platform methods for testing.  You can override
+// compositorSupport() method via a TestingPlatformSupport::Config parameter.
+// Other virtual Platform methods that are overriden in this class will call
+// the original Platform methods that can be obtained by Platform::current()
+// in the consturctor.
+// Or you may to consider to implement your own inheritance of this class to
+// override arbitorary virtual methods.
+// You should use ScopedTestingPlatformSupport to hold any instance of this
+// class and inheritances in order to setup and shutdown Platform instance
+// correctly.  See also comments on ScopedTestingPlatformSupport.
 class TestingPlatformSupport : public Platform {
   WTF_MAKE_NONCOPYABLE(TestingPlatformSupport);
 
@@ -142,6 +153,8 @@ class TestingPlatformSupport : public Platform {
   std::unique_ptr<TestingInterfaceProvider> m_interfaceProvider;
 };
 
+// This class adds mocked scheduler support to TestingPlatformSupport.  See also
+// comments on TestingPlatformSupport to use this class correctly.
 class TestingPlatformSupportWithMockScheduler : public TestingPlatformSupport {
   WTF_MAKE_NONCOPYABLE(TestingPlatformSupportWithMockScheduler);
 
@@ -188,6 +201,74 @@ class TestingPlatformSupportWithMockScheduler : public TestingPlatformSupport {
   std::unique_ptr<WebThread> m_thread;
 };
 
+// A template class to hold an instance of TestingPlatformSupport class and
+// inheritances.  This class also sets the Platform inheritance to be available
+// through Platform::current(), and safely reset it to the previous instance
+// on destructing the instance.  This instance could be nested.
+//
+// Usage:
+//
+// #include "wtf/PtrUtil.h"
+//
+// TEST_F(SampleTest, sampleTest) {
+//   ScopedTestingPlatformSupport<MyTestingPlatformSupport> platform(
+//       WTF::makeUnique<MyTestingPlatformSupport>(...));
+//
+//   // Or can set an instance later.
+//   platform.reset(WTF::makeUnique<MyTestingPlatformSupport>(...));
+//   ...
+//   // Can call methods of MyTestingPlatformSupport.
+//   EXPECT_TRUE(platform->myMethodIsCalled());
+// }
+//
+// Note: A valid Platform instance should be already set before setting a
+// TestingPlatformSupport instance, and the original instance should outlive.
+// These conditions should be always true in usual use cases, but if you are
+// changing ScopedUnittestsEnvironmentSetup or something like that, you should
+// keep these restrictions in mind.
+template <class T>
+class ScopedTestingPlatformSupport {
+ public:
+  ScopedTestingPlatformSupport() : ScopedTestingPlatformSupport(nullptr) {}
+  explicit ScopedTestingPlatformSupport(
+      std::unique_ptr<T> testingPlatformSupport)
+      : m_originalPlatform(nullptr) {
+    reset(std::move(testingPlatformSupport));
+  }
+  void reset(std::unique_ptr<T> testingPlatformSupport) {

[haraken, 2017/01/11 13:03:38]

This method is getting a bit too complicated.

Maybe would it make things simpler if we introduce Vector<Platform*> and provide
a push/pop-style API? Every time a Platform* object is pushed or popped, you can
call Platform::setCurrentPlatformForTesting.

[Takashi Toyoshima, 2017/01/12 08:56:13]

Original intention of this complicated logic was to detect wrong usages that
users own TestingPlatformSupport instance directly without this class. But,
probably it's too complex only for a small benefit.

Also, changing this class to STACK_ALLOCATED would make it possible to remove
almost all logic here without introducing a Vector.

Please review the new patch set.

+    // Try obtaining a valid Platform instance to restore a valid instance
+    // always.
+    if (!m_originalPlatform)
+      m_originalPlatform = Platform::current();
+
+    // To set a TestingPlatformSupport instance, |m_originalPlatform| should be
+    // valid so that a valid instance could be always restored.
+    DCHECK(!testingPlatformSupport || m_originalPlatform);
+
+    // The old instance should be deleted before setting new Platform instance
+    // so to check if the TestingPlatformSupport was registered correctly in
+    // the TestingPlatformSupport's destructor.
+    m_testingPlatformSupport = std::move(testingPlatformSupport);
+
+    // To avoid calling setCurrentPlatformForTesting() with nullptr, do nothing
+    // if Platform::current() is still nullptr, and |testingPlatformSupport| is
+    // also nullptr.  This may happen for stack-allocated instances.
+    if (!m_testingPlatformSupport && !m_originalPlatform)
+      return;
+
+    Platform::setCurrentPlatformForTesting(m_testingPlatformSupport
+                                               ? m_testingPlatformSupport.get()
+                                               : m_originalPlatform);
+  }
+  ~ScopedTestingPlatformSupport() { reset(nullptr); }
+  const T* operator->() const { return m_testingPlatformSupport.get(); }
+  T* operator->() { return m_testingPlatformSupport.get(); }
+
+ private:
+  Platform* m_originalPlatform;
+  std::unique_ptr<T> m_testingPlatformSupport;
+};
+
 class ScopedUnittestsEnvironmentSetup {
   WTF_MAKE_NONCOPYABLE(ScopedUnittestsEnvironmentSetup);
 
@@ -202,7 +283,7 @@ class ScopedUnittestsEnvironmentSetup {
   std::unique_ptr<DummyPlatform> m_platform;
   std::unique_ptr<cc_blink::WebCompositorSupportImpl> m_compositorSupport;
   TestingPlatformSupport::Config m_testingPlatformConfig;
-  std::unique_ptr<TestingPlatformSupport> m_testingPlatformSupport;
+  ScopedTestingPlatformSupport<TestingPlatformSupport> m_testingPlatformSupport;

[Takashi Toyoshima, 2017/01/12 08:56:13]

Unfortunately, we can not use ScopedTestingPlatformSupport here any more. So we
manually manage TestingPlatformSupport only in this class exceptionally.

 };
 
 }  // namespace blink