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

RESOLVED FIXED in Firefox 40

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: bevis, Assigned: hchang)

Tracking

unspecified
2.2 S11 (1may)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:2.2+, firefox38 wontfix, firefox39 wontfix, firefox40 fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [caf priority: p2][CR 820235][grooming][p=5] [SUMO-b2g])

Attachments

(3 attachments, 15 obsolete attachments)

31.31 KB, image/png
Details
24.76 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
23.75 KB, patch
Details | Diff | Splinter Review
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.
(In reply to Bevis Tseng [:bevistseng] (btseng@mozilla.com) 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.
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.
(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.
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
(In reply to Bevis Tseng [:bevistseng] (btseng@mozilla.com) 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.
(In reply to Ken Chang[:ken] from comment #5)
> (In reply to Bevis Tseng [:bevistseng] (btseng@mozilla.com) 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. :(
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
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?
Blocks: 939026
Put this bug into backlog.
blocking-b2g: --- → backlog
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
Assign it to Vicamo, it is a blocker for some carriers, we need to try to fix it before 6/20
Assignee: nobody → vyang
Target Milestone: --- → 2.0 S4 (20june)
Blocks: 998244
Hi Ken, Vicamo, just wonder if there is any update on this issue?

Thanks

Vance
Flags: needinfo?(vyang)
Flags: needinfo?(kchang)
We still don't have a good solution now...:-(.
Flags: needinfo?(kchang)
Blocks: 1032064
Blocks: 1031656
No longer blocks: 939026
Henry, as offline discussion, please take this bug.
Assignee: vyang → hchang
Flags: needinfo?(vyang)
Assignee

Updated

5 years ago
Whiteboard: [p=5]
Target Milestone: 2.0 S4 (20june) → 2.1 S2 (15aug)
Blocks: 1046517
Assignee

Updated

5 years ago
Depends on: 1043114
Assignee

Comment 16

5 years ago
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.
Assignee

Comment 17

5 years ago
Posted patch WIP.patch (obsolete) — Splinter Review
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
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?
Assignee

Updated

5 years ago
Depends on: 1032097
Assignee

Comment 19

5 years ago
Posted patch WIP V2 (obsolete) — Splinter Review
Attachment #8470595 - Attachment is obsolete: true
Assignee

Comment 20

5 years ago
Posted file netd (obsolete) —
Assignee

Comment 21

5 years ago
Posted file libc.so (obsolete) —
Assignee

Comment 22

5 years ago
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
Blocks: 1053650
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
Flags: needinfo?(hchang)
Flags: needinfo?(btseng)
Assignee

Comment 24

5 years ago
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!
Flags: needinfo?(hchang)
Flags: needinfo?(btseng)
Assignee

Updated

5 years ago
Depends on: 1109452
[Tracking Requested - why for this release]:
nominate tracking for it's blocking various MMS bugs.
tracking-b2g: --- → ?
Depends on: 1108957
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.
Flags: in-moztrap?(echang)
Blocks: 1115486
Flags: in-moztrap?(echang) → in-moztrap+
Duplicate of this bug: 1115482
Assignee

Comment 29

5 years ago
Posted patch Bug992772.patch (obsolete) — Splinter Review
Attachment #8470684 - Attachment is obsolete: true
Attachment #8470686 - Attachment is obsolete: true
Attachment #8470687 - Attachment is obsolete: true
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
Whiteboard: [p=5] → [p=5] [SUMO-b2g]

Updated

5 years ago
Flags: needinfo?(hchang)
Assignee

Comment 32

5 years ago
(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.
Flags: needinfo?(hchang)
QA Whiteboard: [COM=RIL]
(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
No longer blocks: 1046517
Assignee

Comment 34

5 years ago
(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!
Assignee

Comment 35

5 years ago
Posted patch Bug992772.patch (obsolete) — Splinter Review
Attachment #8545603 - Attachment is obsolete: true
Assignee

Comment 36

5 years ago
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.
Assignee

Comment 37

5 years ago
Posted patch Bug992772.patch (obsolete) — Splinter Review
This version of patch removes the testing code.
Attachment #8555732 - Attachment is obsolete: true
Whiteboard: [p=5] [SUMO-b2g] → [grooming][p=5] [SUMO-b2g]
Assignee

Comment 38

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

Comment 39

4 years ago
Posted patch Bug992772.patch (obsolete) — Splinter Review
Attachment #8556229 - Attachment is obsolete: true
Assignee

Comment 40

4 years ago
Posted patch Bug992772.patch (obsolete) — Splinter Review
Attachment #8571902 - Attachment is obsolete: true
Assignee

Comment 41

4 years ago
(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.
Assignee

Comment 42

4 years ago
Assignee

Comment 43

4 years ago
Posted patch Bug992772.patch (obsolete) — Splinter Review
Attachment #8572971 - Attachment is obsolete: true
Assignee

Updated

4 years ago
Attachment #8573753 - Flags: review?(echen)
Attachment #8573753 - Flags: review?(btseng)
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.
Attachment #8573753 - Flags: review?(echen)
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.
Assignee

Comment 46

4 years ago
Posted patch Bug992772 v2 (obsolete) — Splinter Review
Comment on attachment 8573753 [details] [diff] [review]
Bug992772.patch

Please see comment 45.
Attachment #8573753 - Flags: review?(btseng)
Assignee

Comment 48

4 years ago
Posted patch Bug992772 v2 (obsolete) — Splinter Review
Attachment #8575915 - Attachment is obsolete: true
Assignee

Comment 49

4 years ago
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!
Attachment #8575934 - Flags: review?(echen)
Attachment #8575934 - Flags: review?(btseng)
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.
Attachment #8575934 - Flags: review?(btseng) → review+
Attachment #8573753 - Attachment is obsolete: true
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.
Attachment #8575934 - Flags: review?(echen) → review+
blocking-b2g: backlog → ---
Assignee

Comment 52

4 years ago
Attachment #8575934 - Attachment is obsolete: true
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.
Blocks: 1152568
blocking-b2g: --- → 2.2+
Assignee

Updated

4 years ago
Attachment #8590204 - Attachment is obsolete: true
Assignee

Updated

4 years ago
Keywords: checkin-needed
(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!
Flags: needinfo?(hchang)
Keywords: checkin-needed
Assignee

Comment 57

4 years ago
(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
Flags: needinfo?(hchang)
Assignee

Updated

4 years ago
Attachment #8591463 - Attachment is obsolete: true
Assignee

Updated

4 years ago
Keywords: checkin-needed
The webidl changes need DOM peer review.
Keywords: checkin-needed
Assignee

Comment 60

4 years ago
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!
Attachment #8592666 - Flags: review?(mrbkap)
Target Milestone: 2.1 S2 (15aug) → 2.2 S11 (1may)
Attachment #8592666 - Flags: review?(mrbkap) → review+
Assignee

Comment 61

4 years ago
Thanks Blake!
Assignee

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/81c1ca9e6344
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Attachment #8592666 - Flags: approval-mozilla-b2g37?
:henry, can you please fill the approval request comment here?
Flags: needinfo?(hchang)
(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 :)
(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/
Assignee

Comment 67

4 years ago
(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!
Flags: needinfo?(hchang)
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.
Attachment #8592666 - Flags: approval-mozilla-b2g37?
Assignee

Comment 69

4 years ago
Assignee

Comment 70

4 years ago
Attachment #8597095 - Attachment is obsolete: true
Assignee

Comment 71

4 years ago
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
Attachment #8597095 - Flags: approval-mozilla-b2g37?

Updated

4 years ago
Attachment #8597095 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+

Updated

4 years ago
Duplicate of this bug: 1152568
Whiteboard: [grooming][p=5] [SUMO-b2g] → [CR 820235][grooming][p=5] [SUMO-b2g]
Whiteboard: [CR 820235][grooming][p=5] [SUMO-b2g] → [caf priority: p2][CR 820235][grooming][p=5] [SUMO-b2g]
Depends on: 1182770
You need to log in before you can comment on or make changes to this bug.