Closed Bug 1183229 Opened 7 years ago Closed 7 years ago

Add a way to count the number of timeline-observed docshells outside of nsDocShell

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect)

defect
Not set
normal

Tracking

(firefox42 fixed)

RESOLVED FIXED
Firefox 42
Tracking Status
firefox42 --- fixed

People

(Reporter: vporof, Assigned: vporof)

References

Details

Attachments

(2 files)

No description provided.
Summary: Add a way to count the number of timeline-observed docshells outside of the docshell header and implementation → Add a way to count the number of timeline-observed docshells outside of nsDocShell
Attached patch v1Splinter Review
TimelineConsumers looks very empty now, but it will end up containing all the docshell-specific marker data structures and logic.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Attachment #8634421 - Flags: review?(ttromey)
Attachment #8634421 - Flags: review?(ttromey) → review?(bugs)
Comment on attachment 8634421 [details] [diff] [review]
v1

># HG changeset patch
># User Victor Porof <vporof@mozilla.com>
># Date 1436998007 14400
>#      Wed Jul 15 18:06:47 2015 -0400
># Node ID aeea07206837241d81a487c739399d44d84ee24f
># Parent  6c364aa1cb619332d39941920150f14b9d8a95e4
>Bug 1183229 - Add a way to count the number of timeline-observed docshells outside of nsDocShell, r=tromey
>
>diff --git a/docshell/base/moz.build b/docshell/base/moz.build
>--- a/docshell/base/moz.build
>+++ b/docshell/base/moz.build
>@@ -45,8 +45,9 @@ EXPORTS.mozilla += [
>     'IHistory.h',
>     'LoadContext.h',
>     'timeline/AutoGlobalTimelineMarker.h',
>     'timeline/AutoTimelineMarker.h',
>+    'timeline/TimelineConsumers.h',
> ]
> 
> UNIFIED_SOURCES += [
>     'LoadContext.cpp',
>@@ -62,8 +63,9 @@ UNIFIED_SOURCES += [
>     'nsWebNavigationInfo.cpp',
>     'SerializedLoadContext.cpp',
>     'timeline/AutoGlobalTimelineMarker.cpp',
>     'timeline/AutoTimelineMarker.cpp',
>+    'timeline/TimelineConsumers.cpp',
>     'timeline/TimelineMarker.cpp',
> ]
This stuff will change a bit after the patch which adds timeline is updated

>+class TimelineConsumers {
{ goes to its own line.

>+private:
>+  // Counter for how many timelines are currently interested in markers.
>+  static unsigned long activeConsumers;
static member variables should be in form sFooBar, 
so, sActiveConsumers
Attachment #8634421 - Flags: review?(bugs) → review+
Comment on attachment 8634421 [details] [diff] [review]
v1

>+++ b/docshell/base/timeline/TimelineConsumers.h
>@@ -0,0 +1,31 @@
>+/* -*- 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 TimelineConsumers_h__
>+#define TimelineConsumers_h__

mozilla_TimelineConsumers_h_
Attached patch v2Splinter Review
Addressed comments
Attachment #8635464 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/080a2d9acd71
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
This was backed out in: https://hg.mozilla.org/integration/fx-team/rev/fdff9c45b9c9
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/a6cdbd61e6d2
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.