security/frameNavigation tests should test for DENIED cases

• Component: Tools / Tests
• Status: ASSIGNED ---

[Adam Barth]

Currently, the tests in LayoutTests/http/tests/security/frameNavigation test only for ALLOWED cases.  To provide better coverage, they should tests DENIED cases too.

[Adam Barth]

Created attachment 18141 [details]
More frameNavigation tests

This patch adds a few tests for DENIED frame navigation cases.  Currently, they should all pass, but hopefully they'll catch future regressions.

[Adam Barth]

I'm not sure these tests properly call notifyDone when the navigation is blocked.  One possibility is to wait for Bug 16522 to be fixed.  After that patch lands, failed navigations will cause navigation-changed-iframe.html to be opened in a popup window (as in other browsers).  If we go that route, we'll want to set layoutTestController.setCanOpenWindows() and clean up the popup if top === self.

[Darin Adler]

Comment on attachment 18141 [details]
More frameNavigation tests

r=me

[Darin Adler]

Comment on attachment 18141 [details]
More frameNavigation tests

As you suspected, these tests don't all pass. Also, the patch doesn't include expected results for navigation-ALLOWED-descendant.html or navigation-ALLOWED-top.html.

[Adam Barth]

Thanks for taking a look at this.  I should have access to a Mac in a week or two and will update the patch then.

[Sam Weinig]

A way I have considered testing the denied case, after the behavior has been changed to open the denied navigation in a new window, would be to use the layoutTestController.windowCount() function.

[Adam Barth]

Created attachment 18184 [details]
More tests (still not working)

Added another test, but haven't addressed the issues raised by Darin.

[Adam Barth]

Created attachment 18459 [details]
Mozilla frame navigation tests

We wrote a fairly extensive test suite for Mozilla that you're welcome to use.  It's written for their testing harness, and I'm not sure what would be involved in porting it to DRT.  Also, their rules for when you can script data: URLs are different than yours, so you might want to replace those with actual documents.

[Adam Barth]

Created attachment 21507 [details]
More test coverage

This patch significantly improves our LayoutTest coverage for navigation tests.  The patch tries each method of navigation (assigning to location, window.open, clicking hyperlinks, submitting forms, and plugs calling getURL) in a variety of frame configurations.  The tests reveal a bug in our plugin code where we don't properly set the opener of frames we create, but we should fix that in a separate bug.

[Darin Adler]

Comment on attachment 21507 [details]
More test coverage

I'm concerned about timing dependencies in the tests. I'm not happy with the 50ms timeouts in some tests.

r=me

[Adam Barth]

Comment on attachment 21507 [details]
More test coverage

The tests didn't pass on the build bots.  I'm not entirely sure why, but I'll go another round with them and address Darin's comments.