Closed Bug 970728 Opened 10 years ago Closed 9 years ago

Intermittent test_bug344861.html | window.open has correct height,width dimensions - got 0, expected 200

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86_64
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(firefox37 wontfix, firefox38 wontfix, firefox39 wontfix, firefox40 fixed, firefox-esr31 unaffected, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
mozilla40
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- wontfix
firefox40 --- fixed
firefox-esr31 --- unaffected
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: KWierso, Assigned: martijn.martijn)

References

Details

(Keywords: intermittent-failure)

Attachments

(3 files)

https://tbpl.mozilla.org/php/getParsedLog.php?id=34448525&tree=Mozilla-Central
slave: tst-linux64-ec2-112




15:24:45     INFO -  02-10 23:10:22.258   752   752 I GeckoDump: 1219 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 24353848
15:24:45     INFO -  02-10 23:10:22.448   752   752 I GeckoDump: 1220 INFO TEST-END | /tests/docshell/test/navigation/test_bug278916.html | finished in 5561ms
15:24:45     INFO -  02-10 23:10:23.068   752   752 I GeckoDump: 1221 INFO TEST-START | /tests/docshell/test/navigation/test_bug279495.html
15:24:45     INFO -  02-10 23:10:24.978   752   752 I Gecko   : ###################################### forms.js loaded
15:24:45     INFO -  02-10 23:10:25.078   752   752 I Gecko   : ############################### browserElementPanning.js loaded
15:24:45     INFO -  02-10 23:10:25.283   752   752 I Gecko   : ######################## BrowserElementChildPreload.js loaded
15:24:45     INFO -  02-10 23:10:26.008   752   752 I Gecko   : ###################################### forms.js loaded
15:24:45     INFO -  02-10 23:10:26.114   752   752 I Gecko   : ############################### browserElementPanning.js loaded
15:24:45     INFO -  02-10 23:10:26.320   752   752 I Gecko   : ######################## BrowserElementChildPreload.js loaded
15:24:45  WARNING -  02-10 23:10:28.838   752   752 E GeckoConsole: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<html><body>This%20frame%20was%20navigated.</body></html>" line: 0}]
15:24:45  WARNING -  02-10 23:10:28.901   752   752 E GeckoConsole: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,<html><body>This%20frame%20was%20navigated.</body></html>" line: 0}]
15:24:45     INFO -  02-10 23:10:29.606   752   752 E GeckoConsole: [JavaScript Warning: "No meta-viewport tag found. Please explicitly specify one to prevent unexpected behavioural changes in future versions. For more help https://developer.mozilla.org/en/docs/Mozilla/Mobile/Viewport_meta_tag" {file: "data:text/html,<html><body>This%20frame%20was%20navigated.</body></html>" line: 0}]
15:24:45     INFO -  02-10 23:10:29.698   752   752 E GeckoConsole: [JavaScript Warning: "No meta-viewport tag found. Please explicitly specify one to prevent unexpected behavioural changes in future versions. For more help https://developer.mozilla.org/en/docs/Mozilla/Mobile/Viewport_meta_tag" {file: "data:text/html,<html><body>This%20frame%20was%20navigated.</body></html>" line: 0}]
15:24:45     INFO -  02-10 23:10:31.790   752   752 I GeckoDump: 1222 INFO TEST-INFO | MEMORY STAT vsize after test: 127676416
15:24:45     INFO -  02-10 23:10:31.815   752   752 I GeckoDump: 1223 INFO TEST-INFO | MEMORY STAT residentFast after test: 60497920
15:24:45     INFO -  02-10 23:10:31.839   752   752 I GeckoDump: 1224 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 24204452
15:24:45     INFO -  02-10 23:10:32.070   752   752 I GeckoDump: 1225 INFO TEST-END | /tests/docshell/test/navigation/test_bug279495.html | finished in 8990ms
15:24:45     INFO -  02-10 23:10:32.748   752   752 I GeckoDump: 1226 INFO TEST-START | /tests/docshell/test/navigation/test_bug344861.html
15:24:45     INFO -  02-10 23:10:34.088   752   752 I Gecko   : ###################################### forms.js loaded
15:24:45     INFO -  02-10 23:10:34.184   752   752 I Gecko   : ############################### browserElementPanning.js loaded
15:24:45     INFO -  02-10 23:10:34.383   752   752 I Gecko   : ######################## BrowserElementChildPreload.js loaded
15:24:45  WARNING -  02-10 23:10:35.833   752   752 E GeckoConsole: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://mochi.test:8888/" line: 0}]
15:24:45     INFO -  02-10 23:10:36.303   752   752 I GeckoDump: 1227 ERROR TEST-UNEXPECTED-FAIL | /tests/docshell/test/navigation/test_bug344861.html | window.open has correct height dimensions - got 0, expected 200
15:24:45     INFO -  02-10 23:10:36.319   752   752 I GeckoDump: 1228 ERROR TEST-UNEXPECTED-FAIL | /tests/docshell/test/navigation/test_bug344861.html | window.open has correct width dimensions - got 0, expected 200
15:24:45     INFO -  02-10 23:10:36.418   752   752 I GeckoDump: 1229 INFO TEST-INFO | MEMORY STAT vsize after test: 127709184
15:24:45     INFO -  02-10 23:10:36.438   752   752 I GeckoDump: 1230 INFO TEST-INFO | MEMORY STAT residentFast after test: 60497920
15:24:45     INFO -  02-10 23:10:36.449   752   752 I GeckoDump: 1231 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 23992324
15:24:45     INFO -  02-10 23:10:36.618   752   752 E GeckoConsole: [JavaScript Warning: "No meta-viewport tag found. Please explicitly specify one to prevent unexpected behavioural changes in future versions. For more help https://developer.mozilla.org/en/docs/Mozilla/Mobile/Viewport_meta_tag" {file: "http://mochi.test:8888/" line: 0}]
15:24:45     INFO -  02-10 23:10:37.345   752   752 I GeckoDump: 1232 INFO TEST-END | /tests/docshell/test/navigation/test_bug344861.html | finished in 4583ms
15:24:45     INFO -  02-10 23:10:37.989   752   752 I GeckoDump: 1233 INFO TEST-START | /tests/docshell/test/navigation/test_bug386782.html
Awesome, this seems to be nearly perma-fail since yesterday, though the range on inbound is a bit fuzzy from when it went from green to failing intermittently to failing pretty much always.

