Closed Bug 591409 Opened 14 years ago Closed 13 years ago

"ASSERTION: constructing frames in the middle of a paint" with flash slow script dialog

Categories

(Core Graveyard :: Plug-ins, defect)

Product:
Core Graveyard
Component:
Plug-ins
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox5- wontfix, firefox6- wontfix, firefox7+ wontfix, firefox8+ fixed, firefox9+ fixed, blocking2.0 -, blocking1.9.2 -, status1.9.2 wontfix, status1.9.1 wontfix)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox5 - wontfix
firefox6 - wontfix
firefox7 + wontfix
firefox8 + fixed
firefox9 + fixed
blocking2.0 --- -
blocking1.9.2 --- -
status1.9.2 --- wontfix
status1.9.1 --- wontfix

People

(Reporter: dveditz, Assigned: benjamin)

References

Details

(Keywords: assertion, Whiteboard: [sg:critical?][mac to be fixed by asynchronous plugin painting, other platforms ok (on branches?)][qa-]) 

+++ This bug was initially created as a clone of Bug #449129 +++

After bug 449129 was resolved it sprouted another patch that appears to have never landed (had trouble with try server). Cloning bug to prevent it getting lost. From bug 449129 comment 43:

   Created attachment 453403 [details] [diff] [review]
   Part B, rev. 1: do the same thing for in-process painting

   This is the more-scary bit: I'd like to go ahead and land this on
   trunk for 4.0 so we can get crash data from the betas, but probably
   not land this on branches for a while because of the scariness.
blocking2.0: final+ → ?
Dup of bug 552512?
Depends on: 556487
Whiteboard: [sg:critical?] → [sg:critical?][to be fixed by asynchronous plugin painting]
Is this something we still want to do this late in the beta cycle for 4?
Which, asynchronous painting? I think so. Otherwise we'll end up living with the potentially-exploitable scripting during painting for another full release cycle, and that seems worse. There's a fair bit of perf to be gained also, I think.
Ok, I'm confused about this bug. Yes, we should do async painting, and we are... But there's other bugs for that. Isn't this about aborting in case we end up in our event loop while painting, in the in-process case, and we hard abort the whole browser. Do we really need to do that in the async case?

Seems to me that this is a non-issue for trunk since we're doing async painting, which means this is not a blocker for trunk and should be marked to reflect the fact that this only applies to older branches. Or am I out in the weeds here?
No, we don't need to do that in the async case. *If* async-painting makes 2.0, this bug is really relevant for the branch IPP only.
Where's the bug for async painting?
Depends on: 596451
Depends on: 598425
Added deps for the async painting.
blocking2.0: ? → final+
Joe says asynchronous painting cannot be back-ported (requires layers and more). Roc suggests a version of the fix in bug 594774 will mitigate most of the problem (but that can't be directly back-ported)
Depends on: 594774
Whiteboard: [sg:critical?][to be fixed by asynchronous plugin painting] → [sg:critical?][to be fixed by asynchronous plugin painting (on branches?)]
Mostly fixed, but we're not going to hold FF4 for the mac issue.
blocking2.0: final+ → -
Whiteboard: [sg:critical?][to be fixed by asynchronous plugin painting (on branches?)] → [sg:critical?][mac to be fixed by asynchronous plugin painting, other platforms ok (on branches?)]
There's nothing to track for Firefox 6 here, the mac async painting will land for 7.
Bug 598425 landed in early June, so this is fixed in 7 and beyond!
Status: NEW → RESOLVED
Closed: 13 years ago
status-firefox6: affected → wontfix
status-firefox7: --- → fixed
status-firefox8: --- → fixed
tracking-firefox8: --- → +
Resolution: --- → FIXED
Bug 663259 provided the actual patch to turn async on, so this will be fixed in 8, not 7.

And note that a few plugins still run in-process: Java on all platforms, and all plugins on MacOS10.5, so I wouldn't make this bug public any time soon.
status-firefox7: fixed → wontfix
qa- as we don't verify ASSERTIONs. Please re-set to qa+ if a testcase can be provided for QA to verify the fix.
Whiteboard: [sg:critical?][mac to be fixed by asynchronous plugin painting, other platforms ok (on branches?)] → [sg:critical?][mac to be fixed by asynchronous plugin painting, other platforms ok (on branches?)][qa-]
status-firefox9: --- → fixed
tracking-firefox9: --- → +
We clearly aren't going to take async painting on the 1.9.2 branch, and the original comment 0 said the missing patch is too scary to land on branches without trunk testing (which we're not ever going to get because we did async painting instead).

Do we just wontfix this sg:critical bug for the 1.9.2 branch or take a risk on the patch in bug 449129 attachment 453403 [details] [diff] [review]?
blocking1.9.2: --- → ?
status1.9.1: wanted → wontfix
I don't think we have any way of taking the risk: the patch was obviously incomplete (causing false-positive crashiness).
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #15)
> I don't think we have any way of taking the risk: the patch was obviously
> incomplete (causing false-positive crashiness).

In your opinion, is this bug critical enough to pursue a complete and tested patch (or possibly an alternative workaround)? I just want to make sure we're not making too much of a compromise due to the unfinished nature of a proposed fix.
An exploit for this bug has never been developed. I think that the proposed fix is so risky that we wouldn't take it absent an immediate need.
Thanks bsmedberg. Marking as wontfix for 1.9.2.
status1.9.2: wanted → wontfix
blocking1.9.2: ? → -
Group: core-security
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.