Plugins may get instantiated before the first reflow of the object frame. This bug is for the case where a frame changes from display:none to inline (or whatever), i.e. the one where the frame calls HasNewFrame on the content. That call should move to DidReflow with a check that it's the first reflow. Regression from bug 1156

So basically this, however... Consider this set of events: - page has <object data="foo.gif" width="400" height="400"></object> - javascript sets a classid attribute & changes the data attribute (the latter triggers LoadObject) - LoadObject sees the classid attribute and flushes some stuff leading to frame creation but not to reflow - It then calls Instantiate on the frame - Later, reflow happens, that calls HasNewFrame on the content, that calls Instantiate again that seems bad. Perhaps the GetFrame call that bug 381512 is adding to nsObjectLoadingContent::Instantiate should flush layout?

This patch does a few things: - make sure that nsObjectLoadingContent::mContentType is correct (for the case where we instantiate by extension) - remove some notify calls from nsObjectLoadingContent; instead, wait for the HasNewFrame call - remove the FrameNeedsReflow calls from nsObjectFrame; the SetWindow call via Instantiate* should be enough. The widget showing is done by nsObjectFrame::CreateWidget. - Fix the offset calculations in FixupWindow to match the ones done by DidReflow (note: This was inconsistent pre-bug 1156 too)

So why is it OK to remove those Notify() calls? Are we sure that the state didn't change there or something?

what needs to be done will be done by the destructor of AutoNotifier.

Comment on attachment 275146 [details] [diff] [review] patch v2 Looks good. It would also be nice to not flush reflow when we already have a reflowed frame in OnStartRequest... can that happen, though? If it can, file a followup to do that?

you're right, I don't think we can have an object frame there, because we'd be in the loading state until then.

Comment on attachment 275146 [details] [diff] [review] patch v2 this patch makes us more compatible with how 1.8 handled plugins

Comment on attachment 275146 [details] [diff] [review] patch v2 clearing approval request as this is a blocker now

Checking in content/base/src/nsObjectLoadingContent.cpp; /cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v <-- nsObjectLoadingContent.cpp new revision: 1.58; previous revision: 1.57 done Checking in content/base/src/nsObjectLoadingContent.h; /cvsroot/mozilla/content/base/src/nsObjectLoadingContent.h,v <-- nsObjectLoadingContent.h new revision: 1.20; previous revision: 1.19 done Checking in layout/generic/nsObjectFrame.cpp; /cvsroot/mozilla/layout/generic/nsObjectFrame.cpp,v <-- nsObjectFrame.cpp new revision: 1.611; previous revision: 1.610 done

For the record, this caused bug 398213. We'll need to fix that regression, or we'll need to back this out, or find an alternate solution for the regression.