Last Comment Bug 771666 - Update sandbox tests to reflect expected plugin behavior
: Update sandbox tests to reflect expected plugin behavior
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Identity (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: John Schoenick [:johns]
:
: Fernando Jiménez Moreno [:ferjm]
Mentors:
Depends on: 762993
Blocks: 745030
  Show dependency treegraph
 
Reported: 2012-07-06 14:45 PDT by John Schoenick [:johns]
Modified: 2012-08-06 07:43 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Update sandbox test to reflect expected plugin behavior (4.99 KB, patch)
2012-07-19 18:43 PDT, John Schoenick [:johns]
MattN+bmo: review+
Details | Diff | Splinter Review
Update sandbox test to reflect expected plugin behavior, r=mattn (4.83 KB, patch)
2012-07-24 16:35 PDT, John Schoenick [:johns]
no flags Details | Diff | Splinter Review
Update sandbox test to reflect expected plugin behavior. r=mattn (4.92 KB, patch)
2012-08-02 15:04 PDT, John Schoenick [:johns]
no flags Details | Diff | Splinter Review
Update sandbox test to reflect expected plugin behavior. r=mattn (4.92 KB, patch)
2012-08-02 15:09 PDT, John Schoenick [:johns]
no flags Details | Diff | Splinter Review

Description John Schoenick [:johns] 2012-07-06 14:45:48 PDT
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] 2012-07-19 18:43:22 PDT
Created attachment 644117 [details] [diff] [review]
Update sandbox test to reflect expected plugin behavior
Comment 2 John Schoenick [:johns] 2012-07-19 18:50:02 PDT
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] (PM me if requests are blocking you) 2012-07-20 02:40:08 PDT
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] 2012-07-24 16:35:12 PDT
Created attachment 645574 [details] [diff] [review]
Update sandbox test to reflect expected plugin behavior, r=mattn
Comment 5 John Schoenick [:johns] 2012-07-24 16:40:38 PDT
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 8 John Schoenick [:johns] 2012-07-25 13:44:35 PDT
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] 2012-08-02 15:04:13 PDT
Created attachment 648507 [details] [diff] [review]
Update sandbox test to reflect expected plugin behavior. r=mattn

Minor tweak, carrying forward r+
Comment 10 John Schoenick [:johns] 2012-08-02 15:09:27 PDT
Created attachment 648513 [details] [diff] [review]
Update sandbox test to reflect expected plugin behavior. r=mattn

Minor tweak v2!
Comment 12 John Schoenick [:johns] 2012-08-02 20:03:16 PDT
Annnd backed out again for aggravating Bug 778179 go nuts...

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf46b1ea1ccc
Comment 13 John Schoenick [:johns] 2012-08-02 20:05:11 PDT
(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] 2012-08-02 20:08:23 PDT
I'll look at that tomorrow.  If you can catch the crash in a debugger, that would help.
Comment 15 John Schoenick [:johns] 2012-08-05 20:21:36 PDT
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] 2012-08-06 07:43:11 PDT
https://hg.mozilla.org/mozilla-central/rev/3bfbd9a8ed90

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