Closed Bug 1342298 Opened 3 years ago Closed 2 years ago

Intermittent dom/base/test/browser_bug902350.js | Navigating to insecure domain through target='_top' failed. - "https://example.com/tests/dom/base/test/file_bug902350.html" == "http://example.com/" -

Categories

(Core :: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: hchang)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed])

Attachments

(2 files)

Whiteboard: [stockwell unknown]
Priority: -- → P5
Hi Ethan and Winnie,
I blamed the code of browser_bug902350.js which was written for a security feature, mixed content blocker. I wonder if this is something your team could help take a look. Thanks.
Flags: needinfo?(wleung)
Flags: needinfo?(ettseng)
Hi Kate, can you take a look at this bug. Thanks!
Flags: needinfo?(wleung) → needinfo?(kmckinley)
Kate or Chris might be able to help on this.
Flags: needinfo?(ettseng) → needinfo?(ckerschb)
Based on test screenshots (sample uploaded), it looks like either the click in the frame did not happen, or the tab has not actually finished loading before the condition is tested. This further causes an error when the Assert fires and there is no method to capture the error.

I have not been able to reproduce this locally.
Flags: needinfo?(kmckinley)
I suppose fixing this intermittent could wait a few days longer. Ethan, I suppose we could rewrite that test to make it less flaky by using a promise (similar to what we have here [1]). Do you think Henry could take that on once he is done with data: URI stuff? If not, let me know and I'll try to find someone else.

[1] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser_e10s_about_page_triggeringprincipal.js#18
Flags: needinfo?(ckerschb) → needinfo?(ettseng)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #13)
> I suppose fixing this intermittent could wait a few days longer. Ethan, I
> suppose we could rewrite that test to make it less flaky by using a promise
> (similar to what we have here [1]). Do you think Henry could take that on
> once he is done with data: URI stuff? If not, let me know and I'll try to
> find someone else.

Okay, I'll see what we can do for this.
Flags: needinfo?(ettseng)
Component: DOM → Security
Whiteboard: [stockwell needswork:DOM] → [stockwell needswork]
Assignee: nobody → hchang
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #13)
> I suppose fixing this intermittent could wait a few days longer. Ethan, I
> suppose we could rewrite that test to make it less flaky by using a promise
> (similar to what we have here [1]). Do you think Henry could take that on
> once he is done with data: URI stuff? If not, let me know and I'll try to
> find someone else.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/browser/base/content/test/
> general/browser_e10s_about_page_triggeringprincipal.js#18

According to the following debug logs [1][2]:

The first 'BrowserTestUtils.browserLoaded' was resolved with 'about:blank',
which might be due to the async API call: 'newTab.linkedBrowser.stop()' [3].
As a result, we may be too early to call 'ContentTask.spawn' to click. 

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=124457419&repo=try&lineNumber=4075
[2] 
20:55:24     INFO - TEST-PASS | dom/base/test/browser_bug902350.js | MixedTest1A: about:blank - 
20:55:24     INFO - TEST-PASS | dom/base/test/browser_bug902350.js | MixedTest1B: https://example.com/browser/dom/base/test/file_bug902350_frame.html - 
20:55:24     INFO - TEST-PASS | dom/base/test/browser_bug902350.js | Mixed Content Doorhanger did not appear when trying to navigate top - 
20:55:24     INFO - TEST-PASS | dom/base/test/browser_bug902350.js | MixedTest1C: https://example.com/browser/dom/base/test/file_bug902350.html - 

[3] http://searchfox.org/mozilla-central/rev/b258e6864ee3e809d40982bc5d0d5aff66a20780/dom/base/test/browser_bug902350.js#32
Attachment #8899355 - Flags: review?(ckerschb)
Comment on attachment 8899355 [details]
Bug 1342298 - Synchronize with the loaded event for the newly added tab then start the test.

https://reviewboard.mozilla.org/r/170590/#review175772

sounds reasonable, let's give this a TRY. r=ckerschb
Attachment #8899355 - Flags: review?(ckerschb) → review+
Just submitted a patch which is proven to work:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e5944341cfc82583cc519f0c7a9536a490c2f3a

The solution is effective but not modern and appealing (but it works.)
Pushed by hchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c89420075a23
Synchronize with the loaded event for the newly added tab then start the test. r=ckerschb
https://hg.mozilla.org/mozilla-central/rev/c89420075a23
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
https://hg.mozilla.org/releases/mozilla-beta/rev/f236b5a4397c
Flags: in-testsuite+
Whiteboard: [stockwell needswork] → [stockwell fixed]
You need to log in before you can comment on or make changes to this bug.