992772 - [B2G][RIL][NetworkManager] ResolveHostName with the DNS of the specified NetworkInterface.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

In Bug 991988, we realized that it might take too much time to resolve the hostname of MMSC/MMS Proxy when WiFi is ON, and the root cause is that
1. Some host names such as MMSC/MMS Proxy are reachable only in specified networkInterface (for example, mms data connection).
2. It wastes time to resolve these kinds of hostnames via other network interfaces such as Wifi.

Fire this bug to see if we have better solution to resolve the the hostname more specifically.

Comment 1

[Ken Chang[:kenkai]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #0) 
> Fire this bug to see if we have better solution to resolve the the hostname
> more specifically.
One straightforward idea is whether gecko can specify the dns server while looking up a hostname. But I am not if there is this command supported in lib. Need to have a check.

Comment 2

[Julien Wajsberg [:julienw]]

And is the hostname finally resolved?

If Gecko uses the default DNS set up for the connection (that we get by DHCP), I guess it might never get an IP for this domain if the domain is private.

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] from comment #2)
> And is the hostname finally resolved?
> 
> If Gecko uses the default DNS set up for the connection (that we get by
> DHCP), I guess it might never get an IP for this domain if the domain is
> private.

The host will be resolved eventually with correct DNS, but 
we took too much time to resolve it with wrong DNS.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Update finding so far:
1. The |getaddrinfo| provided in C library of netdb.h is used to resolve hostname to IP address.
2. However, in this API There is no way to specify the DNS server to resolve the hostname.
3. In AOSP, there is a hack inside getaddrinfo and netd that
   a. Allow App to specify the DNS with PID into netd when App setup a specific connection like MMS.
   b. Inside getaddrinfo, getPid() is used to identify which DNS the app prefers to use.
   c. Hence, while an App would like to resolve a hostname, 
      the DNS with same PID will be used in 1st priorty.
This design is workable in Android but not possible in current gecko design:
1. All the connections are established by gecko.
2. MMS transactions are also handled in gecko.
3. PID becomes useless to identify the PID specific DNS to be queried.

[1] http://man7.org/linux/man-pages/man3/getaddrinfo.3.html

Comment 5