Boris, I see you wrote this test. Mind taking a look while I try to narrow down the range?
Flags: needinfo?(bzbarsky)
There's no magic here.  The test just checks that the web-exposed behavior is that once a window has opened and loaded it has the right size.  If that's failing, then our UI fucked up somewhere....
Flags: needinfo?(bzbarsky)
As best I can tell, it was bug 858787 that made this near perma-fail. I've backed it out with the most recent round of merges to m-c, so we should see this drop off once the backout merges around and the currently running jobs taper off.
The default policy for certified apps in B2G does not allow inline-styles [0,1]. Flipping the pref to use the spec-compliant parser for enforcing CSP reveals that an inline style is actually used in this testcase [2], which calls [3], which is then 'correctly' banned by CSP and causes the test to fail.

Can someone of the B2G folks talk a look and probably rewrite [3] so it's not using an inline-style?

[0] https://developer.mozilla.org/en-US/Apps/CSP
[1] http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#397
[2] http://mxr.mozilla.org/mozilla-central/source/docshell/test/navigation/test_bug344861.html?force=1#23
[3] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/b2g_start_script.js#25
No longer blocks: 858787
Hey Martjin,

since you wrote [1], I was wondering whether you have any insights how we could make this part CSP compatible.

Thanks!

[1] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/b2g_start_script.js#25
Flags: needinfo?(martijn.martijn)
Why does the mochitest app need to be enforced CSP?

