Skip to content

Commit

Permalink
Bug 1455691 - Make the transaction id a struct instead of a uint64_t.…
Browse files Browse the repository at this point in the history
… r=mattwoodrow

MozReview-Commit-ID: 9yZknygQvFr
  • Loading branch information
staktrace committed Apr 20, 2018
1 parent df8eb8d commit ecb05b5
Show file tree
Hide file tree
Showing 36 changed files with 193 additions and 125 deletions.
2 changes: 1 addition & 1 deletion dom/base/nsDOMWindowUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2474,7 +2474,7 @@ nsDOMWindowUtils::GetLastTransactionId(uint64_t *aLastTransactionId)
}

nsRefreshDriver* driver = presContext->GetRootPresContext()->RefreshDriver();
*aLastTransactionId = driver->LastTransactionId();
*aLastTransactionId = uint64_t(driver->LastTransactionId());
return NS_OK;
}

Expand Down
2 changes: 1 addition & 1 deletion dom/ipc/TabChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3124,7 +3124,7 @@ TabChild::GetFrom(layers::LayersId aLayersId)
}

void
TabChild::DidComposite(uint64_t aTransactionId,
TabChild::DidComposite(mozilla::layers::TransactionId aTransactionId,
const TimeStamp& aCompositeStart,
const TimeStamp& aCompositeEnd)
{
Expand Down
2 changes: 1 addition & 1 deletion dom/ipc/TabChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ class TabChild final : public TabChildBase,
layers::LayersId GetLayersId() { return mLayersId; }
Maybe<bool> IsLayersConnected() { return mLayersConnected; }

void DidComposite(uint64_t aTransactionId,
void DidComposite(mozilla::layers::TransactionId aTransactionId,
const TimeStamp& aCompositeStart,
const TimeStamp& aCompositeEnd);

Expand Down
4 changes: 2 additions & 2 deletions gfx/layers/Layers.h
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ class LayerManager {

virtual void SetLayerObserverEpoch(uint64_t aLayerObserverEpoch) {}

virtual void DidComposite(uint64_t aTransactionId,
virtual void DidComposite(TransactionId aTransactionId,
const mozilla::TimeStamp& aCompositeStart,
const mozilla::TimeStamp& aCompositeEnd) {}

Expand All @@ -710,7 +710,7 @@ class LayerManager {

virtual void SetTransactionIdAllocator(TransactionIdAllocator* aAllocator) {}

virtual uint64_t GetLastTransactionId() { return 0; }
virtual TransactionId GetLastTransactionId() { return TransactionId{0}; }

virtual CompositorBridgeChild* GetCompositorBridgeChild() { return nullptr; }

Expand Down
51 changes: 51 additions & 0 deletions gfx/layers/LayersTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,57 @@ struct LayersId {
};
};

struct TransactionId {
uint64_t mId;

bool IsValid() const {
return mId != 0;
}

MOZ_MUST_USE TransactionId Next() const {
return TransactionId{mId + 1};
}

MOZ_MUST_USE TransactionId Prev() const {
return TransactionId{mId - 1};
}

int64_t operator-(const TransactionId& aOther) const {
return mId - aOther.mId;
}

// Allow explicit cast to a uint64_t for now
explicit operator uint64_t() const
{
return mId;
}

bool operator<(const TransactionId& aOther) const
{
return mId < aOther.mId;
}

bool operator<=(const TransactionId& aOther) const
{
return mId <= aOther.mId;
}

bool operator>(const TransactionId& aOther) const
{
return mId > aOther.mId;
}

bool operator>=(const TransactionId& aOther) const
{
return mId >= aOther.mId;
}

bool operator==(const TransactionId& aOther) const
{
return mId == aOther.mId;
}
};

enum class LayersBackend : int8_t {
LAYERS_NONE = 0,
LAYERS_BASIC,
Expand Down
11 changes: 6 additions & 5 deletions gfx/layers/TransactionIdAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#define GFX_TRANSACTION_ID_ALLOCATOR_H

#include "nsISupportsImpl.h"
#include "mozilla/layers/LayersTypes.h"
#include "mozilla/TimeStamp.h"

namespace mozilla {
Expand All @@ -29,14 +30,14 @@ class TransactionIdAllocator {
* "throttling" behaviour can be skipped by passing aThrottle=false.
* Otherwise call sites should generally be passing aThrottle=true.
*/
virtual uint64_t GetTransactionId(bool aThrottle) = 0;
virtual TransactionId GetTransactionId(bool aThrottle) = 0;

/**
* Return the transaction id that for the last non-revoked transaction.
* This allows the caller to tell whether a composite was triggered by
* a paint that occurred after a call to TransactionId().
*/
virtual uint64_t LastTransactionId() const = 0;
virtual TransactionId LastTransactionId() const = 0;

/**
* Notify that all work (including asynchronous composites)
Expand All @@ -46,15 +47,15 @@ class TransactionIdAllocator {
* of having too many outstanding id's, then this may
* resume it.
*/
virtual void NotifyTransactionCompleted(uint64_t aTransactionId) = 0;
virtual void NotifyTransactionCompleted(TransactionId aTransactionId) = 0;

/**
* Revoke a transaction id that isn't needed to track
* completion of asynchronous work. This is similar
* to NotifyTransactionCompleted except avoids
* return ordering issues.
*/
virtual void RevokeTransactionId(uint64_t aTransactionId) = 0;
virtual void RevokeTransactionId(TransactionId aTransactionId) = 0;

/**
* Stop waiting for pending transactions, if any.
Expand All @@ -73,7 +74,7 @@ class TransactionIdAllocator {
* id to the last transaction id from previous refresh driver, so that all
* completed transactions of previous refresh driver will be ignored.
*/
virtual void ResetInitialTransactionId(uint64_t aTransactionId) = 0;
virtual void ResetInitialTransactionId(TransactionId aTransactionId) = 0;

/**
* Get the start time of the current refresh tick.
Expand Down
8 changes: 4 additions & 4 deletions gfx/layers/client/ClientLayerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ NS_IMPL_ISUPPORTS(ClientLayerManager::MemoryPressureObserver, nsIObserver)
ClientLayerManager::ClientLayerManager(nsIWidget* aWidget)
: mPhase(PHASE_NONE)
, mWidget(aWidget)
, mLatestTransactionId(0)
, mLatestTransactionId{0}
, mLastPaintTime(TimeDuration::Forever())
, mTargetRotation(ROTATION_0)
, mRepeatTransaction(false)
Expand Down Expand Up @@ -139,7 +139,7 @@ ClientLayerManager::Destroy()
// pending transaction. Do this at the top of the event loop so we don't
// cause a paint to occur during compositor shutdown.
RefPtr<TransactionIdAllocator> allocator = mTransactionIdAllocator;
uint64_t id = mLatestTransactionId;
TransactionId id = mLatestTransactionId;

RefPtr<Runnable> task = NS_NewRunnableFunction(
"TransactionIdAllocator::NotifyTransactionCompleted",
Expand Down Expand Up @@ -505,7 +505,7 @@ ClientLayerManager::ScheduleComposite()
}

void
ClientLayerManager::DidComposite(uint64_t aTransactionId,
ClientLayerManager::DidComposite(TransactionId aTransactionId,
const TimeStamp& aCompositeStart,
const TimeStamp& aCompositeEnd)
{
Expand All @@ -519,7 +519,7 @@ ClientLayerManager::DidComposite(uint64_t aTransactionId,

// |aTransactionId| will be > 0 if the compositor is acknowledging a shadow
// layers transaction.
if (aTransactionId) {
if (aTransactionId.IsValid()) {
nsIWidgetListener *listener = mWidget->GetWidgetListener();
if (listener) {
listener->DidCompositeWindow(aTransactionId, aCompositeStart, aCompositeEnd);
Expand Down
6 changes: 3 additions & 3 deletions gfx/layers/client/ClientLayerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class ClientLayerManager final : public LayerManager
virtual void ScheduleComposite() override;
virtual void GetFrameUniformity(FrameUniformityData* aFrameUniformityData) override;

virtual void DidComposite(uint64_t aTransactionId,
virtual void DidComposite(TransactionId aTransactionId,
const mozilla::TimeStamp& aCompositeStart,
const mozilla::TimeStamp& aCompositeEnd) override;

Expand Down Expand Up @@ -237,7 +237,7 @@ class ClientLayerManager final : public LayerManager

virtual void SetTransactionIdAllocator(TransactionIdAllocator* aAllocator) override;

virtual uint64_t GetLastTransactionId() override { return mLatestTransactionId; }
virtual TransactionId GetLastTransactionId() override { return mLatestTransactionId; }

float RequestProperty(const nsAString& aProperty) override;

Expand Down Expand Up @@ -332,7 +332,7 @@ class ClientLayerManager final : public LayerManager
RefPtr<gfxContext> mShadowTarget;

RefPtr<TransactionIdAllocator> mTransactionIdAllocator;
uint64_t mLatestTransactionId;
TransactionId mLatestTransactionId;
TimeDuration mLastPaintTime;

// Sometimes we draw to targets that don't natively support
Expand Down
2 changes: 1 addition & 1 deletion gfx/layers/ipc/CompositorBridgeChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ CompositorBridgeChild::RecvHideAllPlugins(const uintptr_t& aParentWidget)

mozilla::ipc::IPCResult
CompositorBridgeChild::RecvDidComposite(const LayersId& aId,
const uint64_t& aTransactionId,
const TransactionId& aTransactionId,
const TimeStamp& aCompositeStart,
const TimeStamp& aCompositeEnd)
{
Expand Down
3 changes: 2 additions & 1 deletion gfx/layers/ipc/CompositorBridgeChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ class CompositorBridgeChild final : public PCompositorBridgeChild,
static bool CompositorIsInGPUProcess();

virtual mozilla::ipc::IPCResult
RecvDidComposite(const LayersId& aId, const uint64_t& aTransactionId,
RecvDidComposite(const LayersId& aId,
const TransactionId& aTransactionId,
const TimeStamp& aCompositeStart,
const TimeStamp& aCompositeEnd) override;

Expand Down
14 changes: 7 additions & 7 deletions gfx/layers/ipc/CompositorBridgeParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ CompositorBridgeParent::CompositorBridgeParent(CompositorManagerParent* aManager
, mWidget(nullptr)
, mScale(aScale)
, mVsyncRate(aVsyncRate)
, mPendingTransaction(0)
, mPendingTransaction{0}
, mPaused(false)
, mUseExternalSurfaceSize(aUseExternalSurfaceSize)
, mEGLSurfaceSize(aSurfaceSize)
Expand Down Expand Up @@ -1265,7 +1265,7 @@ CompositorBridgeParent::ShadowLayersUpdated(LayerTransactionParent* aLayerTree,
// The transaction ID might get reset to 1 if the page gets reloaded, see
// https://bugzilla.mozilla.org/show_bug.cgi?id=1145295#c41
// Otherwise, it should be continually increasing.
MOZ_ASSERT(aInfo.id() == 1 || aInfo.id() > mPendingTransaction);
MOZ_ASSERT(aInfo.id() == TransactionId{1} || aInfo.id() > mPendingTransaction);
mPendingTransaction = aInfo.id();
mTxnStartTime = aInfo.transactionStart();
mFwdTime = aInfo.fwdTime();
Expand Down Expand Up @@ -2045,7 +2045,7 @@ CompositorBridgeParent::DidComposite(TimeStamp& aCompositeStart,
} else {
NotifyDidComposite(mPendingTransaction, aCompositeStart, aCompositeEnd);
#if defined(ENABLE_FRAME_LATENCY_LOG)
if (mPendingTransaction) {
if (mPendingTransaction.IsValid()) {
if (mTxnStartTime) {
uint32_t latencyMs = round((aCompositeEnd - mTxnStartTime).ToMilliseconds());
printf_stderr("From transaction start to end of generate frame latencyMs %d this %p\n", latencyMs, this);
Expand All @@ -2058,7 +2058,7 @@ CompositorBridgeParent::DidComposite(TimeStamp& aCompositeStart,
mTxnStartTime = TimeStamp();
mFwdTime = TimeStamp();
#endif
mPendingTransaction = 0;
mPendingTransaction = TransactionId{0};
}
}

Expand All @@ -2083,7 +2083,7 @@ CompositorBridgeParent::NotifyDidCompositeToPipeline(const wr::PipelineId& aPipe
}

if (mWrBridge->PipelineId() == aPipelineId) {
uint64_t transactionId = mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
TransactionId transactionId = mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
Unused << SendDidComposite(LayersId{0}, transactionId, aCompositeStart, aCompositeEnd);

nsTArray<ImageCompositeNotificationInfo> notifications;
Expand All @@ -2100,14 +2100,14 @@ CompositorBridgeParent::NotifyDidCompositeToPipeline(const wr::PipelineId& aPipe
lts->mWrBridge &&
lts->mWrBridge->PipelineId() == aPipelineId) {
CrossProcessCompositorBridgeParent* cpcp = lts->mCrossProcessParent;
uint64_t transactionId = lts->mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
TransactionId transactionId = lts->mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
Unused << cpcp->SendDidComposite(aLayersId, transactionId, aCompositeStart, aCompositeEnd);
}
});
}

void
CompositorBridgeParent::NotifyDidComposite(uint64_t aTransactionId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd)
CompositorBridgeParent::NotifyDidComposite(TransactionId aTransactionId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd)
{
Unused << SendDidComposite(LayersId{0}, aTransactionId, aCompositeStart, aCompositeEnd);

Expand Down
4 changes: 2 additions & 2 deletions gfx/layers/ipc/CompositorBridgeParent.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ class CompositorBridgeParent final : public CompositorBridgeParentBase
TimeStamp& aCompositeEnd) override;
void NotifyPipelineRemoved(const wr::PipelineId& aPipelineId) override;

void NotifyDidComposite(uint64_t aTransactionId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd);
void NotifyDidComposite(TransactionId aTransactionId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd);

// The indirect layer tree lock must be held before calling this function.
// Callback should take (LayerTreeState* aState, const LayersId& aLayersId)
Expand All @@ -603,7 +603,7 @@ class CompositorBridgeParent final : public CompositorBridgeParentBase
CSSToLayoutDeviceScale mScale;
TimeDuration mVsyncRate;

uint64_t mPendingTransaction;
TransactionId mPendingTransaction;
TimeStamp mTxnStartTime;
TimeStamp mFwdTime;

Expand Down
8 changes: 4 additions & 4 deletions gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,13 +386,13 @@ CrossProcessCompositorBridgeParent::DidCompositeLocked(
{
sIndirectLayerTreesLock->AssertCurrentThreadOwns();
if (LayerTransactionParent *layerTree = sIndirectLayerTrees[aId].mLayerTree) {
uint64_t transactionId = layerTree->FlushTransactionId(aCompositeEnd);
if (transactionId) {
TransactionId transactionId = layerTree->FlushTransactionId(aCompositeEnd);
if (transactionId.IsValid()) {
Unused << SendDidComposite(aId, transactionId, aCompositeStart, aCompositeEnd);
}
} else if (WebRenderBridgeParent* wrbridge = sIndirectLayerTrees[aId].mWrBridge) {
uint64_t transactionId = wrbridge->FlushPendingTransactionIds();
if (transactionId) {
TransactionId transactionId = wrbridge->FlushPendingTransactionIds();
if (transactionId.IsValid()) {
Unused << SendDidComposite(aId, transactionId, aCompositeStart, aCompositeEnd);
}
}
Expand Down
12 changes: 6 additions & 6 deletions gfx/layers/ipc/LayerTransactionParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ LayerTransactionParent::LayerTransactionParent(HostLayerManager* aManager,
, mId(aId)
, mChildEpoch(0)
, mParentEpoch(0)
, mPendingTransaction(0)
, mPendingTransaction{0}
, mDestroyed(false)
, mIPCOpen(false)
{
Expand Down Expand Up @@ -149,7 +149,7 @@ class MOZ_STACK_CLASS AutoLayerTransactionParentAsyncMessageSender
};

mozilla::ipc::IPCResult
LayerTransactionParent::RecvPaintTime(const uint64_t& aTransactionId,
LayerTransactionParent::RecvPaintTime(const TransactionId& aTransactionId,
const TimeDuration& aPaintTime)
{
mCompositorBridge->UpdatePaintTime(this, aPaintTime);
Expand Down Expand Up @@ -891,11 +891,11 @@ bool LayerTransactionParent::IsSameProcess() const
return OtherPid() == base::GetCurrentProcId();
}

uint64_t
TransactionId
LayerTransactionParent::FlushTransactionId(TimeStamp& aCompositeEnd)
{
#if defined(ENABLE_FRAME_LATENCY_LOG)
if (mPendingTransaction) {
if (mPendingTransaction.IsValid()) {
if (mTxnStartTime) {
uint32_t latencyMs = round((aCompositeEnd - mTxnStartTime).ToMilliseconds());
printf_stderr("From transaction start to end of generate frame latencyMs %d this %p\n", latencyMs, this);
Expand All @@ -908,8 +908,8 @@ LayerTransactionParent::FlushTransactionId(TimeStamp& aCompositeEnd)
mTxnStartTime = TimeStamp();
mFwdTime = TimeStamp();
#endif
uint64_t id = mPendingTransaction;
mPendingTransaction = 0;
TransactionId id = mPendingTransaction;
mPendingTransaction = TransactionId{0};
return id;
}

Expand Down
Loading

0 comments on commit ecb05b5

Please sign in to comment.