[Ken Chang[:kenkai]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #4)
> This design is workable in Android but not possible in current gecko design:
> 1. All the connections are established by gecko.
> 2. MMS transactions are also handled in gecko.
> 3. PID becomes useless to identify the PID specific DNS to be queried.

It seems that we don't have a good solution for this now. By the way, because network service store DNS server address into "net.dns.i" now and netbsd would query all "net.dns.i" for resolving hostname, we might be able to take this advantage to fix this bug.

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Ken Chang[:ken] from comment #5)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #4)
> > This design is workable in Android but not possible in current gecko design:
> > 1. All the connections are established by gecko.
> > 2. MMS transactions are also handled in gecko.
> > 3. PID becomes useless to identify the PID specific DNS to be queried.
> 
> It seems that we don't have a good solution for this now. By the way,
> because network service store DNS server address into "net.dns.i" now and
> netbsd would query all "net.dns.i" for resolving hostname, we might be able
> to take this advantage to fix this bug.

Bad news is that the system properties of "net.dns%d" is deprecated since JB 4.3. :(

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

After comparing different revision of AOSP, I found that since JB 4.3, the netd allows to accept |getaddrinfo| request with specified |iface|.

See the following codes for reference:
http://androidxref.com/4.3_r2.1/xref/bionic/libc/netbsd/net/getaddrinfo.c#449
http://androidxref.com/4.3_r2.1/xref/system/netd/DnsProxyListener.cpp#192
http://androidxref.com/4.3_r2.1/xref/system/netd/DnsProxyListener.cpp#120

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #7)
> http://androidxref.com/4.3_r2.1/xref/system/netd/DnsProxyListener.cpp#120
It shall be line#131 instead.
http://androidxref.com/4.3_r2.1/xref/system/netd/DnsProxyListener.cpp#131

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

In NetworkManager, we are able to resolve hostname by netd with specified iface.

Next topic is to see how can we specific the interface of getaddrinfo in all the call path in gecko.
Take MMS for example, MmsService use XMLHttpRequest for all the MMS Connections.
Then,
1. How does XMLHttpRequest resolve MMSC/MMS Proxy hostname correctly?
2. Are all the call path going through nsIDNSService?

Comment 10

[Ken Chang[:kenkai]]

Put this bug into backlog.

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Update finding so far:
1. 3 APIs from netdb.h is inherit by AOSP to accept additional |iface| parameter [1]:
   a. getaddrinfo() -> android_getaddrinfoforiface()
   b. getnameinfo() -> android_getnameinfoforiface()
   c. gethostbyname2() -> android_gethostbynameforiface()

2. The corresponding APIs in gecko are defined in prnetdb.h [2]:
   a. getaddrinfo() -> PR_GetAddrInfoByName()
      - the only call path in gecko is from nsHostResolver -> nsDNSService2::AsycResolve()/Resolve() 
   b. getnameinfo() -> PR_NetAddrToString() and is used by:
      - ProxyAutoConfig, nsProtocolProxyService, nsSocketTransportService, SSL, etc.
   c. gethostbyname2() -> PR_GetIPNodeByName()
      - Not used.

Hence, the main principle to support getaddrinfo/getnameinfo by 
selected NetworkInterface could be:
1. Provide a mapping amoung { hostname, addr, iface }in NetworkManager, for example.
2. Provide 2 new APIs in NetworkManager called getInterfaceByName() and getInterfaceByAddr()
3. Defines 3 new APIs in prnetdb.h as followed:
   - PR_GetAddrInfoByNameForIface(), PR_NetAddrToStringForIface(), PR_GetIPNodeByNameForIface()
Then, for any scenario that the interface is needed to specify:
1. Get |iface| from NetworkManager via ByName or ByAddr.
2. Invoke PR_XxxForIface() if |iface| is available. Otherwise, the original PR_Xxx() is used.

[1] http://androidxref.com/4.4.2_r2/xref/bionic/libc/include/netdb.h
[2] http://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/include/prnetdb.h

Comment 12

[Ken Chang[:kenkai]]

Assign it to Vicamo, it is a blocker for some carriers, we need to try to fix it before 6/20

Comment 13

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Ken, Vicamo, just wonder if there is any update on this issue?

Thanks

Vance

Comment 14

[Ken Chang[:kenkai]]

We still don't have a good solution now...:-(.

Comment 15

[Ken Chang[:kenkai]]

Henry, as offline discussion, please take this bug.

Comment 16

[Henry Chang [:hchang]]

I tend to add a new network service to invoke android_gethostbynameforiface
directly. "Get host by name for certain interface" may be too android specific 
to be a "PR" function or to add to nsIDNSService.

Comment 17

[Henry Chang [:hchang]]

Attachment: WIP.patch

Proposed approach after JB:

1) Add a network service "resolveHostnameForInterface",
   which works with "setDNS" to use the name server
   that we bound to certain interface before.

2) Resolve the mmsc/mmsproxy [1] by using NetworkService.resolveHostnameForInterface

3) Pre-resolve mmsc/mmsproxy [2] in MmsService.js since we
   have no way to bind XMLHttpRequest to certain name server.

If we need to also support ICS, the only solution is to implement 
getaddrinfo on our own ...

[1] http://hg.mozilla.org/mozilla-central/file/70be728521e3/dom/system/gonk/NetworkManager.js#l536

[2] http://hg.mozilla.org/mozilla-central/file/70be728521e3/dom/mobilemessage/src/gonk/MmsService.js#l218

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

Sorry for jumping in

In bug 1032097, 
1. 2 APIs will be provided in NetworkManager:
  /**
   * Add extra host route to the specified network into routing table.
   *
   * @param network
   *        The Network type. One of the NETWORK_TYPE_* constants.
   * @param serviceId
   *        The serviceId of the selected Network type.
   * @param host
   *        The host to be added.
   *
   * @return a Promise
   *         resolved if added; rejected, otherwise.
   */
  jsval addExtraHostRoute(in long type,
                          in unsigned long serviceId,
                          in DOMString host);

  /**
   * Remove extra host route to the specified network from routing table.
   *
   * @param network
   *        The Network type. One of the NETWORK_TYPE_* constants.
   * @param serviceId
   *        The serviceId of the selected Network type.
   * @param host
   *        The host to be removed.
   *
   * @return a Promise
   *         resolved if removed; rejected, otherwise.
   */
  jsval removeExtraHostRoute(in long type,
                             in unsigned long serviceId,
                             in DOMString host);
2. In these 2 APIs, 
   - gDNSService.asyncResolve() will be used for DNS resolving if the host is not a ip-address.
   - gNetworkService.addHostRoute/gNetworkService.removeHostRoute will be used to update the routing.