I guess the popupFrame could get its styles from the external test_container.css stylesheet and then changing the relevant cssRules of the popupFrame selector of that stylesheet.
Flags: needinfo?(martijn.martijn) → needinfo?(mozilla)
(In reply to Martijn Wargers [:mwargers] (QA) from comment #73)
> Why does the mochitest app need to be enforced CSP?
> I guess the popupFrame could get its styles from the external
> test_container.css stylesheet and then changing the relevant cssRules of the
> popupFrame selector of that stylesheet.

That is awesome. It would be great if you could go ahead and do that change of loading the styles from test_container.css so we can land 858787 ASAP.
Flags: needinfo?(mozilla)
> Why does the mochitest app need to be enforced CSP?

It doesn't need to be, but unfortunately is because:

1. The test-container needs to be a certified app so it can test certain APIs
2. certified apps have a default CSP

See https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Mochitests#Technical_Implementation_Details and https://bugzilla.mozilla.org/show_bug.cgi?id=858787#c14 for more details.

Maybe this was cargo-culted from an earlier time and could be removed without breaking tests? That would simplify things. I can't understand how it's even making a difference now, due to the short-circuit behavior in nsPrincipal.cpp::GetAppStatus, as described in the last link above.
No longer blocks: 858787
The first two attachments/patches in bug 858787 have fixed the problem described in this bug. I think we can mark this one as resolved. Ryan, are you ok with closing this one?
Flags: needinfo?(ryanvm)
What about comment 78?
Flags: needinfo?(ryanvm)
Fwiw, I'm trying to work on this bug, but I'm stuck on bug 1002545 for now.
(In reply to Martijn Wargers [:mwargers] (QA) from comment #82)
> Fwiw, I'm trying to work on this bug, but I'm stuck on bug 1002545 for now.

This is happening pretty frequently again. Any chance you could take another stab, Martijn? :)
Flags: needinfo?(martijn.martijn)
Attached patch 970728_a.diffSplinter Review
This is the b2g_start_script.js part of the fix.

It turned out impossible for me to test if the changes I made, make this bug go away, because:
- For some reason, test_bug344861.html does pass for me every time (I'm doing ./mach mochitest-remote docshell/test/navigation/ in an emulator setup).
- Every change I made in test_bug344861.html didn't show up at all when I was doing the mochitest-remote run.

But on itself, the changes seem to be working.

I guess this has to land after the test_container.css fix (which is a fix in gaia).
Flags: needinfo?(martijn.martijn)
Attachment #8590779 - Flags: review?(jgriffin)
Comment on attachment 8590783 [details] [review]
[gaia] mwargers:test_container.css > mozilla-b2g:master

test_container.css part of the fix.
Attachment #8590783 - Flags: review?(jgriffin)
Assignee: nobody → martijn.martijn
You can test these on try, by pointing gaia.json to your own github gaia fork.  See https://wiki.mozilla.org/ReleaseEngineering/TryServer#Using_a_custom_Gaia
But that link says: "For Gaia-related tests on B2G desktop builds,". This is for a mochitest (not Gaia related) on the B2G emulator.
(In reply to Martijn Wargers [:mwargers] (QA) from comment #233)
> But that link says: "For Gaia-related tests on B2G desktop builds,". This is
> for a mochitest (not Gaia related) on the B2G emulator.

It works the same...all jobs use gaia.json to load gaia, including for building the emulator.
(In reply to Jonathan Griffin (:jgriffin) from comment #234)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #233)
> > But that link says: "For Gaia-related tests on B2G desktop builds,". This is
> > for a mochitest (not Gaia related) on the B2G emulator.
> 
> It works the same...all jobs use gaia.json to load gaia, including for
> building the emulator.

Actually I'm wrong, RyanVM pointed out emulator builds use sources.xml.  You may be able to test this, by updating the "gaia.git" entry of https://dxr.mozilla.org/mozilla-central/source/b2g/config/emulator/sources.xml

I haven't tried doing this myself on a try job, however.
Comment on attachment 8590783 [details] [review]
[gaia] mwargers:test_container.css > mozilla-b2g:master

I'll rubberstamp since this should be harmless, even though it isn't clear to me how or why this would fix this problem.
Attachment #8590783 - Flags: review?(jgriffin) → review+
Attachment #8590779 - Flags: review?(jgriffin) → review+
(In reply to Jonathan Griffin (:jgriffin) (not reading bugmail until April 20) from comment #236)
> Comment on attachment 8590783 [details] [review]
> [gaia] mwargers:test_container.css > mozilla-b2g:master
> 
> I'll rubberstamp since this should be harmless, even though it isn't clear
> to me how or why this would fix this problem.

https://github.com/mozilla-b2g/gaia/commit/be1a3183dc6027eddf16256f46513314f291c2cf
Thanks, I merged this part, this part is really harmless.

With that landed, I can create a tryserver run with the  970728_a.diff patch on b2g emulator mochitest. I'm not sure when tryserver has picked up the gaia changes, so I'll wait a bit before triggering a tryserver run.
You can wait until b2g-inbound picks up the change via the B2G Bumper Bot (probably in the next 15-20min - look at the commit message for the gaia.json update in the push to get a list of included revisions) and push to Try on top of that, or you'll have to wait until it's merged to m-c from b2g-inbound at some point over the weekend most-likely.
Pushed to try the 970728_a.diff patch now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14f505a0073c
Try is all green.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/437a59be3446
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(In reply to Treeherder Robot from comment #247)
Status: RESOLVED → REOPENED
Flags: needinfo?(martijn.martijn)
Resolution: FIXED → ---
Target Milestone: mozilla40 → ---
The situation in commment 71 isn't there anymore, afaik. Christoph, CSP is enforced here, right?

What I understand from this: http://mxr.mozilla.org/gaia/source/apps/settings/js/settings.js#86
The 980*980 dimensions that get returned as result instead of the expected 200, is because of the async pan zoom enable stuff that sets the initial viewport size to that value.
Kats, do you know why that happens? And why it doesn't return the value 200?
Flags: needinfo?(mozilla)
Flags: needinfo?(martijn.martijn)
Flags: needinfo?(bugmail.mozilla)
I'm not sure I understand what the problem is. For one thing, the 980 that is related to APZ is the width of the default CSS viewport, which on B2G is defined at [1]. Note that the height there is *not* 980, it is 480, whereas comment 249 seems to indicate a height of 980 as well. Also, I don't understand what the settings.js link in comment 250 has to do with the failure in comment 249, as that is specific to the settings app. Or is just that you were looking for where the 980 was coming from, and happened to see that comment?

[1] http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?rev=6816cc58e19b#112
Flags: needinfo?(bugmail.mozilla)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #251)
> failure in comment 249, as that is specific to the settings app. Or is just
> that you were looking for where the 980 was coming from, and happened to see
> that comment?

Yeah, I have no idea where that 980 width and height value is coming from at all.
Honestly I don't know if this test even makes sense on b2g. The skip-if conditions for this test are bizarre, I would say it makes sense to just skip entirely on b2g. window.open on b2g is always going to open a "fullscreen" window since b2g doesn't support setting the size on new windows, so IMO the test is expected to fail.
Well, window.open sizing is covered here:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/b2g_start_script.js#36
And it seems to work locally. I don't know why this intermittent failure is happening at all, at this point.

But I agree, probably easier to just disable completely for b2g.
Attached patch 970728_2.diffSplinter Review
This disables the test for b2g.
Attachment #8592869 - Flags: review?(ryanvm)
Comment on attachment 8592869 [details] [diff] [review]
970728_2.diff

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

rs=me
Attachment #8592869 - Flags: review?(ryanvm) → review+
https://hg.mozilla.org/mozilla-central/rev/4d627d329107
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: