1325813 - JS-implemented (Adblock?) nsIContentPolicy can be called for fonts during TabChild::ForcePaint

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Nickolay_Ponomarev]

This bug was filed from the Socorro interface and is 
report bp-8c5e9702-0eba-435a-9140-ef7f62161225.
=============================================================

I had a crash in 2016-12-21 Nightly caused by, I guess, Adblock's implementation of content policy being called during TabChild::ForcePaint via 

* gfxFontGroup::GetFirstValidFont calling
* gfxUserFontEntry::LoadNextSrc calling
* gfxUserFontSet::UserFontCache::GetFont calling
* FontFaceSet::UserFontSet::IsFontLoadAllowed calling
* NS_CheckContentLoadPolicy

As far as I understand it's caused by the change in bug 1279086.

Comment 1

[Nickolay_Ponomarev]

This seems to be responsible for 47 crashes with this signature [2] out of 70 [1] for the last week.

[1] 70 crashes with this signature https://crash-stats.mozilla.com/search/?signature=%3Djs%3A%3ABarrierMethods%3CT%3E%3A%3AexposeToJS&product=Firefox&date=%3E%3D2016-12-18T12%3A50%3A00.000Z&date=%3C2016-12-25T12%3A50%3A00.000Z&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature
[2] 47 crashes with this signature and "ContentPolicy" and "FontFace" in the stack ("proto_signature" in soccoro's terms): <https://crash-stats.mozilla.com/signature/?product=Firefox&proto_signature=~ContentPolicy&proto_signature=~FontFace&signature=js%3A%3ABarrierMethods%3CT%3E%3A%3AexposeToJS&date=%3E%3D2016-12-18T12%3A50%3A00.000Z&date=%3C2016-12-25T12%3A50%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_sort=-date&page=1#reports>)

Comment 3

[Ryan VanderMeulen [:RyanVM]]

More specifically, bug 1308039 added the release assert that's being hit here.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Bug 1328423 tracks backing out bug 1308039, which should resolve this at the same time.

Comment 7

[(no longer active)]

This isn't fixed, for example: bf9f9505-d21c-481f-9944-d22512170312

Bill, what should we do about this?

Comment 8

[(no longer active)]

Looks like we are coming from this code: <https://hg.mozilla.org/mozilla-central/annotate/3e73fd638e68/gfx/thebes/gfxTextRun.cpp#l1878>.

Jonathan, is it possible to somehow ensure that the font isn't used for painting while LoadState() returns gfxUserFontEntry::STATUS_NOT_LOADED?

Comment 9

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to :Ehsan Akhgari from comment #7)
> This isn't fixed, for example: bf9f9505-d21c-481f-9944-d22512170312
> 
> Bill, what should we do about this?

The buildid for that crash is 20161102030205. Someone was running a really old build. The fixes for this stuff landed in bug 1328423 (so January 2017).

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Since the crash volume is low (less than 5 per week), the severity is downgraded to S3. Feel free to change it back if you think the bug is still critical.

For more information, please visit auto_nag documentation.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:dholbert, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 12

[Gregory Pappas [:gregp]]

Extensions aren't allowed to reimplement nsIContentPolicy anymore