Bug 1371457 - Change restyle marker data to animation state. r=bholley,gregtatum draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 28 Jun 2017 11:59:28 -0700
changeset 602210 97656f10f36bf8abd72134da4a5228959c2141c8
parent 601255 cc903e3f61894e60c3b0efaf05e9a446d1d85888
child 602211 c57dc5e153e2230890bc285a5c7ec3a7b80ebbff
push id66342
push userbmo:jryans@gmail.com
push dateThu, 29 Jun 2017 21:00:49 +0000
reviewersbholley, gregtatum
bugs1371457
milestone56.0a1
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
devtools/client/locales/en-US/markers.properties
devtools/client/performance/docs/markers.md
devtools/client/performance/modules/marker-formatters.js
devtools/client/performance/test/browser_perf-marker-details.js
devtools/server/tests/browser/browser_markers-styles.js
docshell/base/timeline/AutoRestyleTimelineMarker.cpp
docshell/base/timeline/AutoRestyleTimelineMarker.h
docshell/base/timeline/RestyleTimelineMarker.h
docshell/base/timeline/moz.build
dom/webidl/ProfileTimelineMarker.webidl
layout/base/RestyleTracker.cpp
--- a/devtools/client/locales/en-US/markers.properties
+++ b/devtools/client/locales/en-US/markers.properties
@@ -75,17 +75,17 @@ marker.field.consoleTimerName=Timer Name
 # For DOM Event markers
 marker.field.DOMEventType=Event Type:
 marker.field.DOMEventPhase=Phase:
 
 # Non-incremental cause for a Garbage Collection marker
 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
 marker.worker.serializeDataOnMainThread=Serialize data on the main thread
 marker.worker.deserializeDataOffMainThread=Deserialize data in Worker
 marker.worker.deserializeDataOnMainThread=Deserialize data on the main thread
 
 # The type of operation performed by a MessagePort
