The default bug view has changed. See this FAQ.

Tons of "NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome()) failed: file content/base/src/nsImageLoadingContent.cpp" when sorting a file list

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla17
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

5 years ago
STR:
 1. Visit ftp://ftp.mozilla.org/pub/mozilla.org/artwork/ in a debug mozilla-central build

 2. Click "Name" or "Last Modified" to sort by those fields

ACTUAL RESULTS:
A bunch of spam like this to your console:
WARNING: NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome()) failed: file ../../../../mozilla/content/base/src/nsImageLoadingContent.cpp, line 153
WARNING: NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome()) failed: file ../../../../mozilla/content/base/src/nsImageLoadingContent.cpp, line 177
WARNING: NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome()) failed: file ../../../../mozilla/content/base/src/nsImageLoadingContent.cpp, line 188
WARNING: NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome()) failed: file ../../../../mozilla/content/base/src/nsImageLoadingContent.cpp, line 198
WARNING: NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome()) failed: file ../../../../mozilla/content/base/src/nsImageLoadingContent.cpp, line 208

EXPECTED RESULTS:
No spam.


This is from code like this:
150 NS_IMETHODIMP
151 nsImageLoadingContent::OnStartDecode(imgIRequest* aRequest)
152 {
153   NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome(), NS_ERROR_NOT_AVAILABLE);
154 
155   LOOP_OVER_OBSERVERS(OnStartDecode(aRequest));
156   return NS_OK;
157 }

and we're failing that first line.

NOTE:  The spam seems to correspond to the file-icons in the list, and it does *not* seem to happen for folder-icons, for some reason.

So for example, if I repeat the STR at the URL ftp://ftp.mozilla.org/pub/mozilla.org/labs/weave/ , which just has folder-icons, I don't get any spam whatsoever -- whereas if I visit ftp://ftp.mozilla.org/pub/mozilla.org/data/ , which has folders and a single file icon, I get the spam.

We probably (?) want to just change these NS_ENSURE_TRUE methods to standard early-returns.
Uh.  What does the callstack look like there?
(Assignee)

Comment 2

5 years ago
Created attachment 654760 [details]
backtrace

Here's the backtrace, the first time IsCallerChrome returns false and triggers console-spam.
(Assignee)

Comment 3

5 years ago
Reduced testcase:
data:text/html,Click:<img src="moz-icon://.html" onclick="this.parentNode.removeChild(this)">

Clicking the image (removing it from the DOM) triggers the warning.
Hmm.  that makes sense for OnDiscard.  What's the stack for OnStartDecode?
(Assignee)

Comment 5

5 years ago
Created attachment 654772 [details]
backtrace for OnStartDecode

Looks like it's just on BindToTree when the <img> element is (re)inserted.

And because the img is a special "moz-icon" image, we get an immediate decode, rather having it be async.
Ah.  So I think we should be pushing a null JSContext or whatever the equivalent is nowadays of saying "Pretend I'm not calling from script" around the AddImage call there.  And add a comment that says we're doing that so we don't look like we're calling our callbacks from JS.

For discard we should consider the same thing, actually.
(In reply to Boris Zbarsky (:bz) from comment #6)
> For discard we should consider the same thing, actually.

We should be waiting for the discard timer, not discarding synchronously.
(Assignee)

Comment 8

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #6)
> Ah.  So I think we should be pushing a null JSContext or whatever the
> equivalent is nowadays of saying "Pretend I'm not calling from script"
> around the AddImage call there.
Looks like there's an example of that in nsImageLoadingContent::ClearPendingRequest:

1096   // Push a null JSContext on the stack so that code that runs within
1097   // the below code doesn't think it's being called by JS. See bug
1098   // 604262.
1099   nsCxPusher pusher;
1100   pusher.PushNull();

https://mxr.mozilla.org/mozilla-central/source/content/base/src/nsImageLoadingContent.cpp#1090

So we just need to crib that code in BindToTree and UnbindFromTree (except it sounds like bz wants to make UnbindFromTree a little fancier, per comment 7).
(Assignee)

Comment 9

5 years ago
I'll add the pusher.PushNull() stuff, to fix this bug, and then I'll file a followup for Comment 7, if that sounds OK.  (since it's conceptually separate, to a certain degree)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
(Assignee)

Comment 10

5 years ago
Created attachment 654782 [details] [diff] [review]
fix

This fixes the STR for me.
Attachment #654782 - Flags: review?(bzbarsky)
Comment on attachment 654782 [details] [diff] [review]
fix

r=me
Attachment #654782 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 12

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=fa18001d0a76
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #7)
> (In reply to Boris Zbarsky (:bz) from comment #6)
> > For discard we should consider the same thing, actually.
> 
> We should be waiting for the discard timer, not discarding synchronously.

I agree with this.
Why are we discarding synchronously? We don't want discard when moving <img> around DOM.
(Assignee)

Comment 14

5 years ago
(In reply to Daniel Holbert [:dholbert] from comment #8)
> (except it sounds like bz wants to make UnbindFromTree a little fancier, per comment
> 7).

er, s/bz/khuey/

I filed bug 785231 on the sync vs async discarding -- let's take that discussion over there.
(Assignee)

Comment 15

5 years ago
Try has several completed all-green rows, so I think this is OK to push:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/6e67b20609d2
https://hg.mozilla.org/mozilla-central/rev/6e67b20609d2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.