Bug 1371457 - Change restyle marker data to animation state. r=bholley,gregtatum

Stylo doesn't have a good equivalent for restyle hints to expose in markers and
the ones exposed for Gecko aren't very accurate either, so we don't want to
expose the restyle hint anymore.

At the same time, several animation restyle tests currently use the hint inside
the marker to check when animation-only restyles have happened.  We can preserve
this by changing the data inside the marker to be a flag for whether the restyle
is animation only, which we know for both Gecko and Stylo.

MozReview-Commit-ID: 8L8KU8Ush7P

--HG--
extra : rebase_source : 4eef80653c1ef79ee1539d27fe6a70fbfaf441ad

devtools/client/locales/en-US/markers.properties

@@ -80,7 +80,7 @@ marker.field.DOMEventPhase=Phase:
 marker.field.nonIncrementalCause=Non-incremental Cause:
 
 # For "Recalculate Style" markers
-marker.field.restyleHint=Restyle Hint:
+marker.field.isAnimationOnly=Animation Only:
 
 # The type of operation performed by a Worker.
 marker.worker.serializeDataOffMainThread=Serialize data in Worker

devtools/client/performance/docs/markers.md

@@ -43,23 +43,6 @@ Style markers (labeled as "Recalculating Styles") are triggered when Gecko
 needs to figure out the computational style of an element. Fired via
 `RestyleTracker::DoProcessRestyles` when there are elements to restyle.
 
-* DOMString restyleHint - A string indicating what kind of restyling will need
-  to be processed; for example "eRestyle_StyleAttribute" is relatively cheap,
-  whereas "eRestyle_Subtree" is more expensive. The hint can be a string of
-  any amount of the following, separated via " | ". All future restyleHints
-  are from `RestyleManager::RestyleHintToString`.
-
-  * "eRestyle_Self"
-  * "eRestyle_Subtree"
-  * "eRestyle_LaterSiblings"
-  * "eRestyle_CSSTransitions"
-  * "eRestyle_CSSAnimations"
-  * "eRestyle_StyleAttribute"
-  * "eRestyle_StyleAttribute_Animations"
-  * "eRestyle_Force"
-  * "eRestyle_ForceDescendants"
-
-
 ## Javascript
 
 `Javascript` markers are emitted indicating when JS execution begins and ends,

devtools/client/performance/modules/marker-formatters.js

