object/embed/applet tags need instantiation tests

RESOLVED FIXED in mozilla22

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: johns, Assigned: johns)

Tracking

Trunk
mozilla22
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

6 years ago
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

6 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

6 years ago
Depends on: 767631
(Assignee)

Updated

6 years ago
Blocks: 767631
No longer depends on: 767631
(Assignee)

Updated

6 years ago
Blocks: 767627
(Assignee)

Updated

6 years ago
Blocks: 738396
(Assignee)

Updated

5 years ago
Depends on: 812629
(Assignee)

Updated

5 years ago
No longer blocks: 767627
(Assignee)

Updated

5 years ago
Depends on: 784131
(Assignee)

Updated

5 years ago
Depends on: 832032
(Assignee)

Comment 2

5 years ago
Created attachment 717416 [details] [diff] [review]
Plugin instantation test matrix

Still need to make hasRunningPlugin callable via SpecialPowers to test for instantiation....
(Assignee)

Comment 3

5 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.
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.
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

5 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

5 years ago
Created attachment 718189 [details] [diff] [review]
Plugin instantation test matrix

@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
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

5 years ago
Created attachment 718634 [details] [diff] [review]
Plugin instantiation test matrix

This should take an order of magnitude less time
Attachment #718189 - Attachment is obsolete: true
Ah, great.  That runs and passes with my WebIDL patches.
(Assignee)

Comment 11

5 years ago
Created attachment 722572 [details] [diff] [review]
Plugin instantiation tests for embed/object tags
Attachment #718634 - Attachment is obsolete: true
Attachment #722572 - Flags: review?(joshmoz)
(Assignee)

Comment 12

5 years ago
Created attachment 722573 [details] [diff] [review]
Add do_lookupGetter helper to SpecialPowers
(Assignee)

Comment 13

5 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 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+
(Assignee)

Updated

5 years ago
Depends on: 786650
(Assignee)

Updated

5 years ago
Depends on: 803159

Comment 15

5 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

5 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

5 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

5 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

5 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

5 years ago
Attachment #722573 - Flags: checkin+ → checkin-
(Assignee)

Comment 21

5 years ago
Created attachment 722986 [details] [diff] [review]
Plugin instantiation tests for embed/object tags. r=josh

Exclude this test on android since we have no test plugin...
Attachment #722572 - Attachment is obsolete: true
(Assignee)

Comment 22

5 years ago
Created attachment 722988 [details] [diff] [review]
Add do_lookupGetter helper to SpecialPowers. r=bholley
Attachment #722573 - Attachment is obsolete: true
(Assignee)

Comment 24

5 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

5 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+
https://hg.mozilla.org/mozilla-central/rev/c7ff3b1f040f
https://hg.mozilla.org/mozilla-central/rev/18df15462240
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.