Based on this assumption, will gDNSService.asyncResolve() be enhanced with networkInterface specified?

Comment 19

[Henry Chang [:hchang]]

Attachment: WIP V2

Comment 20

[Henry Chang [:hchang]]

Attachment: netd

Comment 21

[Henry Chang [:hchang]]

Attachment: libc.so

Comment 22

[Henry Chang [:hchang]]

Attached WIP V2 which is verified working on emulator. Testing on flame
with base image v122, you have to replace libc.so and netd since the 
base image doesn't include proper ones. Waiting for T2M's response.

$ adb remount && adb push netd system/bin && adb push libc.so system/lib && adb reboot

Comment 23

[Ralph Daub [:rdaub]]

Hi Bevis and Henry,

Is there any new development on this issue, or an estimated timeframe? It blocks bug 1053650, making users unable to send/receive MMS when connected to a Wi-Fi, which can be a pretty common scenario.

Thanks in advance.

- Ralph

Comment 24

[Henry Chang [:hchang]]

This issue is still blocked by Bug 1053650 where we are looking for the best solution. The necko peer just gave us a feedback and the final approach is not decided yet. Thanks!

Comment 25

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

[Tracking Requested - why for this release]:
nominate tracking for it's blocking various MMS bugs.

Comment 26

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Eric,
Can we put the relevant MMS testing into MozTrap? (refer to dependent bugs)
Seems like these are known issues for a while, but let's keep track.

Comment 27

[Eric Chang [:ericcc] [:echang]]

Let me check with Hsinyi to schedule a test run, and also the impact of the change, thanks
https://moztrap.mozilla.org/manage/suites/?pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-status=active&filter-name=%5Bcomms&filter-name=messages

Comment 29

[Henry Chang [:hchang]]

Attachment: Bug992772.patch

Comment 30

[Henry Chang [:hchang]]

Applying the patches in Bug 1108957 and Bug 1053650

Attachment 8545599 [details] [diff]

Attachment 8545600 [details] [diff]
Attachment 8545601 [details] [diff]
Attachment 8545602 [details] [diff]

and the patch in this bug (Attachment 8545603 [details] [diff]) should fix the mms issue.

Comment 31

[Ralph Daub [:rdaub]]

Hi Henry Chang,

Can you please confirm if the patch submitted fixes issue described in bug 1046517 - "Unable to send/receive MMS when Wi-Fi is turned on"?

A user in the SUMO forums is reporting the issue being tracked in bug 1046517, which appears to ultimately be dependent on this bug 992772.

Thanks in advance for your insight.

- Ralph

Comment 32

[Henry Chang [:hchang]]