@@ -48,10 +48,9 @@ exports.Formatters = {
   /* Group 0 - Reflow and Rendering pipeline */
 
   StylesFields: function (marker) {
-    if ("restyleHint" in marker) {
-      let label = marker.restyleHint.replace(/eRestyle_/g, "");
+    if ("isAnimationOnly" in marker) {
       return {
-        [L10N.getStr("marker.field.restyleHint")]: label
+        [L10N.getStr("marker.field.isAnimationOnly")]: marker.isAnimationOnly
       };
     }
     return null;

devtools/client/performance/test/browser_perf-marker-details.js

@@ -87,9 +87,6 @@ function* spawnTest() {
     },
     Styles: function (marker) {
       info("Got `Styles` marker with data: " + JSON.stringify(marker));
-      if (marker.restyleHint) {
-        shouldHaveLabel($, "Restyle Hint:", marker.restyleHint.replace(/eRestyle_/g, ""), marker);
-      }
       if (marker.stack) {
         shouldHaveStack($, "stack", marker);
         return true;

devtools/server/tests/browser/browser_markers-styles.js

@@ -19,15 +19,11 @@ add_task(function* () {
   yield front.connect();
   let rec = yield front.startRecording({ withMarkers: true });
 
-  let markers = yield waitForMarkerType(front, MARKER_NAME, function (marker) {
-    return marker.some(({restyleHint}) => restyleHint != void 0);
-  });
+  let markers = yield waitForMarkerType(front, MARKER_NAME);
 
   yield front.stopRecording(rec);
 
   ok(markers.some(m => m.name === MARKER_NAME), `got some ${MARKER_NAME} markers`);
-  ok(markers.some(({restyleHint}) => restyleHint != void 0),
-    "Some markers have a restyleHint.");
 
   yield client.close();
   gBrowser.removeCurrentTab();

docshell/base/timeline/AutoRestyleTimelineMarker.cpp

@@ -0,0 +1,60 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* 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 "AutoRestyleTimelineMarker.h"
+
+#include "TimelineConsumers.h"
+#include "MainThreadUtils.h"
+#include "RestyleTimelineMarker.h"
+
+namespace mozilla {
+
+AutoRestyleTimelineMarker::AutoRestyleTimelineMarker(
+    nsIDocShell* aDocShell,
+    bool aIsAnimationOnly
+    MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
+  : mDocShell(nullptr)
+  , mIsAnimationOnly(aIsAnimationOnly)
+{
+  MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (!aDocShell) {
+    return;
+  }
+
+  RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();
+  if (!timelines || !timelines->HasConsumer(aDocShell)) {
+    return;
+  }
+
+  mDocShell = aDocShell;
+  timelines->AddMarkerForDocShell(mDocShell, Move(
+    MakeUnique<RestyleTimelineMarker>(
+      mIsAnimationOnly,
+      MarkerTracingType::START)));
+}
+
+AutoRestyleTimelineMarker::~AutoRestyleTimelineMarker()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (!mDocShell) {
+    return;
+  }
+
+  RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();
+  if (!timelines || !timelines->HasConsumer(mDocShell)) {
+    return;
+  }
+
+  timelines->AddMarkerForDocShell(mDocShell, Move(
+    MakeUnique<RestyleTimelineMarker>(
+      mIsAnimationOnly,
+      MarkerTracingType::END)));
+}
+
+} // namespace mozilla

docshell/base/timeline/AutoRestyleTimelineMarker.h

@@ -0,0 +1,36 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* 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 mozilla_AutoRestyleTimelineMarker_h_
+#define mozilla_AutoRestyleTimelineMarker_h_
+
+#include "mozilla/GuardObjects.h"
+#include "mozilla/RefPtr.h"
+
+class nsIDocShell;
+
+namespace mozilla {
+
+class MOZ_RAII AutoRestyleTimelineMarker
+{
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER;
+
+  RefPtr<nsIDocShell> mDocShell;
+  bool mIsAnimationOnly;
+
+public:
+  AutoRestyleTimelineMarker(nsIDocShell* aDocShell,
+                            bool aIsAnimationOnly
+                            MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
+  ~AutoRestyleTimelineMarker();
+
+  AutoRestyleTimelineMarker(const AutoRestyleTimelineMarker& aOther) = delete;
+  void operator=(const AutoRestyleTimelineMarker& aOther) = delete;
+};
+
+} // namespace mozilla
+
+#endif /* mozilla_AutoRestyleTimelineMarker_h_ */

docshell/base/timeline/RestyleTimelineMarker.h

@@ -15,13 +15,11 @@ namespace mozilla {
 class RestyleTimelineMarker : public TimelineMarker
 {
 public:
-  RestyleTimelineMarker(nsRestyleHint aRestyleHint,
+  RestyleTimelineMarker(bool aIsAnimationOnly,
                         MarkerTracingType aTracingType)
     : TimelineMarker("Styles", aTracingType)
   {
-    if (aRestyleHint) {
-      mRestyleHint.AssignWithConversion(GeckoRestyleManager::RestyleHintToString(aRestyleHint));
-    }
+    mIsAnimationOnly = aIsAnimationOnly;
   }
 
   virtual void AddDetails(JSContext* aCx, dom::ProfileTimelineMarker& aMarker) override
@@ -29,12 +27,12 @@ public:
     TimelineMarker::AddDetails(aCx, aMarker);
 
     if (GetTracingType() == MarkerTracingType::START) {
-      aMarker.mRestyleHint.Construct(mRestyleHint);
+      aMarker.mIsAnimationOnly.Construct(mIsAnimationOnly);
     }
   }
 
 private:
-  nsString mRestyleHint;
+  bool mIsAnimationOnly;
 };
 
 } // namespace mozilla

docshell/base/timeline/moz.build

@@ -10,6 +10,7 @@ with Files('**'):
 EXPORTS.mozilla += [
     'AbstractTimelineMarker.h',
     'AutoGlobalTimelineMarker.h',
+    'AutoRestyleTimelineMarker.h',
     'AutoTimelineMarker.h',
     'CompositeTimelineMarker.h',
     'ConsoleTimelineMarker.h',
@@ -31,6 +32,7 @@ EXPORTS.mozilla += [
 UNIFIED_SOURCES += [
     'AbstractTimelineMarker.cpp',
     'AutoGlobalTimelineMarker.cpp',
+    'AutoRestyleTimelineMarker.cpp',
     'AutoTimelineMarker.cpp',
     'MarkersStorage.cpp',
     'ObservedDocShell.cpp',

dom/webidl/ProfileTimelineMarker.webidl

@@ -64,7 +64,7 @@ dictionary ProfileTimelineMarker {
   sequence<ProfileTimelineLayerRect> rectangles;
 
   /* For Style markers. */
-  DOMString restyleHint;
+  boolean isAnimationOnly;
 
   /* For MessagePort markers. */
   ProfileTimelineMessagePortOperationType messagePortOperation;

layout/base/RestyleTracker.cpp

@@ -11,14 +11,13 @@
 #include "RestyleTracker.h"
 
 #include "GeckoProfiler.h"
-#include "nsDocShell.h"
 #include "nsFrameManager.h"
 #include "nsIDocument.h"
 #include "nsStyleChangeList.h"
 #include "mozilla/GeckoRestyleManager.h"
 #include "RestyleTrackerInlines.h"
 #include "nsTransitionManager.h"
-#include "mozilla/RestyleTimelineMarker.h"
+#include "mozilla/AutoRestyleTimelineMarker.h"
 
 namespace mozilla {
 
@@ -117,10 +116,6 @@ RestyleTracker::DoProcessRestyles()
   AUTO_PROFILER_LABEL_DYNAMIC("RestyleTracker::DoProcessRestyles", CSS,
                               docURL.get());
 
-  nsDocShell* docShell = static_cast<nsDocShell*>(mRestyleManager->PresContext()->GetDocShell());
-  RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();
-  bool isTimelineRecording = timelines && timelines->HasConsumer(docShell);
-
   // Create a AnimationsWithDestroyedFrame during restyling process to
   // stop animations and transitions on elements that have no frame at the end
   // of the restyling process.
@@ -249,24 +244,17 @@ RestyleTracker::DoProcessRestyles()
           continue;
         }
 
-        if (isTimelineRecording) {
-          timelines->AddMarkerForDocShell(docShell, Move(
-            MakeUnique<RestyleTimelineMarker>(
-              data->mRestyleHint, MarkerTracingType::START)));
-        }
-
-        Maybe<AutoProfilerTracing> tracing;
-        if (profiler_feature_active(ProfilerFeature::Restyle)) {
-          tracing.emplace("Paint", "Styles", Move(data->mBacktrace));
-        }
-        ProcessOneRestyle(element, data->mRestyleHint, data->mChangeHint,
-                          data->mRestyleHintData);
-        AddRestyleRootsIfAwaitingRestyle(data->mDescendants);
-
-        if (isTimelineRecording) {
-          timelines->AddMarkerForDocShell(docShell, Move(
-            MakeUnique<RestyleTimelineMarker>(
-              data->mRestyleHint, MarkerTracingType::END)));
+        {
+          AutoRestyleTimelineMarker marker(
+            mRestyleManager->PresContext()->GetDocShell(),
+            data->mRestyleHint & eRestyle_AllHintsWithAnimations);
+          Maybe<AutoProfilerTracing> tracing;
+          if (profiler_feature_active(ProfilerFeature::Restyle)) {
+            tracing.emplace("Paint", "Styles", Move(data->mBacktrace));
+          }
+          ProcessOneRestyle(element, data->mRestyleHint, data->mChangeHint,
+                            data->mRestyleHintData);
+          AddRestyleRootsIfAwaitingRestyle(data->mDescendants);
         }
       }
 
@@ -363,21 +351,15 @@ RestyleTracker::DoProcessRestyles()
           if (profiler_feature_active(ProfilerFeature::Restyle)) {
             tracing.emplace("Paint", "Styles", Move(currentRestyle->mBacktrace));
           }
-          if (isTimelineRecording) {
-            timelines->AddMarkerForDocShell(docShell, Move(
-              MakeUnique<RestyleTimelineMarker>(
-                currentRestyle->mRestyleHint, MarkerTracingType::START)));
-          }
 
-          ProcessOneRestyle(currentRestyle->mElement,
-                            currentRestyle->mRestyleHint,
-                            currentRestyle->mChangeHint,
-                            currentRestyle->mRestyleHintData);
-
-          if (isTimelineRecording) {
-            timelines->AddMarkerForDocShell(docShell, Move(
-              MakeUnique<RestyleTimelineMarker>(
-                currentRestyle->mRestyleHint, MarkerTracingType::END)));
+          {
+            AutoRestyleTimelineMarker marker(
+              mRestyleManager->PresContext()->GetDocShell(),
+              currentRestyle->mRestyleHint & eRestyle_AllHintsWithAnimations);
+            ProcessOneRestyle(currentRestyle->mElement,
+                              currentRestyle->mRestyleHint,
+                              currentRestyle->mChangeHint,
+                              currentRestyle->mRestyleHintData);
           }
         }
       }