Last Comment Bug 606641 - Remove the DoInstantiateEmbeddedPlugin goop added in bug 484992
: Remove the DoInstantiateEmbeddedPlugin goop added in bug 484992
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Josh Aas
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-22 20:24 PDT by Boris Zbarsky [:bz]
Modified: 2011-08-24 01:33 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix v1.0 (11.00 KB, patch)
2011-04-28 06:52 PDT, Josh Aas
bzbarsky: review+
Details | Diff | Splinter Review
fix v2.0 (6.83 KB, patch)
2011-05-23 10:53 PDT, Josh Aas
jst: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2010-10-22 20:24:37 PDT
See bug 484992 comment 21.  I think nowadays since we have no default plugin we should be able to just use SetUpPluginInstance directly.
Comment 1 Josh Aas 2011-04-28 06:52:40 PDT
Created attachment 528836 [details] [diff] [review]
fix v1.0
Comment 2 Boris Zbarsky [:bz] 2011-04-28 07:07:05 PDT
This isn't quite what I was thinking when I filed this bug...  I was thinking we could rip out this boolean argument altogether.
Comment 3 Josh Aas 2011-04-28 08:14:27 PDT
Ah, I forgot about that. You mind if we take this cleanup first and I'll do that next?
Comment 4 Boris Zbarsky [:bz] 2011-04-28 08:16:20 PDT
Comment on attachment 528836 [details] [diff] [review]
fix v1.0

Sure.
Comment 5 Josh Aas 2011-04-28 16:59:13 PDT
pushed fix v1.0 to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/e9dafe775b31
Comment 6 Josh Aas 2011-05-23 10:53:31 PDT
Created attachment 534483 [details] [diff] [review]
fix v2.0
Comment 7 Josh Aas 2011-05-23 10:55:09 PDT
I'm not sure that fix v2.0 is really something we want yet, but I want to make sure that it addresses bz's concern, or is at least on the right track.
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2011-05-25 17:10:32 PDT
Comment on attachment 534483 [details] [diff] [review]
fix v2.0

Since you want to know whether this addresses bz's concerns I think he should review here :)

Looks good to me fwiw, but I didn't dig too deep here.
Comment 9 Boris Zbarsky [:bz] 2011-05-25 18:32:26 PDT
I already told Josh that this addresses my concerns.  Now it needs correctness review, and I have no clue whether this is correct....  So I can't really review it.
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2011-05-25 23:02:08 PDT
Comment on attachment 534483 [details] [diff] [review]
fix v2.0

Ok, this looks correct to me. r=jst
Comment 11 Josh Aas 2011-05-26 18:51:34 PDT
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/7a6804f6034e
Comment 12 Virgil Dicu [:virgil] [QA] 2011-08-24 01:33:06 PDT
Could anyone provide some clear Steps to reproduce in order to have this issue verified? Thanks.

Note You need to log in before you can comment on or make changes to this bug.