(In reply to Ralph Daub [:rdaub] from comment #31)
> Hi Henry Chang,
> 
> Can you please confirm if the patch submitted fixes issue described in bug
> 1046517 - "Unable to send/receive MMS when Wi-Fi is turned on"?
> 
> A user in the SUMO forums is reporting the issue being tracked in bug
> 1046517, which appears to ultimately be dependent on this bug 992772.
> 
> Thanks in advance for your insight.
> 
> - Ralph

Yes! This bug is to support the per-interface DNS query and Bug 1046517
is caused by this bug as described by Bevis.

Comment 33

[Ralph Daub [:rdaub]]

(In reply to Henry Chang [:henry] from comment #32)
> (In reply to Ralph Daub [:rdaub] from comment #31)
> > Can you please confirm if the patch submitted fixes issue described in bug
> > 1046517 - "Unable to send/receive MMS when Wi-Fi is turned on"?
> > - Ralph
> 
> Yes! This bug is to support the per-interface DNS query and Bug 1046517
> is caused by this bug as described by Bevis.

Hi :henry and :btseng, I can still replicate the issue described in bug 1046517 - "Unable to send/receive MMS when Wi-Fi is turned on". I am running the latest Flame v2.2 image on top of v18D base image.

I am reopening bug 1046517 to track this.

- Ralph

Comment 34

[Henry Chang [:hchang]]

(In reply to Ralph Daub [:rdaub] from comment #33)
> (In reply to Henry Chang [:henry] from comment #32)
> > (In reply to Ralph Daub [:rdaub] from comment #31)
> > > Can you please confirm if the patch submitted fixes issue described in bug
> > > 1046517 - "Unable to send/receive MMS when Wi-Fi is turned on"?
> > > - Ralph
> > 
> > Yes! This bug is to support the per-interface DNS query and Bug 1046517
> > is caused by this bug as described by Bevis.
> 
> Hi :henry and :btseng, I can still replicate the issue described in bug
> 1046517 - "Unable to send/receive MMS when Wi-Fi is turned on". I am running
> the latest Flame v2.2 image on top of v18D base image.
> 
> I am reopening bug 1046517 to track this.
> 
> - Ralph

Since all the related patches have not landed yet, the issue is still not
fixed. By the way, one of the dependent bugs (Bug 1108957) has a new version
of patch and I am testing it right now. Thanks!

Comment 35

[Henry Chang [:hchang]]

Attachment: Bug992772.patch

Comment 36

[Henry Chang [:hchang]]

Since the latest patch in Bug 1108975 has been tested and is going to be
reviewed, I updated the patches on top of it and just wait for its landing.

Comment 37

[Henry Chang [:hchang]]

Attachment: Bug992772.patch

This version of patch removes the testing code.

Comment 38

[Henry Chang [:hchang]]

Since the dependent bug 1108957 has landed, I can continue to work on getting these patches landed now!

Comment 39

[Henry Chang [:hchang]]

Attachment: Bug992772.patch

Comment 40

[Henry Chang [:hchang]]

Attachment: Bug992772.patch

Comment 41

[Henry Chang [:hchang]]

(In reply to Henry Chang [:henry] from comment #40)
> Created attachment 8572971 [details] [diff] [review]
> Bug992772.patch

Let me explain what this patches does:

1) Implement nsINetworkService.getNetId which is a async call to query
   the netId associated with a network interface name.

2) MmsService.js uses this API prior to sending a XMLHttpRequest.
   When sending XMLHttpRequest, tag the obtained netId to make the 
   request occurs on specific network interface.

3) |netId| would be interpreted differently on KK or LL be considered
   as an identifier to communicate with netd.
   On LL, |netId| is an integer string to represent a network.
   On KK, |netId| is simply the interface name. You can regard it as
   the fallback.

4) nsINetworkService.setDNS no longer sets to default interface. 
   Instead, nsINetworkService.setDefaultRoute would set the default 
   DNS interface. Based on this change, we can always call nsINetworkService.setDNS
   whenever a new network interface gets connected.

Comment 42

[Henry Chang [:hchang]]

Attachment: Bug992772-mms-dns.png

Comment 43

[Henry Chang [:hchang]]

Attachment: Bug992772.patch

Comment 44

[Edgar Chen [:edgar]]

Comment on attachment 8573753 [details] [diff] [review]
Bug992772.patch

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

Thank for this patch, please see my comments below.

::: dom/system/gonk/NetworkManager.js
@@ +860,4 @@
>      // TODO: Bug 992772 - Resolve the hostname with specified networkInterface.
> +    gDNSService.asyncResolveExtended(hostname,
> +                                     flags,
> +                                     network.name,

Looks like we should use the "netId" retrieved from NetworkService here, too.

::: dom/system/gonk/nsINetworkService.idl
@@ +494,5 @@
> +   *        The network interface name which we want to query.
> +   *
> +   * @return A deferred promise that resolves with a string to indicate.
> +   *         the queried netId on success and rejects if the interface name
> +   *         is invalid.  

nit: trailing whitespace.

@@ +495,5 @@
> +   *
> +   * @return A deferred promise that resolves with a string to indicate.
> +   *         the queried netId on success and rejects if the interface name
> +   *         is invalid.  
> +   * 

Ditto.

Comment 45

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8573753 [details] [diff] [review]
Bug992772.patch

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

Thanks for your hard work on this bug.

I found several issues to be addressed:
1. Error handling is required for both ensureRouting() & getNetId() in MmsService.
2. getNetId() is required for DNS query in NetworkManager.
3. The logic to the result of getNetId() looks ambiguous.
4. A followup bug to move netId mapping to NetworkManager is required.

In addition, please make sure that the call flows of both positive/negative responses in 
both KK & L are verified before landing these patches.

Fore more detailed info, please see my comments inline. :)

::: dom/mobilemessage/gonk/MmsService.js
@@ +723,5 @@
> +        let onEnsureRoutingFailed = function (aError) {
> +          debug("Failed to ensureRouting: " + aError);
> +          cancellable.done(_HTTP_STATUS_FAILED_TO_ROUTE);
> +        };
> +

Error handling is required for both ensureRouting() and gNetworkService.getNetId().
Have a generic error callback makes more sense to me:
        let onRejected = function (error) {
          debug(error);
          cancellable.done(_HTTP_STATUS_FAILED_TO_ROUTE);
        };

@@ +728,3 @@
>          mmsConnection.ensureRouting(url)
> +          .then(() => gNetworkService.getNetId(mmsConnection.networkInterface.name),
> +                onEnsureRoutingFailed)

Please replace this to:
                (error) => onRejected("Failed to ensureRouting: " + error))

@@ +728,4 @@
>          mmsConnection.ensureRouting(url)
> +          .then(() => gNetworkService.getNetId(mmsConnection.networkInterface.name),
> +                onEnsureRoutingFailed)
> +          .then((netId) => startTransaction(netId));

Please add error handling for getNetId():
               (error) => onRejected("Failed to getNetId: " + error));

@@ +728,4 @@
>          mmsConnection.ensureRouting(url)
> +          .then(() => gNetworkService.getNetId(mmsConnection.networkInterface.name),
> +                onEnsureRoutingFailed)
> +          .then((netId) => startTransaction(netId));

Please also create a follow-up bug to have netId managed in NetworkManager and have comments here and related implementation.

Thanks!

@@ +747,5 @@
>          let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
>                    .createInstance(Ci.nsIXMLHttpRequest);
>  
>          // Basic setups
> +        xhr.networkInterfaceId = netId;

Does this imply that we will always have a netId(ifname before L and netId in L) for mms transactions even shared APN (mobile internet) is applied?

::: dom/system/gonk/NetworkManager.js
@@ +860,4 @@
>      // TODO: Bug 992772 - Resolve the hostname with specified networkInterface.
> +    gDNSService.asyncResolveExtended(hostname,
> +                                     flags,
> +                                     network.name,

getNetId() is required for DNS query.

::: dom/system/gonk/NetworkService.js
@@ +703,5 @@
> +    };
> +
> +    return new Promise((aResolve, aReject) => {
> +      this.controlMessage(params, function(result) {
> +        if (!result.ret) {

This looks weird to me.
Shall we identify error with 'result.error' instead?
In CommandResult::CommandResult(int32_t aResultCode), mRet will always be 'true' even aResultCode is not SUCCESS. [1]
Is this a bug in NetworkUtils?
In addition, in this patch, ret will be set to 'false' when 'netId' is vaild in NetworkUtils.cpp.
This looks quite ambiguous.

Did we test this both in KK and L?

[1] https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/NetworkUtils.cpp#428

@@ +704,5 @@
> +
> +    return new Promise((aResolve, aReject) => {
> +      this.controlMessage(params, function(result) {
> +        if (!result.ret) {
> +          aReject();

It will be nice to know the reason of the failure cause to the caller.
Shall we replace this to:
        aReject(result.reason);

::: dom/system/gonk/NetworkUtils.cpp
@@ +2510,5 @@
> +  NetIdManager::NetIdInfo netIdInfo;
> +  if (-1 == mNetIdManager.lookup(GET_FIELD(mIfname), &netIdInfo)) {
> +    return -1;
> +  }
> +  result.mRet = false;

Shall we set this to 'true' instead.

Comment 46

[Henry Chang [:hchang]]

Attachment: Bug992772 v2

Comment 47

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8573753 [details] [diff] [review]
Bug992772.patch

Please see comment 45.

Comment 48

[Henry Chang [:hchang]]

Attachment: Bug992772 v2

Comment 49

[Henry Chang [:hchang]]

Comment on attachment 8575934 [details] [diff] [review]
Bug992772 v2

Hi Edgar, Bevis,

Your comments are addressed in patch v2. Could you please have a review
again? Thanks!

Comment 50

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8575934 [details] [diff] [review]
Bug992772 v2

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

Looks good to me.
r=me after the issue inline is addressed.

Thanks! :)

::: dom/system/gonk/NetworkUtils.cpp
@@ +1683,5 @@
>      return CommandResult::Pending();
>    }
>    if (SDK_VERSION >= 18) {
>      // JB, KK.
>      static CommandFunc COMMAND_CHAIN[] = {

Since per interface DNS query is only supported if SDK_VERSION >= KK(19) according to the patch in bug 1108957, if we don't setDefaultInterface here for JB (SDK_VERSION == 18), there will be a regression for MMS when MMS is the only active data connection. :(

Hence, we have to ensure the behavior the same for JB.

Comment 51

[Edgar Chen [:edgar]]

Comment on attachment 8575934 [details] [diff] [review]
Bug992772 v2

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

Nice, thank you.

::: dom/system/gonk/nsINetworkService.idl
@@ +492,5 @@
> +   *
> +   * @param interfaceName
> +   *        The network interface name which we want to query.
> +   *
> +   * @return A deferred promise that resolves with a string to indicate.

Nit: Remove the period at the end of this line.

Comment 52

[Henry Chang [:hchang]]

Attachment: Bug992772 v3 (r+'d, rebased to mc, verified on lollipop)

Comment 53

[Ken Chang[:kenkai]]

Blocking bug 1152568, this bug should be a 2.2+ bug as well. If you think 1152568 should be a duplicate, just change the status. Thanks.

Comment 54

[Henry Chang [:hchang]]

Attachment: Bug992772 v4 (r+'d, rebased to mc, verified on lollipop)

Comment 55

[Henry Chang [:hchang]]

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

Comment 56

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Henry Chang [:henry] from comment #54)
> Created attachment 8591463 [details] [diff] [review]
> Bug992772 v4 (r+'d, rebased to mc, verified on lollipop)

Hi Henry,

Sorry to remove "checkin-needed" for now.
I just found that the fix of bug 1152531 was not included in this patch.
We might need your help to rebase again to include the fix of bug 1152531 before landing.

Thanks!

Comment 57

[Henry Chang [:hchang]]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #56)
> (In reply to Henry Chang [:henry] from comment #54)
> > Created attachment 8591463 [details] [diff] [review]
> > Bug992772 v4 (r+'d, rebased to mc, verified on lollipop)
> 
> Hi Henry,
> 
> Sorry to remove "checkin-needed" for now.
> I just found that the fix of bug 1152531 was not included in this patch.
> We might need your help to rebase again to include the fix of bug 1152531
> before landing.
> 
> Thanks!

Thanks Bevis!

Sorry for not rebasing it first :p

Comment 58

[Henry Chang [:hchang]]

Attachment: Bug992772 v5 (r+'d, rebased to mc, verified on JB/KK/L)

Comment 59

[Ryan VanderMeulen [:RyanVM]]

The webidl changes need DOM peer review.

Comment 60

[Henry Chang [:hchang]]

Comment on attachment 8592666 [details] [diff] [review]
Bug992772 v5 (r+'d, rebased to mc, verified on JB/KK/L)

Hi Blake,

Sorry for bothering but could you please help review the change of NetworkOptions.webidl? We add a new member to |NetworkResultOptions|
to make it carry the result we desire. Thanks!

Comment 61

[Henry Chang [:hchang]]

Thanks Blake!

Comment 62

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/81c1ca9e6344

Comment 63

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/81c1ca9e6344

Comment 64

[bhavana bajaj [:bajaj]]

:henry, can you please fill the approval request comment here?

Comment 65

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #63)
> https://hg.mozilla.org/mozilla-central/rev/81c1ca9e6344

Thanks Henry, Bevis, and all the contributors :)

Comment 66

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #65)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #63)
> > https://hg.mozilla.org/mozilla-central/rev/81c1ca9e6344
> 
> Thanks Henry, Bevis, and all the contributors :)

It's Henry's effort, actually. \o/

Comment 67

[Henry Chang [:hchang]]

(In reply to bhavana bajaj [:bajaj] from comment #64)
> :henry, can you please fill the approval request comment here?

I am making a patch for 2.2 and will request for approval once I finish it.
Thanks!

Comment 68

[bhavana bajaj [:bajaj]]

Comment on attachment 8592666 [details] [diff] [review]
Bug992772 v5 (r+'d, rebased to mc, verified on JB/KK/L)

clearing the approval as I am waiting for a ptch from henry.

Comment 69

[Henry Chang [:hchang]]

Attachment: Bug992772_v2.2.patch (for v2.2)

Comment 70

[Henry Chang [:hchang]]

Attachment: Bug992772_v2.2.patch (for v2.2)

Comment 71

[Henry Chang [:hchang]]

Comment on attachment 8597095 [details] [diff] [review]
Bug992772_v2.2.patch (for v2.2)

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): MMS
User impact if declined: User may not be able to send MMS while using domain name proxy
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): No
String or UUID changes made by this patch: nsINetworkService.idl

Comment 73

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0c613646c0c9