It's just an enum, with some fields we don't even need. There's no reason to include/reuse it everywhere.

Comment on attachment 8649955 [details] [diff] [review] v1 Review of attachment 8649955 [details] [diff] [review]: ----------------------------------------------------------------- The patch overall looks good. However I am going to r- it for now because it is missing a crucial file. From the way the code is written I'm going to guess that the new enum is not an "enum class". I think the class form is generally preferable in new code because it introduces a new name space and thus avoids possible name clashes. So I would recommend this; though unfortunately it means another pass through all the files. Another equivalent approach would be to make the non-class enum a member of TimelineMarker. Also, could you see if nsDocShell.h can now drop the GeckoProfiler.h include? I vaguely recall that the enum was the only reason for this, but I'm not certain. ::: docshell/base/timeline/TimelineMarker.h @@ +8,5 @@ > #define mozilla_TimelineMarker_h_ > > #include "nsString.h" > #include "nsContentUtils.h" > +#include "TimelineMarkerEnums.h" This file is missing from the patch.

> > ::: docshell/base/timeline/TimelineMarker.h > @@ +8,5 @@ > > #define mozilla_TimelineMarker_h_ > > > > #include "nsString.h" > > #include "nsContentUtils.h" > > +#include "TimelineMarkerEnums.h" > > This file is missing from the patch. OOOPS

Comment on attachment 8649955 [details] [diff] [review] v1 Mechanical change.

Comment on attachment 8649974 [details] [diff] [review] v2 Review of attachment 8649974 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks. ::: dom/base/Console.cpp @@ +1105,5 @@ > nsAutoJSString key; > if (key.init(aCx, jsString)) { > mozilla::UniquePtr<TimelineMarker> marker = > + MakeUnique<ConsoleTimelineMarker>(key, aMethodName == MethodTime > + ? MarkerTracingType::START Splinter shows some trailing whitespace on these two lines.