--- a/devtools/client/performance/docs/markers.md
+++ b/devtools/client/performance/docs/markers.md
@@ -38,33 +38,16 @@ elements. Fired via `PresShell::DoReflow
   of rectangle objects indicating where painting has occurred.
 
 ## Styles
 
 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,
 with a reason that triggered it (causeName), like a requestAnimationFrame or
 a setTimeout.
 
 * string causeName - The reason that JS was entered. There are many possible
   reasons, and the interesting ones to show web developers (triggered by content) are:
--- a/devtools/client/performance/modules/marker-formatters.js
+++ b/devtools/client/performance/modules/marker-formatters.js
@@ -43,20 +43,19 @@ exports.Formatters = {
    */
   UnknownLabel: function (marker = {}) {
     return marker.name || L10N.getStr("marker.label.unknown");
   },
 
   /* 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;
   },
 
   /* Group 1 - JS */
 
   DOMEventFields: function (marker) {
--- a/devtools/client/performance/test/browser_perf-marker-details.js
+++ b/devtools/client/performance/test/browser_perf-marker-details.js
@@ -82,19 +82,16 @@ function* spawnTest() {
     TimeStamp: function (marker) {
       info("Got `TimeStamp` marker with data: " + JSON.stringify(marker));
       shouldHaveLabel($, "Label:", "go", marker);
       shouldHaveStack($, "stack", marker);
       return true;
     },
     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;
       }
     },
     Reflow: function (marker) {
       info("Got `Reflow` marker with data: " + JSON.stringify(marker));
       if (marker.stack) {
--- a/devtools/server/tests/browser/browser_markers-styles.js
+++ b/devtools/server/tests/browser/browser_markers-styles.js
@@ -14,21 +14,17 @@ add_task(function* () {
 
   initDebuggerServer();
   let client = new DebuggerClient(DebuggerServer.connectPipe());
   let form = yield connectDebuggerClient(client);
   let front = PerformanceFront(client, form);
   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();
 });
new file mode 100644
--- /dev/null
+++ b/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
new file mode 100644
--- /dev/null
+++ b/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_ */
--- a/docshell/base/timeline/RestyleTimelineMarker.h
+++ b/docshell/base/timeline/RestyleTimelineMarker.h
@@ -10,33 +10,31 @@
 #include "TimelineMarker.h"
 #include "mozilla/dom/ProfileTimelineMarkerBinding.h"
 
 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
   {
     TimelineMarker::AddDetails(aCx, aMarker);
 
     if (GetTracingType() == MarkerTracingType::START) {
-      aMarker.mRestyleHint.Construct(mRestyleHint);
+      aMarker.mIsAnimationOnly.Construct(mIsAnimationOnly);
     }
   }
 
 private:
-  nsString mRestyleHint;
+  bool mIsAnimationOnly;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_RestyleTimelineMarker_h_
--- a/docshell/base/timeline/moz.build
+++ b/docshell/base/timeline/moz.build
@@ -5,16 +5,17 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 with Files('**'):
     BUG_COMPONENT = ('Firefox', 'Developer Tools: Performance Tools (Profiler/Timeline)')
 
 EXPORTS.mozilla += [
     'AbstractTimelineMarker.h',
     'AutoGlobalTimelineMarker.h',
+    'AutoRestyleTimelineMarker.h',
     'AutoTimelineMarker.h',
     'CompositeTimelineMarker.h',
     'ConsoleTimelineMarker.h',
     'DocLoadingTimelineMarker.h',
     'EventTimelineMarker.h',
     'JavascriptTimelineMarker.h',
     'LayerTimelineMarker.h',
     'MarkersStorage.h',
@@ -26,16 +27,17 @@ EXPORTS.mozilla += [
     'TimelineMarkerEnums.h',
     'TimestampTimelineMarker.h',
     'WorkerTimelineMarker.h',
 ]
 
 UNIFIED_SOURCES += [
     'AbstractTimelineMarker.cpp',
     'AutoGlobalTimelineMarker.cpp',
+    'AutoRestyleTimelineMarker.cpp',
     'AutoTimelineMarker.cpp',
     'MarkersStorage.cpp',
     'ObservedDocShell.cpp',
     'TimelineConsumers.cpp',
     'TimelineMarker.cpp',
 ]
 
 FINAL_LIBRARY = 'xul'
--- a/dom/webidl/ProfileTimelineMarker.webidl
+++ b/dom/webidl/ProfileTimelineMarker.webidl
@@ -59,16 +59,16 @@ dictionary ProfileTimelineMarker {
   /* For document::DOMContentLoaded and document::Load markers. Using this
    * instead of the `start` and `end` timestamps is strongly discouraged. */
   unsigned long long unixTime; // in microseconds
 
   /* For Paint markers.  */
   sequence<ProfileTimelineLayerRect> rectangles;
 
   /* For Style markers. */
-  DOMString restyleHint;
+  boolean isAnimationOnly;
 
   /* For MessagePort markers. */
   ProfileTimelineMessagePortOperationType messagePortOperation;
 
   /* For Worker markers. */
   ProfileTimelineWorkerOperationType workerOperation;
 };
--- a/layout/base/RestyleTracker.cpp
+++ b/layout/base/RestyleTracker.cpp
@@ -6,24 +6,23 @@
 /**
  * A class which manages pending restyles.  This handles keeping track
  * of what nodes restyles need to happen on and so forth.
  */
 
 #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 {
 
 #ifdef RESTYLE_LOGGING
 static nsCString
 GetDocumentURI(nsIDocument* aDocument)
 {
   nsCString result;
@@ -112,20 +111,16 @@ RestyleTracker::DoProcessRestyles()
     nsIURI *uri = Document()->GetDocumentURI();
     if (uri) {
       docURL = uri->GetSpecOrDefault();
     }
   }
   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.
   RestyleManager::AnimationsWithDestroyedFrame
     animationsWithDestroyedFrame(mRestyleManager);
 
   // Create a ReframingStyleContexts struct on the stack and put it in our
   // mReframingStyleContexts for almost all of the remaining scope of
@@ -244,34 +239,27 @@ RestyleTracker::DoProcessRestyles()
         }
 
         nsAutoPtr<RestyleData> data;
         if (!GetRestyleData(element, data)) {
           LOG_RESTYLE("skipping, already restyled");
           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);
         }
       }
 
       if (mHaveLaterSiblingRestyles) {
         // Keep processing restyles for now
         continue;
       }
 
@@ -358,31 +346,25 @@ RestyleTracker::DoProcessRestyles()
                       FrameTagToString(currentRestyle->mElement).get(),
                       index++, count);
           LOG_RESTYLE_INDENT();
 
           Maybe<AutoProfilerTracing> tracing;
           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);
           }
         }
       }
     }
   }
 
   // mPendingRestyles is now empty.
   mHaveSelectors = false;