[emulator][mochitest] allow changing network connection on emulator mochitest

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
5 years ago
3 years ago

People

(Reporter: schien, Assigned: schien)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Created attachment 8350603 [details] [diff] [review]
minimal reproducing test case

The iframe.src = "some-page-url" stop functioning after enabling RIL data connection. In this test case, iframe1 is loading test_iframe1.html and change to test_iframe2.html after turning on 3G data. However, we can only found the log for test_iframe1.html. Removing the enableDataConnection() will make the entire test set finish correctly without blocking.

TEST PROCEDURE
run |./mach mochitest-remote Harness_sanity/|

EXPECTED
all test cases are executed.

ACTUAL
test cases cannot continue after test_breaker.html, and do not see the "test_iframe2 is loaded" message in console.
@vicamo has some patches for completely emulating 3G data connection on emulator. These patches are aimed to fix the no default route issue on emulator. However, the bug still exist after manually applying those patches. So, this issue is probably not causing by DNS or network. Need some one to take look at document loading process.

Here is the step-by-step guide for patching the emulator 3G network issue. (thanks to @vicamo!). With this patch you can browsing web page on Browser app inside the emulator after turning on 3G.
#### 1 ####
pull two PRs (external/qemu, hardware/ril)in bug 871475.

#### 2 ####
pull the PR (.repo/manifest) in bug 951053
$ ./repo sync external/iproute2

#### 3 ####
# edit system/core/rootdir/etc/init.goldfish.sh and insert following line after line 2
# Setup networking when boot starts
fconfig eth0 10.0.2.15 netmask 255.255.255.0 up
ip route add default via 10.0.2.2 dev eth0 metric 2

#### 4 ####
$ rm out/target/product/generic/system/etc/init.goldfish.sh
$ ./build.sh
This is the bug specifically about the problems we had with loading the next test in bug 950317.

