1200119 - Add a way to create usable markers from different threads

Status: RESOLVED FIXED

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

Description

[Victor Porof [:vporof][:vp]]

We're able to instantiate them, but that's useless. Need a way to tie them to an ObservedDocShell or TimelineConsumers.

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: v1

WIP, testing, probably finished

Comment 2

[Victor Porof [:vporof][:vp]]

Attachment: v2

We're going to have to update the docs after this.

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8659244 [details] [diff] [review]
v2

Review of attachment 8659244 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

::: docshell/base/timeline/AbstractTimelineMarker.cpp
@@ +31,5 @@
> +UniquePtr<AbstractTimelineMarker>
> +AbstractTimelineMarker::Clone()
> +{
> +  MOZ_ASSERT(false, "Clone method not yet implemented on this marker type.");
> +  return nullptr;

This is the only code that I am less than certain about.  Without requiring all subclasses to implement Clone, it means that in the future it would be possible for some other erroneous change to wind up calling this function, causing a crash.  I wonder if you considered just making it "= 0" and fixing up the subclass(es) instead?

That said there's obviously no danger now.

Comment 4

[Victor Porof [:vporof][:vp]]

That was intentional. Not all markers need to be clonable, as most of them will only be added from the main thread. Whenever there's a need to add a marker off the main thread, one'll hit that crash and obviously see that the method needs to be implemented.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/0d9327e758e5

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0d9327e758e5