771666 - Update sandbox tests to reflect expected plugin behavior

Status: RESOLVED FIXED

(Core Graveyard :: Identity, defect)

Description

[John Schoenick [:johns]]

Currently in toolkit/identity/tests/chrome/test_sandbox.xul we test that objects are suppressed by checking for channel opens.

However, in bug 745030, we become slightly smarter about opening channels for <object>/<embed> tags - we will open the channel if the server might return a unblocked MIME type (e.g. if image, subdocument, OR plugin is allowable), and then abort if it returns a blocked type. This means that checking for a channel open isn't sufficient for testing whether the object finished loading.

Comment 1

[John Schoenick [:johns]]

Attachment: Update sandbox test to reflect expected plugin behavior

Comment 2

[John Schoenick [:johns]]

Comment on attachment 644117 [details] [diff] [review]
Update sandbox test to reflect expected plugin behavior

This changes the channel checking slightly to be okay with plugin types, but then adds a new block to specifically check that no plugin tags loaded any content (via nsIObjectLoadingContent.displayedType == TYPE_NULL)

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 644117 [details] [diff] [review]
Update sandbox test to reflect expected plugin behavior

Review of attachment 644117 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing this. r=me with fixes.

::: toolkit/identity/tests/chrome/test_sandbox.xul
@@ +128,5 @@
>  
>      if (aNothingShouldLoad) {
>        is(xhr.responseText, "NOTHING", "Check that nothing was loaded on the server");
>      } else if (!OSX_10_5) {
> +    } else {

I'm guessing this didn't get rebased properly. Remove the |} else {| line so that these checks stay disabled on 10.5 due to intermittent failures (bug 770063).

@@ +135,4 @@
>        let loadedTypes = xhr.responseText == "NOTHING" ? [] : xhr.responseText.split(",");
>  
> +      for (let ind in loadedTypes) {
> +        isnot(allowedTypes.indexOf(loadedTypes[ind]), -1, "Check that " + loadedTypes[ind] + " was expected to load");

You can use for…of here since you are only using |ind| to get loadedTypes[ind]:

|for (let loadedType of loadedTypes) {|

https://developer.mozilla.org/en/JavaScript/Reference/Statements/for...of

@@ +154,5 @@
> +      let nullType = Components.interfaces.nsIObjectLoadingContent.TYPE_NULL;
> +      let pluginTags = [ "embed", "object", "applet" ];
> +      for (let ind = 0; ind < pluginTags.length; ind++) {
> +        let tags = doc.getElementsByTagName(pluginTags[ind]);
> +        for (let tagInd = 0; tagInd < tags.length; tagInd++) {

I think this would be clearer with one loop using querySelectorAll and for…of:

for (let tag of doc.querySelectorAll("embed, object, applet")) {
  tag instanceof Components.interfaces.nsIObjectLoadingContent;
  is(tag.displayedType, nullType, "Check that plugin did not load content");
}

Comment 4

[John Schoenick [:johns]]

Attachment: Update sandbox test to reflect expected plugin behavior, r=mattn

Comment 5

[John Schoenick [:johns]]

for..of? querySelectorAll? That will NEVER work in IE7!

Oh. Right.

Updated with notes, I'll land this tomorrow with bug 745030 if nothing else burns down

Comment 6

[John Schoenick [:johns]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c697eeb050dd

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d5231ba7849
https://hg.mozilla.org/integration/mozilla-inbound/rev/aef63a0ce60c

Comment 8

[John Schoenick [:johns]]

This was backed out along with bug 745030 for random oranges, though 745030 is likely to blame:
https://tbpl.mozilla.org/php/getParsedLog.php?id=13845528&tree=Mozilla-Inbound&full=1

Comment 9

[John Schoenick [:johns]]

Attachment: Update sandbox test to reflect expected plugin behavior. r=mattn

Minor tweak, carrying forward r+

Comment 10

[John Schoenick [:johns]]

Attachment: Update sandbox test to reflect expected plugin behavior. r=mattn

Minor tweak v2!

Comment 11

[John Schoenick [:johns]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f47de086284

Comment 12

[John Schoenick [:johns]]

Annnd backed out again for aggravating Bug 778179 go nuts...

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf46b1ea1ccc

Comment 13

[John Schoenick [:johns]]

(In reply to John Schoenick [:johns] from comment #12)
> Annnd backed out again for aggravating Bug 778179 go nuts...

Err, making* bug 778179 go nuts

Comment 14

[Andrew McCreight [:mccr8]]

I'll look at that tomorrow.  If you can catch the crash in a debugger, that would help.

Comment 15

[John Schoenick [:johns]]

Third time's the charm, now that we think 778179 has been resolved

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bfbd9a8ed90

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3bfbd9a8ed90