Bug 1455302 - Robustify the IsSamplerThread() check similarly to the updater code. r?botond
Same as the previous patch, but adapted for the sampler thread.
MozReview-Commit-ID: 7PVaPl38FkM
--- a/gfx/layers/apz/public/APZSampler.h
+++ b/gfx/layers/apz/public/APZSampler.h
@@ -36,17 +36,18 @@ struct ScrollbarData;
* This interface exposes APZ methods related to "sampling" (i.e. reading the
* async transforms produced by APZ). These methods should all be called on
* the sampler thread.
*/
class APZSampler {
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(APZSampler)
public:
- explicit APZSampler(const RefPtr<APZCTreeManager>& aApz);
+ APZSampler(const RefPtr<APZCTreeManager>& aApz,
+ bool aIsUsingWebRender);
void SetWebRenderWindowId(const wr::WindowId& aWindowId);
/**
* This function is invoked from rust on the render backend thread when it
* is created. It effectively tells the APZSampler "the current thread is
* the sampler thread for this window id" and allows APZSampler to remember
* which thread it is.
@@ -97,41 +98,35 @@ public:
/**
* Returns true if currently on the APZSampler's "sampler thread".
*/
bool IsSamplerThread() const;
protected:
virtual ~APZSampler();
- bool UsingWebRenderSamplerThread() const;
static already_AddRefed<APZSampler> GetSampler(const wr::WrWindowId& aWindowId);
private:
RefPtr<APZCTreeManager> mApz;
+ bool mIsUsingWebRender;
// Used to manage the mapping from a WR window id to APZSampler. These are only
// used if WebRender is enabled. Both sWindowIdMap and mWindowId should only
// be used while holding the sWindowIdLock.
static StaticMutex sWindowIdLock;
static std::unordered_map<uint64_t, APZSampler*> sWindowIdMap;
Maybe<wr::WrWindowId> mWindowId;
+ // Lock used to protected mSamplerThreadId
+ mutable Mutex mThreadIdLock;
// If WebRender is enabled, this holds the thread id of the render backend
// thread (which is the sampler thread) for the compositor associated with
// this APZSampler instance.
- // This is written to once during init and never cleared, and so reading it
- // from multiple threads during normal operation (after initialization)
- // without locking should be fine.
Maybe<PlatformThreadId> mSamplerThreadId;
-#ifdef DEBUG
- // This flag is used to ensure that we don't ever try to do sampler-thread
- // stuff before the updater thread has been properly initialized.
- mutable bool mSamplerThreadQueried;
-#endif
Mutex mSampleTimeLock;
// Can only be accessed or modified while holding mSampleTimeLock.
TimeStamp mSampleTime;
};
} // namespace layers
} // namespace mozilla
--- a/gfx/layers/apz/src/APZSampler.cpp
+++ b/gfx/layers/apz/src/APZSampler.cpp
@@ -17,21 +17,21 @@
namespace mozilla {
namespace layers {
StaticMutex APZSampler::sWindowIdLock;
std::unordered_map<uint64_t, APZSampler*> APZSampler::sWindowIdMap;
-APZSampler::APZSampler(const RefPtr<APZCTreeManager>& aApz)
+APZSampler::APZSampler(const RefPtr<APZCTreeManager>& aApz,
+ bool aIsUsingWebRender)
: mApz(aApz)
-#ifdef DEBUG
- , mSamplerThreadQueried(false)
-#endif
+ , mIsUsingWebRender(aIsUsingWebRender)
+ , mThreadIdLock("APZSampler::mThreadIdLock")
, mSampleTimeLock("APZSampler::mSampleTimeLock")
{
MOZ_ASSERT(aApz);
mApz->SetSampler(this);
}
APZSampler::~APZSampler()
{
@@ -51,18 +51,17 @@ APZSampler::SetWebRenderWindowId(const w
mWindowId = Some(aWindowId);
sWindowIdMap[wr::AsUint64(aWindowId)] = this;
}
/*static*/ void
APZSampler::SetSamplerThread(const wr::WrWindowId& aWindowId)
{
if (RefPtr<APZSampler> sampler = GetSampler(aWindowId)) {
- // Ensure nobody tried to use the updater thread before this point.
- MOZ_ASSERT(!sampler->mSamplerThreadQueried);
+ MutexAutoLock lock(sampler->mThreadIdLock);
sampler->mSamplerThreadId = Some(PlatformThread::CurrentId());
}
}
/*static*/ void
APZSampler::SampleForWebRender(const wr::WrWindowId& aWindowId,
wr::Transaction* aTransaction)
{
@@ -209,39 +208,27 @@ APZSampler::AssertOnSamplerThread() cons
if (APZThreadUtils::GetThreadAssertionsEnabled()) {
MOZ_ASSERT(IsSamplerThread());
}
}
bool
APZSampler::IsSamplerThread() const
{
- if (UsingWebRenderSamplerThread()) {
- return PlatformThread::CurrentId() == *mSamplerThreadId;
+ if (mIsUsingWebRender) {
+ // If the sampler thread id isn't set yet then we cannot be running on the
+ // sampler thread (because we will have the thread id before we run any
+ // other C++ code on it, and this function is only ever invoked from C++
+ // code), so return false in that scenario.
+ MutexAutoLock lock(mThreadIdLock);
+ return mSamplerThreadId && PlatformThread::CurrentId() == *mSamplerThreadId;
}
return CompositorThreadHolder::IsInCompositorThread();
}
-bool
-APZSampler::UsingWebRenderSamplerThread() const
-{
- // If mSamplerThreadId is not set at the point that this is called, then
- // that means that either (a) WebRender is not enabled for the compositor
- // to which this APZSampler is attached or (b) we are attempting to do
- // something sampler-related before WebRender is up and running. In case
- // (a) falling back to the compositor thread is correct, and in case (b)
- // we should stop doing the sampler-related thing so early. We catch this
- // case by setting the mSamplerThreadQueried flag and asserting on WR
- // initialization.
-#ifdef DEBUG
- mSamplerThreadQueried = true;
-#endif
- return mSamplerThreadId.isSome();
-}
-
/*static*/ already_AddRefed<APZSampler>
APZSampler::GetSampler(const wr::WrWindowId& aWindowId)
{
RefPtr<APZSampler> sampler;
StaticMutexAutoLock lock(sWindowIdLock);
auto it = sWindowIdMap.find(wr::AsUint64(aWindowId));
if (it != sWindowIdMap.end()) {
sampler = it->second;
--- a/gfx/layers/apz/test/gtest/APZCBasicTester.h
+++ b/gfx/layers/apz/test/gtest/APZCBasicTester.h
@@ -27,17 +27,17 @@ protected:
virtual void SetUp()
{
gfxPrefs::GetSingleton();
APZThreadUtils::SetThreadAssertionsEnabled(false);
APZThreadUtils::SetControllerThread(MessageLoop::current());
tm = new TestAPZCTreeManager(mcc);
updater = new APZUpdater(tm, false);
- sampler = new APZSampler(tm);
+ sampler = new APZSampler(tm, false);
apzc = new TestAsyncPanZoomController(LayersId{0}, mcc, tm, mGestureBehavior);
apzc->SetFrameMetrics(TestFrameMetrics());
apzc->GetScrollMetadata().SetIsLayersIdRoot(true);
}
/**
* Get the APZC's scroll range in CSS pixels.
*/
--- a/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h
+++ b/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h
@@ -23,17 +23,17 @@ protected:
virtual void SetUp() {
gfxPrefs::GetSingleton();
gfxPlatform::GetPlatform();
APZThreadUtils::SetThreadAssertionsEnabled(false);
APZThreadUtils::SetControllerThread(MessageLoop::current());
manager = new TestAPZCTreeManager(mcc);
updater = new APZUpdater(manager, false);
- sampler = new APZSampler(manager);
+ sampler = new APZSampler(manager, false);
}
virtual void TearDown() {
while (mcc->RunThroughDelayedTasks());
manager->ClearTree();
manager->ClearContentController();
}
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -380,17 +380,17 @@ CompositorBridgeParent::Initialize()
MOZ_ASSERT(CompositorThread(),
"The compositor thread must be Initialized before instanciating a CompositorBridgeParent.");
if (mOptions.UseAPZ()) {
MOZ_ASSERT(!mApzcTreeManager);
MOZ_ASSERT(!mApzSampler);
MOZ_ASSERT(!mApzUpdater);
mApzcTreeManager = new APZCTreeManager(mRootLayerTreeID);
- mApzSampler = new APZSampler(mApzcTreeManager);
+ mApzSampler = new APZSampler(mApzcTreeManager, mOptions.UseWebRender());
mApzUpdater = new APZUpdater(mApzcTreeManager, mOptions.UseWebRender());
}
mCompositorBridgeID = 0;
// FIXME: This holds on the the fact that right now the only thing that
// can destroy this instance is initialized on the compositor thread after
// this task has been processed.
MOZ_ASSERT(CompositorLoop());