1310275 - XMLDocument.load warnings/telemetry are useless

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

We're calling WarnOnceAbout on the XMLDocument itself.  In the "normal" load() use case, this won't be a document loaded in a docshell, so won't have a windowid (so warning won't be visible) or get telemetry accumulated for it.

We should warn on the entry doc instead, if we have one.

This still won't help load() calls from JSMs, so we might have no way of knowing whether we have chrome callers into this stuff.  :(

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix the document.load() warning and telemetry to have a shot at actually being noticed

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8801188 [details] [diff] [review]
Fix the document.load() warning and telemetry to have a shot at actually being noticed

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

r=me, I guess, with the below change--assuming I'm not missing something.

::: dom/xml/XMLDocument.cpp
@@ +292,5 @@
>    }
>  
> +  // Reporting a warning on ourselves is rather pointless, because we probably
> +  // have no window id and probably aren't considered a "content document"
> +  // because we're not loaded in a docshell, so won't accumulate telemetry for

Just to be clear, this fix is really only for the telemetry, right?  As I read WarnOnceAbort, there's no discussion of window IDs and content documents there.  So this comment should be amended, along with the commit message?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Just to be clear, this fix is really only for the telemetry, right? 

No.

> As I read WarnOnceAbort, there's no discussion of window IDs and content documents there.

The window ID part is key for WarnOnceAbout.  Not having one means the console log doesn't show up in web console, because we don't know which web console to show it in.  I'll make the comments clearer.

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #3)
> > As I read WarnOnceAbort, there's no discussion of window IDs and content documents there.
> 
> The window ID part is key for WarnOnceAbout.  Not having one means the
> console log doesn't show up in web console, because we don't know which web
> console to show it in.  I'll make the comments clearer.

Ah, thanks for the explanation; I didn't look closely enough.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/21ba15fa0c44
Fix the document.load() warning and telemetry to have a shot at actually being noticed.  r=froydnj

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/21ba15fa0c44