From 26f02a2001f65d7808d17e6f2ded4944187ebe32 Mon Sep 17 00:00:00 2001 From: trav90 Date: Sat, 18 Aug 2018 15:10:01 -0500 Subject: Avoid using memset on a not-trivial type like nsTabSizes nsTabSizes is non-trivial only because of the user-defined constructor. The idea desired here is certainly to zero all the members without listing them -- but the very act of doing so with a user-defined constructor, makes the idea impossible. Arguably this is something that is permissible in the language, and that the warning should be tailored to permit. I don't think this falls afoul of any of the issues flagged in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01527.html for example. In the meantime, just explicitly zeroing the three member fields is easy and fixes the warnings. --- layout/base/nsArenaMemoryStats.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'layout/base') diff --git a/layout/base/nsArenaMemoryStats.h b/layout/base/nsArenaMemoryStats.h index ba09baaa4..2a872cfe8 100644 --- a/layout/base/nsArenaMemoryStats.h +++ b/layout/base/nsArenaMemoryStats.h @@ -18,7 +18,12 @@ public: Other // Everything else. }; - nsTabSizes() { mozilla::PodZero(this); } + nsTabSizes() + : mDom(0) + , mStyle(0) + , mOther(0) + { + } void add(Kind kind, size_t n) { -- cgit v1.2.3 From ab961aeb54335fd07c66de2e3b8c3b6af6f89ea2 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 3 Sep 2018 10:11:38 +0200 Subject: Remove all C++ Telemetry Accumulation calls. This creates a number of stubs and leaves some surrounding code that may be irrelevant (eg. recorded time stamps, status variables). Stub resolution/removal should be a follow-up to this. --- layout/base/nsDisplayList.cpp | 6 ------ layout/base/nsDocumentViewer.cpp | 4 ---- layout/base/nsLayoutUtils.cpp | 4 ---- layout/base/nsPresShell.cpp | 40 ---------------------------------------- layout/base/nsRefreshDriver.cpp | 12 ------------ 5 files changed, 66 deletions(-) (limited to 'layout/base') diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index cb55bb2da..2bf20144a 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -7491,9 +7491,6 @@ PaintTelemetry::AutoRecordPaint::~AutoRecordPaint() double totalMs = (TimeStamp::Now() - mStart).ToMilliseconds(); - // Record the total time. - Telemetry::Accumulate(Telemetry::CONTENT_PAINT_TIME, static_cast(totalMs)); - // If the total time was >= 16ms, then it's likely we missed a frame due to // painting. In this case we'll gather some detailed metrics below. if (totalMs <= 16.0) { @@ -7504,9 +7501,6 @@ PaintTelemetry::AutoRecordPaint::~AutoRecordPaint() MOZ_ASSERT(aDurationMs <= totalMs); uint32_t amount = static_cast((aDurationMs / totalMs) * 100.0); - - nsDependentCString key(aKey); - Telemetry::Accumulate(Telemetry::CONTENT_LARGE_PAINT_PHASE_WEIGHT, key, amount); }; double dlMs = sMetrics[Metric::DisplayList]; diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 7b8734928..8b76227ff 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -1275,7 +1275,6 @@ nsDocumentViewer::PermitUnloadInternal(bool *aShouldPrompt, nsAutoSyncOperation sync(mDocument); mInPermitUnloadPrompt = true; - mozilla::Telemetry::Accumulate(mozilla::Telemetry::ONBEFOREUNLOAD_PROMPT_COUNT, 1); rv = prompt->ConfirmEx(title, message, buttonFlags, leaveLabel, stayLabel, nullptr, nullptr, &dummy, &buttonPressed); @@ -1290,15 +1289,12 @@ nsDocumentViewer::PermitUnloadInternal(bool *aShouldPrompt, // XXX: Are there other cases where prompts can abort? Is it ok to // prevent unloading the page in those cases? if (NS_FAILED(rv)) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::ONBEFOREUNLOAD_PROMPT_ACTION, 2); *aPermitUnload = false; return NS_OK; } // Button 0 == leave, button 1 == stay *aPermitUnload = (buttonPressed == 0); - mozilla::Telemetry::Accumulate(mozilla::Telemetry::ONBEFOREUNLOAD_PROMPT_ACTION, - (*aPermitUnload ? 1 : 0)); // If the user decided to go ahead, make sure not to prompt the user again // by toggling the internal prompting bool to false: if (*aPermitUnload) { diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 062fbfe15..d0f790a0b 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -3569,8 +3569,6 @@ nsLayoutUtils::PaintFrame(nsRenderingContext* aRenderingContext, nsIFrame* aFram } builder.LeavePresShell(aFrame, &list); - Telemetry::AccumulateTimeDelta(Telemetry::PAINT_BUILD_DISPLAYLIST_TIME, - startBuildDisplayList); bool profilerNeedsDisplayList = profiler_feature_active("displaylistdump"); bool consoleNeedsDisplayList = gfxUtils::DumpDisplayList() || gfxEnv::DumpPaint(); @@ -3651,8 +3649,6 @@ nsLayoutUtils::PaintFrame(nsRenderingContext* aRenderingContext, nsIFrame* aFram TimeStamp paintStart = TimeStamp::Now(); RefPtr layerManager = list.PaintRoot(&builder, aRenderingContext, flags); - Telemetry::AccumulateTimeDelta(Telemetry::PAINT_RASTERIZE_TIME, - paintStart); if (gfxPrefs::GfxLoggingPaintedPixelCountEnabled()) { TimeStamp now = TimeStamp::Now(); diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 3e5320c22..88539dc4a 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -1114,17 +1114,6 @@ PresShell::Destroy() if (mPresContext) { const bool mayFlushUserFontSet = false; gfxUserFontSet* fs = mPresContext->GetUserFontSet(mayFlushUserFontSet); - if (fs) { - uint32_t fontCount; - uint64_t fontSize; - fs->GetLoadStatistics(fontCount, fontSize); - Telemetry::Accumulate(Telemetry::WEBFONT_PER_PAGE, fontCount); - Telemetry::Accumulate(Telemetry::WEBFONT_SIZE_PER_PAGE, - uint32_t(fontSize/1024)); - } else { - Telemetry::Accumulate(Telemetry::WEBFONT_PER_PAGE, 0); - Telemetry::Accumulate(Telemetry::WEBFONT_SIZE_PER_PAGE, 0); - } } #ifdef MOZ_REFLOW_PERF @@ -2276,8 +2265,6 @@ PresShell::ScrollPage(bool aForward) nsIScrollableFrame* scrollFrame = GetFrameToScrollAsScrollable(nsIPresShell::eVertical); if (scrollFrame) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS, - (uint32_t) ScrollInputMethod::MainThreadScrollPage); scrollFrame->ScrollBy(nsIntPoint(0, aForward ? 1 : -1), nsIScrollableFrame::PAGES, nsIScrollableFrame::SMOOTH, @@ -2294,9 +2281,6 @@ PresShell::ScrollLine(bool aForward) nsIScrollableFrame* scrollFrame = GetFrameToScrollAsScrollable(nsIPresShell::eVertical); if (scrollFrame) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS, - (uint32_t) ScrollInputMethod::MainThreadScrollLine); - int32_t lineCount = Preferences::GetInt("toolkit.scrollbox.verticalScrollDistance", NS_DEFAULT_VERTICAL_SCROLL_DISTANCE); scrollFrame->ScrollBy(nsIntPoint(0, aForward ? lineCount : -lineCount), @@ -2315,8 +2299,6 @@ PresShell::ScrollCharacter(bool aRight) nsIScrollableFrame* scrollFrame = GetFrameToScrollAsScrollable(nsIPresShell::eHorizontal); if (scrollFrame) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS, - (uint32_t) ScrollInputMethod::MainThreadScrollCharacter); int32_t h = Preferences::GetInt("toolkit.scrollbox.horizontalScrollDistance", NS_DEFAULT_HORIZONTAL_SCROLL_DISTANCE); scrollFrame->ScrollBy(nsIntPoint(aRight ? h : -h, 0), @@ -2335,8 +2317,6 @@ PresShell::CompleteScroll(bool aForward) nsIScrollableFrame* scrollFrame = GetFrameToScrollAsScrollable(nsIPresShell::eVertical); if (scrollFrame) { - mozilla::Telemetry::Accumulate(mozilla::Telemetry::SCROLL_INPUT_METHODS, - (uint32_t) ScrollInputMethod::MainThreadCompleteScroll); scrollFrame->ScrollBy(nsIntPoint(0, aForward ? 1 : -1), nsIScrollableFrame::WHOLE, nsIScrollableFrame::SMOOTH, @@ -8231,16 +8211,6 @@ PresShell::HandleEventInternal(WidgetEvent* aEvent, } } - if (Telemetry::CanRecordBase() && - !aEvent->mTimeStamp.IsNull() && - aEvent->AsInputEvent()) { - double millis = (TimeStamp::Now() - aEvent->mTimeStamp).ToMilliseconds(); - Telemetry::Accumulate(Telemetry::INPUT_EVENT_RESPONSE_MS, millis); - if (mDocument && mDocument->GetReadyStateEnum() != nsIDocument::READYSTATE_COMPLETE) { - Telemetry::Accumulate(Telemetry::LOAD_INPUT_EVENT_RESPONSE_MS, millis); - } - } - return rv; } @@ -9522,16 +9492,6 @@ PresShell::ProcessReflowCommands(bool aInterruptible) UnsuppressAndInvalidate(); } - if (mDocument->GetRootElement()) { - TimeDuration elapsed = TimeStamp::Now() - timerStart; - int32_t intElapsed = int32_t(elapsed.ToMilliseconds()); - - if (intElapsed > NS_LONG_REFLOW_TIME_MS) { - Telemetry::Accumulate(Telemetry::LONG_REFLOW_INTERRUPTIBLE, - aInterruptible ? 1 : 0); - } - } - return !interrupted; } diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index 6676bea97..bc1a27852 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -526,10 +526,6 @@ private: if (XRE_IsParentProcess()) { TimeDuration vsyncLatency = TimeStamp::Now() - aVsyncTimestamp; uint32_t sample = (uint32_t)vsyncLatency.ToMilliseconds(); - Telemetry::Accumulate(Telemetry::FX_REFRESH_DRIVER_CHROME_FRAME_DELAY_MS, - sample); - Telemetry::Accumulate(Telemetry::FX_REFRESH_DRIVER_SYNC_SCROLL_FRAME_DELAY_MS, - sample); RecordJank(sample); } else if (mVsyncRate != TimeDuration::Forever()) { TimeDuration contentDelay = (TimeStamp::Now() - mLastChildTick) - mVsyncRate; @@ -539,10 +535,6 @@ private: contentDelay = TimeDuration::FromMilliseconds(0); } uint32_t sample = (uint32_t)contentDelay.ToMilliseconds(); - Telemetry::Accumulate(Telemetry::FX_REFRESH_DRIVER_CONTENT_FRAME_DELAY_MS, - sample); - Telemetry::Accumulate(Telemetry::FX_REFRESH_DRIVER_SYNC_SCROLL_FRAME_DELAY_MS, - sample); RecordJank(sample); } else { // Request the vsync rate from the parent process. Might be a few vsyncs @@ -2027,10 +2019,6 @@ nsRefreshDriver::Tick(int64_t aNowEpoch, TimeStamp aNowTime) notifyGC = true; } -#ifndef ANDROID /* bug 1142079 */ - mozilla::Telemetry::AccumulateTimeDelta(mozilla::Telemetry::REFRESH_DRIVER_TICK, mTickStart); -#endif - nsTObserverArray::ForwardIterator iter(mPostRefreshObservers); while (iter.HasMore()) { nsAPostRefreshObserver* observer = iter.GetNext(); -- cgit v1.2.3 From 93cae908bcbd063f21d5663a7d3149464af2ad20 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 4 Sep 2018 09:41:24 +0200 Subject: Remove all C++ telemetry autotimers --- layout/base/nsCSSRendering.cpp | 1 - layout/base/nsDocumentViewer.cpp | 10 ++-------- 2 files changed, 2 insertions(+), 9 deletions(-) (limited to 'layout/base') diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index 054632ad7..71ebfad22 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/layout/base/nsCSSRendering.cpp @@ -2516,7 +2516,6 @@ nsCSSRendering::PaintGradient(nsPresContext* aPresContext, PROFILER_LABEL("nsCSSRendering", "PaintGradient", js::ProfileEntry::Category::GRAPHICS); - Telemetry::AutoTimer gradientTimer; if (aDest.IsEmpty() || aFillArea.IsEmpty()) { return; } diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 8b76227ff..5478c61b0 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -1206,11 +1206,8 @@ nsDocumentViewer::PermitUnloadInternal(bool *aShouldPrompt, nsIDocument::PageUnloadingEventTimeStamp timestamp(mDocument); mInPermitUnload = true; - { - Telemetry::AutoTimer telemetryTimer; - EventDispatcher::DispatchDOMEvent(window, nullptr, event, mPresContext, + EventDispatcher::DispatchDOMEvent(window, nullptr, event, mPresContext, nullptr); - } mInPermitUnload = false; } @@ -1389,10 +1386,7 @@ nsDocumentViewer::PageHide(bool aIsUnload) nsIDocument::PageUnloadingEventTimeStamp timestamp(mDocument); - { - Telemetry::AutoTimer telemetryTimer; - EventDispatcher::Dispatch(window, mPresContext, &event, nullptr, &status); - } + EventDispatcher::Dispatch(window, mPresContext, &event, nullptr, &status); } #ifdef MOZ_XUL -- cgit v1.2.3 From 4c1f33b169b74dce2af414db30fcf113c4bf2a56 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 10 Sep 2018 22:12:07 +0200 Subject: Bug 1470260 - Part 1: Ensure that 'this' stays alive for the duration of the TickRefreshDriver call. --- layout/base/nsRefreshDriver.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'layout/base') diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index bc1a27852..8a62b517e 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -478,6 +478,9 @@ private: bool NotifyVsync(TimeStamp aVsyncTimestamp) override { + // IMPORTANT: All paths through this method MUST hold a strong ref on + // |this| for the duration of the TickRefreshDriver callback. + if (!NS_IsMainThread()) { MOZ_ASSERT(XRE_IsParentProcess()); // Compress vsync notifications such that only 1 may run at a time @@ -498,6 +501,7 @@ private: aVsyncTimestamp); NS_DispatchToMainThread(vsyncEvent); } else { + RefPtr kungFuDeathGrip(this); TickRefreshDriver(aVsyncTimestamp); } -- cgit v1.2.3 From 69b88dfccc38b9022e65bf459b199bdf4145ce67 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 10 Sep 2018 22:22:16 +0200 Subject: Bug 1470260 - Part 2: Make RefreshDriverTimer ref-counted and hold a strong ref on it on the stack when nsRefreshDriver::Tick can be reached. --- layout/base/nsRefreshDriver.cpp | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) (limited to 'layout/base') diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index 8a62b517e..b975a69dd 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -143,11 +143,7 @@ public: { } - virtual ~RefreshDriverTimer() - { - MOZ_ASSERT(mContentRefreshDrivers.Length() == 0, "Should have removed all content refresh drivers from here by now!"); - MOZ_ASSERT(mRootRefreshDrivers.Length() == 0, "Should have removed all root refresh drivers from here by now!"); - } + NS_INLINE_DECL_REFCOUNTING(RefreshDriverTimer) virtual void AddRefreshDriver(nsRefreshDriver* aDriver) { @@ -253,6 +249,12 @@ public: } protected: + virtual ~RefreshDriverTimer() + { + MOZ_ASSERT(mContentRefreshDrivers.Length() == 0, "Should have removed all content refresh drivers from here by now!"); + MOZ_ASSERT(mRootRefreshDrivers.Length() == 0, "Should have removed all root refresh drivers from here by now!"); + } + virtual void StartTimer() = 0; virtual void StopTimer() = 0; virtual void ScheduleNextTick(TimeStamp aNowTime) = 0; @@ -335,10 +337,11 @@ protected: nsTArray > mRootRefreshDrivers; // useful callback for nsITimer-based derived classes, here - // bacause of c++ protected shenanigans + // because of c++ protected shenanigans static void TimerTick(nsITimer* aTimer, void* aClosure) { - RefreshDriverTimer *timer = static_cast(aClosure); + RefPtr timer = + static_cast(aClosure); timer->Tick(); } }; @@ -459,9 +462,7 @@ public: private: // Since VsyncObservers are refCounted, but the RefreshDriverTimer are // explicitly shutdown. We create an inner class that has the VsyncObserver - // and is shutdown when the RefreshDriverTimer is deleted. The alternative is - // to (a) make all RefreshDriverTimer RefCounted or (b) use different - // VsyncObserver types. + // and is shutdown when the RefreshDriverTimer is deleted. class RefreshDriverVsyncObserver final : public VsyncObserver { public: @@ -576,7 +577,9 @@ private: // the scheduled TickRefreshDriver() runs. Check mVsyncRefreshDriverTimer // before use. if (mVsyncRefreshDriverTimer) { - mVsyncRefreshDriverTimer->RunRefreshDrivers(aVsyncTimestamp); + RefPtr timer = mVsyncRefreshDriverTimer; + timer->RunRefreshDrivers(aVsyncTimestamp); + // Note: mVsyncRefreshDriverTimer might be null now. } } @@ -829,7 +832,8 @@ protected: static void TimerTickOne(nsITimer* aTimer, void* aClosure) { - InactiveRefreshDriverTimer *timer = static_cast(aClosure); + RefPtr timer = + static_cast(aClosure); timer->TickOne(); } @@ -881,8 +885,8 @@ NS_IMPL_ISUPPORTS(VsyncChildCreateCallback, nsIIPCBackgroundChildCreateCallback) } // namespace mozilla -static RefreshDriverTimer* sRegularRateTimer; -static InactiveRefreshDriverTimer* sThrottledRateTimer; +static StaticRefPtr sRegularRateTimer; +static StaticRefPtr sThrottledRateTimer; #ifdef XP_WIN static int32_t sHighPrecisionTimerRequests = 0; @@ -964,8 +968,6 @@ GetFirstFrameDelay(imgIRequest* req) nsRefreshDriver::Shutdown() { // clean up our timers - delete sRegularRateTimer; - delete sThrottledRateTimer; sRegularRateTimer = nullptr; sThrottledRateTimer = nullptr; @@ -2229,16 +2231,15 @@ nsRefreshDriver::PVsyncActorCreated(VsyncChild* aVsyncChild) { MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(!XRE_IsParentProcess()); - auto* vsyncRefreshDriverTimer = - new VsyncRefreshDriverTimer(aVsyncChild); + RefPtr vsyncRefreshDriverTimer = + new VsyncRefreshDriverTimer(aVsyncChild); // If we are using software timer, swap current timer to // VsyncRefreshDriverTimer. if (sRegularRateTimer) { sRegularRateTimer->SwapRefreshDrivers(vsyncRefreshDriverTimer); - delete sRegularRateTimer; } - sRegularRateTimer = vsyncRefreshDriverTimer; + sRegularRateTimer = vsyncRefreshDriverTimer.forget(); } void -- cgit v1.2.3