From 3a110e2974abc0705af529565bf16507289e7b70 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Thu, 14 Jan 2016 00:16:49 -0500 Subject: [PATCH] Bug 1219339: switch GetStaticInstance to use IPC's Singleton impl r=froyd --- media/webrtc/signaling/test/FakeIPC.cpp | 35 +++++ media/webrtc/signaling/test/FakeIPC.h | 22 ++++ .../signaling/test/jsep_session_unittest.cpp | 3 + .../signaling/test/jsep_track_unittest.cpp | 3 + .../signaling/test/mediaconduit_unittests.cpp | 3 + .../signaling/test/mediapipeline_unittest.cpp | 3 + .../webrtc/signaling/test/sdp_file_parser.cpp | 3 + media/webrtc/signaling/test/sdp_unittests.cpp | 3 + .../signaling/test/signaling_unittests.cpp | 3 + .../modules/rtp_rtcp/source/ssrc_database.h | 2 +- .../interface/static_instance.h | 120 +----------------- .../system_wrappers/source/rw_lock_win.cc | 8 +- .../system_wrappers/source/trace_impl.cc | 8 +- 13 files changed, 97 insertions(+), 119 deletions(-) create mode 100644 media/webrtc/signaling/test/FakeIPC.cpp create mode 100644 media/webrtc/signaling/test/FakeIPC.h diff --git a/media/webrtc/signaling/test/FakeIPC.cpp b/media/webrtc/signaling/test/FakeIPC.cpp new file mode 100644 index 000000000000..767082c29e08 --- /dev/null +++ b/media/webrtc/signaling/test/FakeIPC.cpp @@ -0,0 +1,35 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "FakeIPC.h" +#include + +// The implementations can't be in the .h file for some annoying reason + +/* static */ void +PlatformThread:: YieldCurrentThread() +{ + sleep(1); +} + +namespace base { + +void AtExitManager::RegisterCallback(AtExitCallbackType func, void* param) +{ +} + +} + +// see atomicops_internals_x86_gcc.h +// This cheats to get the unittests to build + +struct AtomicOps_x86CPUFeatureStruct { + bool field1; + bool field2; +}; + +struct AtomicOps_x86CPUFeatureStruct AtomicOps_Internalx86CPUFeatures = { + false, + false, +}; diff --git a/media/webrtc/signaling/test/FakeIPC.h b/media/webrtc/signaling/test/FakeIPC.h new file mode 100644 index 000000000000..e13fc271d2b0 --- /dev/null +++ b/media/webrtc/signaling/test/FakeIPC.h @@ -0,0 +1,22 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef FAKE_IPC_H_ +#define FAKE_IPC_H_ +#include + +class PlatformThread { +public: + static void YieldCurrentThread(); +}; + +namespace base { +class AtExitManager { +public: + typedef void (*AtExitCallbackType)(void*); + + static void RegisterCallback(AtExitCallbackType func, void* param); +}; +} +#endif diff --git a/media/webrtc/signaling/test/jsep_session_unittest.cpp b/media/webrtc/signaling/test/jsep_session_unittest.cpp index 48923152a928..4536e2056ba4 100644 --- a/media/webrtc/signaling/test/jsep_session_unittest.cpp +++ b/media/webrtc/signaling/test/jsep_session_unittest.cpp @@ -32,6 +32,9 @@ #include "mtransport_test_utils.h" +#include "FakeIPC.h" +#include "FakeIPC.cpp" + namespace mozilla { static std::string kAEqualsCandidate("a=candidate:"); const static size_t kNumCandidatesPerComponent = 3; diff --git a/media/webrtc/signaling/test/jsep_track_unittest.cpp b/media/webrtc/signaling/test/jsep_track_unittest.cpp index 44d39ddfa5c6..87f783c35f62 100644 --- a/media/webrtc/signaling/test/jsep_track_unittest.cpp +++ b/media/webrtc/signaling/test/jsep_track_unittest.cpp @@ -19,6 +19,9 @@ #include "mtransport_test_utils.h" +#include "FakeIPC.h" +#include "FakeIPC.cpp" + namespace mozilla { class JsepTrackTest : public ::testing::Test diff --git a/media/webrtc/signaling/test/mediaconduit_unittests.cpp b/media/webrtc/signaling/test/mediaconduit_unittests.cpp index c14dc373c778..3578e45d3560 100644 --- a/media/webrtc/signaling/test/mediaconduit_unittests.cpp +++ b/media/webrtc/signaling/test/mediaconduit_unittests.cpp @@ -22,6 +22,9 @@ using namespace std; #include "runnable_utils.h" #include "signaling/src/common/EncodingConstraints.h" +#include "FakeIPC.h" +#include "FakeIPC.cpp" + #define GTEST_HAS_RTTI 0 #include "gtest/gtest.h" #include "gtest_utils.h" diff --git a/media/webrtc/signaling/test/mediapipeline_unittest.cpp b/media/webrtc/signaling/test/mediapipeline_unittest.cpp index d67fc6dacbd0..bc6e9a056107 100644 --- a/media/webrtc/signaling/test/mediapipeline_unittest.cpp +++ b/media/webrtc/signaling/test/mediapipeline_unittest.cpp @@ -36,6 +36,9 @@ #include "webrtc/modules/interface/module_common_types.h" +#include "FakeIPC.h" +#include "FakeIPC.cpp" + #define GTEST_HAS_RTTI 0 #include "gtest/gtest.h" #include "gtest_utils.h" diff --git a/media/webrtc/signaling/test/sdp_file_parser.cpp b/media/webrtc/signaling/test/sdp_file_parser.cpp index fe2f63b00398..3fb0c2f1ceda 100644 --- a/media/webrtc/signaling/test/sdp_file_parser.cpp +++ b/media/webrtc/signaling/test/sdp_file_parser.cpp @@ -18,6 +18,9 @@ #include "signaling/src/sdp/SipccSdpParser.h" +#include "FakeIPC.h" +#include "FakeIPC.cpp" + namespace mozilla { const std::string kDefaultFilename((char *)"/tmp/sdp.bin"); diff --git a/media/webrtc/signaling/test/sdp_unittests.cpp b/media/webrtc/signaling/test/sdp_unittests.cpp index c13d87b8baa4..fc4e58970c94 100644 --- a/media/webrtc/signaling/test/sdp_unittests.cpp +++ b/media/webrtc/signaling/test/sdp_unittests.cpp @@ -44,6 +44,9 @@ extern "C" { #endif #define CRLF "\r\n" +#include "FakeIPC.h" +#include "FakeIPC.cpp" + using namespace mozilla; namespace test { diff --git a/media/webrtc/signaling/test/signaling_unittests.cpp b/media/webrtc/signaling/test/signaling_unittests.cpp index c3817171ba1b..92bf6a2a4116 100644 --- a/media/webrtc/signaling/test/signaling_unittests.cpp +++ b/media/webrtc/signaling/test/signaling_unittests.cpp @@ -45,6 +45,9 @@ #include "PeerConnectionImplEnumsBinding.cpp" #endif +#include "FakeIPC.h" +#include "FakeIPC.cpp" + #include "ice_ctx.h" #include "ice_peer_ctx.h" diff --git a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/ssrc_database.h b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/ssrc_database.h index 2d4932afa7f2..e95b8324d686 100644 --- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/ssrc_database.h +++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/ssrc_database.h @@ -29,10 +29,10 @@ public: int32_t RegisterSSRC(const uint32_t ssrc); int32_t ReturnSSRC(const uint32_t ssrc); -protected: SSRCDatabase(); virtual ~SSRCDatabase(); +protected: static SSRCDatabase* CreateInstance() { return new SSRCDatabase(); } private: diff --git a/media/webrtc/trunk/webrtc/system_wrappers/interface/static_instance.h b/media/webrtc/trunk/webrtc/system_wrappers/interface/static_instance.h index dad9c52dc9fc..071edabfa019 100644 --- a/media/webrtc/trunk/webrtc/system_wrappers/interface/static_instance.h +++ b/media/webrtc/trunk/webrtc/system_wrappers/interface/static_instance.h @@ -13,10 +13,10 @@ #include -#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" -#ifdef _WIN32 -#include "webrtc/system_wrappers/interface/fix_interlocked_exchange_pointer_win.h" +#if defined(WEBRTC_ANDROID) || defined(WEBRTC_GONK) +#define OS_LINUX #endif +#include "base/singleton.h" namespace webrtc { @@ -35,119 +35,9 @@ template // Construct On First Use idiom. Avoids // "static initialization order fiasco". static T* GetStaticInstance(CountOperation count_operation) { - // TODO (hellner): use atomic wrapper instead. - static volatile long instance_count = 0; - static T* volatile instance = NULL; - CreateOperation state = kInstanceExists; -#ifndef _WIN32 - // This memory is staticly allocated once. The application does not try to - // free this memory. This approach is taken to avoid issues with - // destruction order for statically allocated memory. The memory will be - // reclaimed by the OS and memory leak tools will not recognize memory - // reachable from statics leaked so no noise is added by doing this. - static CriticalSectionWrapper* crit_sect( - CriticalSectionWrapper::CreateCriticalSection()); - CriticalSectionScoped lock(crit_sect); - - if (count_operation == - kAddRefNoCreate && instance_count == 0) { - return NULL; - } - if (count_operation == - kAddRef || - count_operation == kAddRefNoCreate) { - instance_count++; - if (instance_count == 1) { - state = kCreate; - } - } else { - instance_count--; - if (instance_count == 0) { - state = kDestroy; - } - } - if (state == kCreate) { - instance = T::CreateInstance(); - } else if (state == kDestroy) { - T* old_instance = instance; - instance = NULL; - // The state will not change past this point. Release the critical - // section while deleting the object in case it would be blocking on - // access back to this object. (This is the case for the tracing class - // since the thread owned by the tracing class also traces). - // TODO(hellner): this is a bit out of place but here goes, de-couple - // thread implementation with trace implementation. - crit_sect->Leave(); - if (old_instance) { - delete old_instance; - } - // Re-acquire the lock since the scoped critical section will release - // it. - crit_sect->Enter(); - return NULL; - } -#else // _WIN32 - if (count_operation == - kAddRefNoCreate && instance_count == 0) { - return NULL; - } - if (count_operation == kAddRefNoCreate) { - if (1 == InterlockedIncrement(&instance_count)) { - // The instance has been destroyed by some other thread. Rollback. - InterlockedDecrement(&instance_count); - assert(false); - return NULL; - } - // Sanity to catch corrupt state. - if (instance == NULL) { - assert(false); - InterlockedDecrement(&instance_count); - return NULL; - } - } else if (count_operation == kAddRef) { - if (instance_count == 0) { - state = kCreate; - } else { - if (1 == InterlockedIncrement(&instance_count)) { - // InterlockedDecrement because reference count should not be - // updated just yet (that's done when the instance is created). - InterlockedDecrement(&instance_count); - state = kCreate; - } - } - } else { - int new_value = InterlockedDecrement(&instance_count); - if (new_value == 0) { - state = kDestroy; - } - } - - if (state == kCreate) { - // Create instance and let whichever thread finishes first assign its - // local copy to the global instance. All other threads reclaim their - // local copy. - T* new_instance = T::CreateInstance(); - if (1 == InterlockedIncrement(&instance_count)) { - InterlockedExchangePointer(reinterpret_cast(&instance), - new_instance); - } else { - InterlockedDecrement(&instance_count); - if (new_instance) { - delete static_cast(new_instance); - } - } - } else if (state == kDestroy) { - T* old_value = static_cast(InterlockedExchangePointer( - reinterpret_cast(&instance), NULL)); - if (old_value) { - delete static_cast(old_value); - } - return NULL; - } -#endif // #ifndef _WIN32 - return instance; + // Simple solution since we don't use this for large objects anymore + return Singleton::get(); } - } // namspace webrtc #endif // WEBRTC_SYSTEM_WRAPPERS_INTERFACE_STATIC_INSTANCE_H_ diff --git a/media/webrtc/trunk/webrtc/system_wrappers/source/rw_lock_win.cc b/media/webrtc/trunk/webrtc/system_wrappers/source/rw_lock_win.cc index aea74fa4a37c..a1d4b58195f8 100644 --- a/media/webrtc/trunk/webrtc/system_wrappers/source/rw_lock_win.cc +++ b/media/webrtc/trunk/webrtc/system_wrappers/source/rw_lock_win.cc @@ -69,7 +69,11 @@ bool RWLockWin::LoadModule() { if (!library) { return false; } - WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Kernel.dll"); + // Don't log here, since we may be creating a FileWrapper for + // the TraceImpl, from within GetStaticInstance. With singleton.h, you + // can't safely call GetStaticInstance on the same object from within + // creating that object (go figure...) + //WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Kernel.dll"); initialize_srw_lock = (InitializeSRWLock)GetProcAddress(library, "InitializeSRWLock"); @@ -88,7 +92,7 @@ bool RWLockWin::LoadModule() { if (initialize_srw_lock && acquire_srw_lock_exclusive && release_srw_lock_exclusive && acquire_srw_lock_shared && release_srw_lock_shared) { - WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Native RW Lock"); + //WEBRTC_TRACE(kTraceStateInfo, kTraceUtility, -1, "Loaded Native RW Lock"); native_rw_locks_supported = true; } return native_rw_locks_supported; diff --git a/media/webrtc/trunk/webrtc/system_wrappers/source/trace_impl.cc b/media/webrtc/trunk/webrtc/system_wrappers/source/trace_impl.cc index 661aefd68315..4322603412d7 100644 --- a/media/webrtc/trunk/webrtc/system_wrappers/source/trace_impl.cc +++ b/media/webrtc/trunk/webrtc/system_wrappers/source/trace_impl.cc @@ -14,6 +14,7 @@ #include #include #include +#include "base/singleton.h" #ifdef _WIN32 #include "webrtc/system_wrappers/source/trace_win.h" @@ -63,7 +64,12 @@ TraceImpl* TraceImpl::StaticInstance(CountOperation count_operation, } } TraceImpl* impl = - GetStaticInstance(count_operation); +#if defined(_WIN32) + GetStaticInstance(count_operation); +#else + GetStaticInstance(count_operation); +#endif + return impl; }