TestingPlatformSupport: register Platform instance correctly

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

 /*
  * Copyright (C) 2014 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 19 matching lines @@
 
 #ifndef TestingPlatformSupport_h
 #define TestingPlatformSupport_h
 
 #include "platform/PlatformExport.h"
 #include "platform/WebTaskRunner.h"
 #include "public/platform/Platform.h"
 #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>
 
 namespace base {
 class SimpleTestTickClock;
 class TestDiscardableMemoryAllocator;
 }
 
 namespace cc {
 class OrderedSimpleTaskRunner;
@@ skipping 50 matching lines @@
       InterventionReporter*,
       WebViewScheduler::WebViewSchedulerSettings*) override {
     return nullptr;
   }
   void suspendTimerQueue() override {}
   void resumeTimerQueue() override {}
   void addPendingNavigation(WebScheduler::NavigatingFrameType) override {}
   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);
 
  public:
   struct Config {
     WebCompositorSupport* compositorSupport = nullptr;
   };
 
   TestingPlatformSupport();
   explicit TestingPlatformSupport(const Config&);
@@ skipping 15 matching lines @@
   InterfaceProvider* interfaceProvider() override;
 
  protected:
   class TestingInterfaceProvider;
 
   const Config m_config;
   Platform* const m_oldPlatform;
   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);
 
  public:
   TestingPlatformSupportWithMockScheduler();
   explicit TestingPlatformSupportWithMockScheduler(const Config&);
   ~TestingPlatformSupportWithMockScheduler() override;
 
   // Platform:
   WebThread* currentThread() override;
@@ skipping 26 matching lines @@
 
  protected:
   static double getTestTime();
 
   std::unique_ptr<base::SimpleTestTickClock> m_clock;
   scoped_refptr<cc::OrderedSimpleTaskRunner> m_mockTaskRunner;
   std::unique_ptr<scheduler::RendererSchedulerImpl> m_scheduler;
   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);
 
  public:
   ScopedUnittestsEnvironmentSetup(int argc, char** argv);
   ~ScopedUnittestsEnvironmentSetup();
 
  private:
   class DummyPlatform;
   std::unique_ptr<base::TestDiscardableMemoryAllocator>
       m_discardableMemoryAllocator;
   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
 
 #endif  // TestingPlatformSupport_h