1305580 - Count chrome and content callers of XMLDocument's load() method separately in telemetry

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Actually, we may want to separately count system and non-system callers...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Oh, we already have something here, because it's a DeprecatedOperation and we WarnOnceAbout.

Looks like this only produces a "page" counter, not a "document" counter?  What is the difference between the two?

Am I right that <https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2016-08-23&keys=__none__!__none__!__none__&max_channel_version=release%252F48&measure=USE_COUNTER2_DEPRECATED_UseOfDOM3LoadMethod_PAGE&min_channel_version=null&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2016-07-26&table=0&trim=1&use_submission_date=0> shows that 2.65% of pageloads called this method in Firefox 48 release?

I guess we could try changing to separate content/chrome deprecated operations here and see what the split looks like...

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #1)
> Looks like this only produces a "page" counter, not a "document" counter? 
> What is the difference between the two?

"page" is roughly "anything in a top-level tab and sub-documents thereof" whereas "document" is individual pages without considering sub-documents.

> Am I right that
> <https://telemetry.mozilla.org/new-pipeline/dist.html#!
> cumulative=0&end_date=2016-08-23&keys=__none__!__none__!
> __none__&max_channel_version=release%252F48&measure=USE_COUNTER2_DEPRECATED_U
> seOfDOM3LoadMethod_PAGE&min_channel_version=null&product=Firefox&sanitize=1&s
> ort_keys=submissions&start_date=2016-07-
> 26&table=0&trim=1&use_submission_date=0> shows that 2.65% of pageloads
> called this method in Firefox 48 release?

That looks like a correct interpretation to me.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Remove all the methods from nsIDOMXMLDocument and make it not inherit from nsIDOMDocument anymore

This will let us know for sure that we're being called from script in things
like nsXMLDocument::Load.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Change nsXMLDocument::Load telemetry to separately count chrome and content callers

Comment 5

[Boris Zbarsky [:bzbarsky]]

I'm hoping beyond hope that most of our callers are chrome and hence we can make this chromeonly, but let's try measuring....

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8795797 [details] [diff] [review]
part 2.  Change nsXMLDocument::Load telemetry to separately count chrome and content callers

Telemetry data may look a bit weird for this stuff for awhile, but I guess that is fine. I mean, it collects currently a + b, and after this patch a and b.


And WarnOnceAbout after principal check is fine, since if we throw because of principal check, it would be fine to throw because of missing method too.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/521286426a93
part 1.  Remove all the methods from nsIDOMXMLDocument and make it not inherit from nsIDOMDocument anymore.  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/1124e3458403
part 2.  Change nsXMLDocument::Load telemetry to separately count chrome and content callers.  r=smaug

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I backed this out for Windows tsvgr failures like https://treeherder.mozilla.org/logviewer.html#?job_id=36655072&repo=mozilla-inbound

Philor says this also caused some other crashes like https://treeherder.mozilla.org/logviewer.html#?job_id=36659239&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/696adef2e21a

Comment 9

[Boris Zbarsky [:bzbarsky]]

Um... I bet the problem is that I didn't end up inheriting nsIDOMXMLDocument from nsISupports.  At least given where that QI failure is.

I guess I'll do a try run of the relevant tests...

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/22f757d49ef3
part 1.  Remove all the methods from nsIDOMXMLDocument and make it not inherit from nsIDOMDocument anymore.  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7c4ba6cbf07
part 2.  Change nsXMLDocument::Load telemetry to separately count chrome and content callers.  r=smaug

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/22f757d49ef3
https://hg.mozilla.org/mozilla-central/rev/e7c4ba6cbf07

Comment 12

[Boris Zbarsky [:bzbarsky]]

Bug 1310275 filed on both use counters being pretty bogus.