I'm not 100% sure that schien is correct that DNS/network can be the issue, as I don't know how the mochitests feed the tests from the framework to the device
Flags: needinfo?(mrbkap)
Flags: needinfo?(ahalberstadt)
Adding a few more people who might have ideas about what's up with the b2g mochitest framework here
(In reply to Randell Jesup [:jesup] from comment #2)
> I'm not 100% sure that schien is correct that DNS/network can be the issue,

"can't be the issue", not can.
So in the test case the test is waiting for a message from the parent process before continuing. This could actually be bug 951895. Though if this is the case, then test_breaker.html wouldn't be executed either.. Do you see 'enable radio' in the logs? If not try applying the patch from 951895.
Flags: needinfo?(ahalberstadt)
(In reply to Randell Jesup [:jesup] from comment #2)
> I'm not 100% sure that schien is correct that DNS/network can be the issue,
> as I don't know how the mochitests feed the tests from the framework to the
> device

To answer this question, we just serve all the tests on the host, load up SimpleTest and pass a manifest in via url parameter. SimpleTest then just loads each test one at a time in an iframe.
(In reply to Andrew Halberstadt [:ahal] from comment #5)
> So in the test case the test is waiting for a message from the parent
> process before continuing. This could actually be bug 951895. Though if this
> is the case, then test_breaker.html wouldn't be executed either.. Do you see
> 'enable radio' in the logs? If not try applying the patch from 951895.
Not necessary, this test case is intend to remove all the dependency from other bug. The message we are waiting for is send from the BreakerLoadChromeScript.js which we load for this test case. I see all the log I printed in test_breaker.html on my site.
(In reply to Andrew Halberstadt [:ahal] from comment #6)
> (In reply to Randell Jesup [:jesup] from comment #2)
> > I'm not 100% sure that schien is correct that DNS/network can be the issue,
> > as I don't know how the mochitests feed the tests from the framework to the
> > device
> 
> To answer this question, we just serve all the tests on the host, load up
> SimpleTest and pass a manifest in via url parameter. SimpleTest then just
> loads each test one at a time in an iframe.

"loads each tests" - loads from where?  That's where installing a new data connection could fubar things...
So to summarize, we've figured out how to enable 3g in the mochitests by synthesizing the message-listener-ready event, but once we do so, all subsequent mochitests fail to run. The host serves the tests and the emulator uses a special ip (10.0.2.2) which resolves to 127.0.0.1 on the host [1]. The theory is that enabling data somehow messes with the network and the emulator can no longer reach the tests on the host.

[1] http://developer.android.com/tools/devices/emulator.html#networkaddresses
After setup 3G data, ProxyAutoConfig::GetProxyForURI() is never invoked before DNS resolution. The PAC file we used in mochitest should redirect http://mochi.test:8888 to an local IP address. I think that's the reason we saw DNS error for http://mochi.test.
So could we listen for ondatachange and then invoke it manually, possibly in b2g_start_script again?
I'm not an expert of PAC so I'm not sure if it is normal discarding PAC setting after network change.

@Patrick, mochitest uses PAC file for translating the fake domain name to local IP address. B2G emulator seems discarding the PAC setting after we manually change the network interface in testcase. Should we manually restore the PAC setting after changing network interface?
Flags: needinfo?(mcmanus)
Ok, now I got the whole picture of this bug. The root cause is that the pref "network.proxy.type" is been reset to PROXYCONFIG_SYSTEM after turning on 3G data [1]. Mochitest framework uses user-defined PAC file for mapping mochi.test to local IP address, and overwrite the "network.proxy.type" to PROXYCONFIG_PAC in user preference. Changing proxy type will make Gecko discard the user-defined PAC setting, which will break the mochi.test name lookup.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkService.js#329
Flags: needinfo?(mrbkap)
Flags: needinfo?(mcmanus)
Created attachment 8358338 [details] [diff] [review]
manually revert proxy type

Document loading will success if we manually set "network.proxy.type" back to 2 after turning on 3G data.
Attachment #8358338 - Flags: feedback?(ahalberstadt)
Comment on attachment 8358338 [details] [diff] [review]
manually revert proxy type

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

\o/

Would listening to ondatachange work instead of the timeout?
Attachment #8358338 - Flags: feedback?(ahalberstadt) → feedback+
(In reply to Andrew Halberstadt [:ahal] from comment #16)
> Would listening to ondatachange work instead of the timeout?
There will be 3 datachange events being fired and proxy.type will also being set multiple times. I should reach out to @vicamo for a better timing instead of timeout.
Comment on attachment 8358338 [details] [diff] [review]
manually revert proxy type

Hi @vicamo, I found there will be 3 datachange events being fired after turning on 3g data on emulator. Therefore, I cannot use ondatachange for reverting proxy.type setting. Could you point out a better timing for reverting proxy.type?
Attachment #8358338 - Flags: feedback?(vyang)
Comment on attachment 8358338 [details] [diff] [review]
manually revert proxy type

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

Listening to datachange and check "mozMobileConnections[N].data.connected" would be the most correct way.
Attachment #8358338 - Flags: feedback?(vyang)
After having a quick chat with @ahal and @vchang, we could modify NetworkService.js to restore the system initial proxy setting instead of clear all the user settings [1].

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkService.js?from=NetworkService.js#329
Created attachment 8359587 [details] [diff] [review]
revert-proxy-setting-in-network-service.patch

Revert proxy settings to system initial values instead of default values.
Assignee: nobody → schien
Attachment #8358338 - Attachment is obsolete: true
Attachment #8359587 - Flags: review?(vchang)
@ahal, you can try revert-proxy-setting-in-network-service.patch and see if it solves the problem while running mochitest on real device.
Flags: needinfo?(ahalberstadt)
David Clarke is the one working on the device mochitests (bug 915810). He had already hardcoded the pref as a work-around, but it seems like we are still hitting another unrelated problem. I think he's rebuilding with your patch applied now just to see.
Flags: needinfo?(ahalberstadt)
@schien, i just tested this patch, and it works from my perspective.  Your patch doesn't apply cleanly, could you respin it. 

the other problems i'm seeing aren't related to this patch.
Created attachment 8359629 [details] [diff] [review]
revert-proxy-setting-in-network-service.patch

I forgot that I reverted my repository back to last Friday because of other issues. Rebase to latest m-c.
Attachment #8359587 - Attachment is obsolete: true
Attachment #8359587 - Flags: review?(vchang)
Attachment #8359629 - Flags: review?(vchang)
Summary: [emulator][mochitest] enable RIL data connection on emulator mochitest → [emulator][mochitest] allow changing network connection on emulator mochitest
Created attachment 8360270 [details] [diff] [review]
Part 2 - gonk-network-testcase.patch

add mochitest case to dom/system/gonk/tests/mochitest
Attachment #8350603 - Attachment is obsolete: true
Attachment #8360270 - Flags: review?(vchang)
Comment on attachment 8359629 [details] [diff] [review]
revert-proxy-setting-in-network-service.patch

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

The patch looks good. But I feel that I am not familiar with mochitest, I would like to redirect the review to vicamo.
Attachment #8359629 - Flags: review?(vchang) → review?(vyang)
Comment on attachment 8359629 [details] [diff] [review]
revert-proxy-setting-in-network-service.patch

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

The patch looks good. But I feel that I am not familiar with mochitest, I would like to redirect the review to vicamo.
Attachment #8359629 - Flags: review?(vchang) → review?(vyang)
Attachment #8359629 - Flags: review?(vyang)
Comment on attachment 8360270 [details] [diff] [review]
Part 2 - gonk-network-testcase.patch

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

Redirect to vicamo :-)
Attachment #8360270 - Flags: review?(vchang) → review?(vyang)
When combined with bug 950317, test_switch_dataConnection.html doesn't exit (see try results in that bug)
Created attachment 8360844 [details] [diff] [review]
Part 1 - revert-proxy-setting-in-network-service.patch

Update the reviewer info on hg commit log.
Attachment #8359629 - Attachment is obsolete: true
Attachment #8359629 - Flags: review?(vyang)
Attachment #8360844 - Flags: review?(vyang)
Created attachment 8360848 [details] [diff] [review]
Part 2 - gonk-network-testcase.patch, v2

Reset data connection before test and update the reviewer info in hg commit log.

Let's see how it works with WebRTC mochitest enabled.
https://tbpl.mozilla.org/?tree=Try&rev=c71da9db09d0
Attachment #8360270 - Attachment is obsolete: true
Attachment #8360270 - Flags: review?(vyang)
Attachment #8360848 - Flags: review?(vyang)
No longer blocks: 950317
Comment on attachment 8360844 [details] [diff] [review]
Part 1 - revert-proxy-setting-in-network-service.patch

Remove review request because this patch might store wrong proxy setting as system initial value.
Attachment #8360844 - Flags: review?(vyang)
Attachment #8360848 - Flags: review?(vyang)
Blocks: 915810
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.