Closed
Bug 783059
Opened 12 years ago
Closed 12 years ago
object/embed/applet tags need instantiation tests
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla22
People
(Reporter: johns, Assigned: johns)
References
Details
Attachments
(2 files, 5 obsolete files)
(deleted),
patch
|
johns
:
checkin+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
johns
:
checkin+
|
Details | Diff | Splinter Review |
In cases like bug 782707 and bug 782703 it's clear that we don't have good test coverage for making sure objects instantiate when they should. This is mostly because the pre-bug 745030 logic for "when they should" was approximately "sometimes, maybe" Incomplete list of things we need coverage on - Removing and re-adding to the document - obj.data = obj.data while in and out of the document - Changing obj.data multiple times with and w/o an associated channel - Changing object type hints without changing channel - Moving into subdocuments that change the effective URI, when obj.data is relative
Assignee | ||
Comment 1•12 years ago
|
||
also, - Move running plugins between documents, make sure they don't reinstantiate, but do if left out for a full event loop cycle. Try changing plugin parameters while in this limbo state.
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 2•12 years ago
|
||
Still need to make hasRunningPlugin callable via SpecialPowers to test for instantiation....
Assignee | ||
Comment 3•12 years ago
|
||
@bz This might be useful for testing the object/embed webidl stuff, though it does not much touch script access. It also takes a good thirty seconds to run.
Comment 4•12 years ago
|
||
30 seconds is nothing compared to the time I've already spent on this junk. Running pretty much any plug-in test in our test harness takes that long. I'll run this in my local build with all those changes.
Comment 5•12 years ago
|
||
Hmm. Do we want to land this first and then the WebIDL stuff? Or the other way around? The merge is not quite trivial: hasRunningPlugin needs a WebIDL version. Just trying to figure out which direction I should merge in locally.
Assignee | ||
Comment 6•12 years ago
|
||
I still need to fix the hasRunningPlugin stuff to actually work right, so I wouldn't block the webidl changes on this.
Assignee | ||
Comment 7•12 years ago
|
||
@bz this version has finished hasRunningPlugin checks and tries to poke at the plugin to test scriptability in all cases where it finds it should be instantiated. It also is up to 3 minutes to run, which I have to work on...
Attachment #717416 -
Attachment is obsolete: true
Comment 8•12 years ago
|
||
Well, the whole 14000-test thing probably doesn't help. ;) I can't manage to run this test: it always takes longer than the test harness timeout for me (in a debug build, granted).
Assignee | ||
Comment 9•12 years ago
|
||
This should take an order of magnitude less time
Attachment #718189 -
Attachment is obsolete: true
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #718634 -
Attachment is obsolete: true
Attachment #722572 -
Flags: review?(joshmoz)
Assignee | ||
Comment 12•12 years ago
|
||
Assignee | ||
Comment 13•12 years ago
|
||
Comment on attachment 722573 [details] [diff] [review] Add do_lookupGetter helper to SpecialPowers The much simpler than we discussed but still groan-inducing helper needed for the test.
Attachment #722573 -
Flags: review?(bobbyholley+bmo)
Comment 14•12 years ago
|
||
Comment on attachment 722573 [details] [diff] [review] Add do_lookupGetter helper to SpecialPowers Review of attachment 722573 [details] [diff] [review]: ----------------------------------------------------------------- Please add a comment explaining why this is necessary. r=bholley with that. ::: testing/specialpowers/content/specialpowersAPI.js @@ +710,5 @@ > obj2=unwrapIfWrapped(obj2); > return obj1 instanceof obj2; > }, > > + do_lookupGetter: function(obj, getter) { I'd call the param "name" rather than "getter".
Attachment #722573 -
Flags: review?(bobbyholley+bmo) → review+
Comment 15•12 years ago
|
||
Comment on attachment 722572 [details] [diff] [review] Plugin instantiation tests for embed/object tags Review of attachment 722572 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsObjectLoadingContent.h @@ +199,5 @@ > } > + bool HasRunningPlugin() const > + { > + return !!mInstanceOwner; > + } Where is this used?
Attachment #722572 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 17•12 years ago
|
||
(In reply to Josh Aas (Mozilla Corporation) from comment #15) > Comment on attachment 722572 [details] [diff] [review] > Plugin instantiation tests for embed/object tags > > Review of attachment 722572 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: content/base/src/nsObjectLoadingContent.h > @@ +199,5 @@ > > } > > + bool HasRunningPlugin() const > > + { > > + return !!mInstanceOwner; > > + } > > Where is this used? This is the webIDL equivalent to GetHasRunningPlugin() for the XPIDL, though GetHasRunningPlugin should just be calling this! The test uses this to test if the plugin spawned of its own accord (if we just call .getObjectValue() it will force-spawn the plugin if it hasn't yet)
Assignee | ||
Comment 18•12 years ago
|
||
Comment on attachment 722573 [details] [diff] [review] Add do_lookupGetter helper to SpecialPowers https://hg.mozilla.org/integration/mozilla-inbound/rev/36cde68b1bf4
Attachment #722573 -
Flags: checkin+
Assignee | ||
Comment 19•12 years ago
|
||
Comment on attachment 722572 [details] [diff] [review] Plugin instantiation tests for embed/object tags https://hg.mozilla.org/integration/mozilla-inbound/rev/9f5cff5a1a3d
Attachment #722572 -
Flags: checkin+
Assignee | ||
Comment 20•12 years ago
|
||
Comment on attachment 722572 [details] [diff] [review] Plugin instantiation tests for embed/object tags Blew up on android. https://hg.mozilla.org/integration/mozilla-inbound/rev/ad90d1366dba
Attachment #722572 -
Flags: checkin+ → checkin-
Assignee | ||
Updated•12 years ago
|
Attachment #722573 -
Flags: checkin+ → checkin-
Assignee | ||
Comment 21•12 years ago
|
||
Exclude this test on android since we have no test plugin...
Attachment #722572 -
Attachment is obsolete: true
Assignee | ||
Comment 24•12 years ago
|
||
Comment on attachment 722986 [details] [diff] [review] Plugin instantiation tests for embed/object tags. r=josh Attempt #2 https://hg.mozilla.org/integration/mozilla-inbound/rev/18df15462240
Attachment #722986 -
Flags: checkin+
Assignee | ||
Comment 25•12 years ago
|
||
Comment on attachment 722988 [details] [diff] [review] Add do_lookupGetter helper to SpecialPowers. r=bholley https://hg.mozilla.org/integration/mozilla-inbound/rev/c7ff3b1f040f
Attachment #722988 -
Flags: checkin+
Comment 26•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c7ff3b1f040f https://hg.mozilla.org/mozilla-central/rev/18df15462240
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•