Closed Bug 864931 Opened 7 years ago Closed 6 years ago

Rewrite net worker in C++

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: justin.lebar+bug, Assigned: dimi)

References

Details

(Whiteboard: [MemShrink:P1][~2.5MB])

Attachments

(4 files, 12 obsolete files)

1.41 MB, text/plain
Details
1.39 MB, text/plain
Details
139.52 KB, patch
Details | Diff | Splinter Review
139.19 KB, patch
Details | Diff | Splinter Review
The net_worker JS worker thread uses a lot of memory in B2G.  I strongly suspect that if we rewrote it in C++, we'd be able to reduce the component's memory usage significantly.

Here's a dump of the worker thread's memory usage.  Most-to-all of unused-arenas will go away after bug 829482, but even after that this worker uses way too much memory.

> 2.31 MB (06.75%) -- worker(resource://gre/modules/net_worker.js, 0x44486800)
> ├──1.73 MB (05.06%) -- gc-heap
> │  ├──1.70 MB (04.97%) ── unused-arenas
> │  └──0.03 MB (00.09%) -- (2 tiny)
> │     ├──0.03 MB (00.09%) ── chunk-admin
> │     └──0.00 MB (00.00%) ── unused-chunks
> └──0.58 MB (01.69%) -- (3 tiny)
>    ├──0.26 MB (00.77%) -- compartment(web-worker)
>    │  ├──0.16 MB (00.47%) -- gc-heap
>    │  │  ├──0.07 MB (00.22%) ── unused-gc-things [2]
>    │  │  ├──0.04 MB (00.11%) -- objects
>    │  │  │  ├──0.02 MB (00.06%) ── function
>    │  │  │  └──0.02 MB (00.05%) ── non-function
>    │  │  ├──0.03 MB (00.10%) -- shapes
>    │  │  │  ├──0.02 MB (00.07%) ── tree
>    │  │  │  └──0.01 MB (00.03%) ── base
>    │  │  └──0.02 MB (00.05%) ── sundries [2]
>    │  ├──0.03 MB (00.09%) ── analysis-temporary
>    │  ├──0.02 MB (00.07%) ── other-sundries [2]
>    │  ├──0.02 MB (00.05%) ── script-data
>    │  ├──0.02 MB (00.05%) ── objects-extra/slots
>    │  └──0.01 MB (00.04%) ── shapes-extra/compartment-tables
>    ├──0.20 MB (00.60%) -- runtime
>    │  ├──0.13 MB (00.36%) ── gc-marker
>    │  ├──0.04 MB (00.10%) ── runtime-object
>    │  ├──0.02 MB (00.05%) ── script-sources
>    │  ├──0.02 MB (00.05%) ── atoms-table
>    │  ├──0.00 MB (00.01%) ── dtoa
>    │  ├──0.00 MB (00.01%) ── stack
>    │  ├──0.00 MB (00.01%) ── temporary
>    │  ├──0.00 MB (00.00%) ── contexts
>    │  ├──0.00 MB (00.00%) ── script-filenames
>    │  ├──0.00 MB (00.00%) ── ion-code
>    │  ├──0.00 MB (00.00%) ── jaeger-code
>    │  ├──0.00 MB (00.00%) ── math-cache
>    │  ├──0.00 MB (00.00%) ── regexp-code
>    │  └──0.00 MB (00.00%) ── unused-code
>    └──0.11 MB (00.33%) -- compartment(web-worker-atoms)
>       ├──0.10 MB (00.29%) -- gc-heap
>       │  ├──0.09 MB (00.27%) ── strings
>       │  └──0.01 MB (00.02%) ── sundries
>       ├──0.01 MB (00.04%) ── string-chars/non-huge
>       └──0.00 MB (00.01%) ── other-sundries
There are four relevant workers here:

 * RIL worker (bug 864927)
 * Net worker (this bug)
 * wifi worker x2 (bug 864932)
Whiteboard: [MemShrink] → [MemShrink:P1]
Assignee: nobody → dzbarsky
Attached patch WIP Part 1 (obsolete) — Splinter Review
Attached patch WIP Part 2 (obsolete) — Splinter Review
Assignee: dzbarsky → nobody
Hi David, do you mind if I take this bug ?
Flags: needinfo?(dzbarsky)
Vincent, please do!
Flags: needinfo?(dzbarsky)
Assignee: nobody → vchang
Assignee: vchang → hchang
Assignee: hchang → dlee
Dimi, are you actively working on that?
Whiteboard: [MemShrink:P1] → [MemShrink:P1][tarako]
Flags: needinfo?(dlee)
(In reply to Fabrice Desré [:fabrice] from comment #6)
> Dimi, are you actively working on that?

Yes, I am working on it.
Current status is that net worker is changed to c++ and can communicate with netd.
And I am working on the command parsing and command chain now.
Flags: needinfo?(dlee)
Hi Dimi, does networker still use a lot of memory after GC?
Flags: needinfo?(dlee)
(In reply to Joe Cheng [:jcheng] from comment #8)
> Hi Dimi, does networker still use a lot of memory after GC?

Hi Joy, The networker is not yet finished, so memory measurement may not precise.
I will update it once the implementation is done.
Flags: needinfo?(dlee)
Hi Dimi, do you mind providing updates on when you may have this done? 
and if you can please provide the memory saving when this is implemented, please update the bug. Thanks
Flags: needinfo?(dlee)
Hi Joy,
  I will provide the patch and update the memory saving result before 1/10, is that ok for you ?
Flags: needinfo?(dlee)
From the experience of the wifi worker, I expect a saving of 2 to 2.5MB
Attached patch WIP v1 (obsolete) — Splinter Review
Attachment #758217 - Attachment is obsolete: true
Attachment #758216 - Attachment is obsolete: true
Whiteboard: [MemShrink:P1][tarako] → [MemShrink:P1][tarako][~2MB]
Comment on attachment 8357629 [details] [diff] [review]
WIP v1

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

::: dom/network/src/NetUtils.h
@@ +64,5 @@
> +// Set up a dlsymed function ready to use.
> +#define USE_DLFUNC(name)                                                      \
> +  FUNC##name name = (FUNC##name) dlsym(GetSharedLibrary(), #name);            \
> +  if (!name) {                                                                \
> +    MOZ_ASSUME_UNREACHABLE("Symbol not found in shared library : " #name);         \

drive-by nit: line up slashes.

::: dom/webidl/NetworkOptions.webidl
@@ +2,5 @@
> +* License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +* You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/**
> +* This dictionnary holds the parameters sent to the network worker.

drive-by nit: 'dictionary'
Comment on attachment 8357629 [details] [diff] [review]
WIP v1

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

I didn't check the logic itself, but I'm super happy to see that coming up!

::: dom/system/gonk/NetworkService.js
@@ +49,5 @@
>   */
>  function NetworkService() {
>    if(DEBUG) debug("Starting net_worker.");
> +  let networkWorker = Cc["@mozilla.org/network/worker;1"];
> +  var self = this;

nit: s/var/let

::: dom/system/gonk/NetworkUtils.cpp
@@ +44,5 @@
> +static const char* DUMMY_COMMAND = "tether status";
> +
> +// Retry 20 times (2 seconds) for usb state transition.
> +static const uint32_t USB_FUNCTION_RETRY_TIMES = 20;
> +// Check "sys.usb.state" every 100ms. 

nit: trailing white space

@@ +133,5 @@
> +  if (!f) {
> +    delete aChain;
> +    return;
> +  }
> +  

nit: whitespace

@@ +148,5 @@
> +  }
> +  QueueData data = gCommandQueue[0];
> +  gCommandQueue.RemoveElementAt(0);
> +
> +  sprintf(gCurrentCommand, "%s", data.a->mData);

use snprintf(gCurrentCommand, MAX_COMMAND_SIZE -1, ...) instead, here and elsewhere too.

@@ +848,5 @@
> +/**
> + * Handle received data from netd.
> + */
> +void NetworkUtils::onNetdMessage(const NetdCommand& aCommand)
> +{  

nit: whitespace

@@ +859,5 @@
> +    nextNetdCommand();
> +    return;
> +  }
> +  uint32_t code = atoi(result);
> +  char* reason = NULL;

here and everywhere, s/NULL/nullptr

@@ +1102,5 @@
> +           GET_CHAR(mInternalIfname), GET_CHAR(mExternalIfname));
> +    RUN_CHAIN(aOptions, gWifiEnableChain, wifiTetheringFail)
> +  } else {
> +    DEBUG("Stopping Wifi Tethering on %s <-> %s",
> +           GET_CHAR(mInternalIfname), GET_CHAR(mExternalIfname));    

nit: whitespace

@@ +1152,5 @@
> +    result.mResult = true;
> +    postMessage(aOptions, result);
> +    retry = 0;
> +    return;
> +  } 

nit: whitespace

::: dom/system/gonk/NetworkUtils.h
@@ +71,5 @@
> +    mCurInternalIfname = aOther.mCurInternalIfname;
> +    mCurExternalIfname = aOther.mCurExternalIfname;
> +    mThreshold = aOther.mThreshold;
> +  }
> +  

nit: whitespace

@@ +210,5 @@
> +public:
> +  CommandChain(const NetworkParams& aParams,
> +               COMMAND aCmds[],
> +               uint32_t aLength,
> +               ERROR_CALLBACK aError) 

nit: whitespace

@@ +261,5 @@
> +  bool removeDefaultRoute(NetworkParams& aOptions);
> +  bool removeHostRoute(NetworkParams& aOptions);
> +  bool removeHostRoutes(NetworkParams& aOptions);
> +  bool removeNetworkRoute(NetworkParams& aOptions);
> +  bool getNetworkInterfaceStats(NetworkParams& aOptions);  

nit: whitespace

::: dom/system/gonk/NetworkWorker.cpp
@@ +23,5 @@
> +using namespace mozilla::ipc;
> +
> +namespace mozilla {
> +
> +// The singleton Wifi service, to be used on the main thread.

I see where you copy/pasted from... ;)

@@ +26,5 @@
> +
> +// The singleton Wifi service, to be used on the main thread.
> +StaticRefPtr<NetworkWorker> gNetworkWorker;
> +
> +// The singleton supplicant class, that can be used on any thread.

Here too!

@@ +222,5 @@
> +  mWorkerThread->Dispatch(runnable, nsIEventTarget::DISPATCH_NORMAL);
> +}
> +
> +// Callback function for worker thread to update result to main thread.
> +void 

nit: whitespace
Test on Nexus4 WITH net_worker.js

1.10 MB (100.0%) -- explicit
├──1.00 MB (91.54%) -- workers/workers()
│  ├──1.00 MB (91.54%) -- worker(resource://gre/modules/net_worker.js, 0xNNN)
│  │  ├──0.38 MB (34.44%) -- runtime
│  │  │  ├──0.10 MB (08.94%) ── script-sources [+]
│  │  │  ├──0.09 MB (08.00%) ── script-data [+]
│  │  │  ├──0.06 MB (05.70%) ── atoms-table [+]
│  │  │  ├──0.06 MB (05.70%) -- code
│  │  │  │  ├──0.04 MB (03.62%) ── other [+]
│  │  │  │  ├──0.01 MB (01.24%) ── baseline [+]
│  │  │  │  └──0.01 MB (00.84%) ── unused [+]
│  │  │  ├──0.04 MB (03.56%) ── runtime-object [+]
│  │  │  ├──0.02 MB (01.43%) ── gc-marker [+]
│  │  │  └──0.01 MB (01.11%) ++ (4 tiny)
│  │  ├──0.24 MB (22.11%) -- zone(0xb163c800)
│  │  │  ├──0.19 MB (17.14%) -- compartment(web-worker)
│  │  │  │  ├──0.09 MB (08.21%) -- objects
│  │  │  │  │  ├──0.05 MB (04.35%) -- malloc-heap
│  │  │  │  │  │  ├──0.03 MB (02.52%) ── ctypes-data [+]
│  │  │  │  │  │  └──0.02 MB (01.83%) ── slots [+]
│  │  │  │  │  └──0.04 MB (03.86%) -- gc-heap
│  │  │  │  │     ├──0.03 MB (02.31%) ── function [+]
│  │  │  │  │     └──0.02 MB (01.55%) ── ordinary [+]
│  │  │  │  ├──0.06 MB (05.78%) -- shapes
│  │  │  │  │  ├──0.04 MB (03.28%) -- gc-heap
│  │  │  │  │  │  ├──0.02 MB (01.94%) ── tree/global-parented
│  │  │  │  │  │  └──0.01 MB (01.34%) ── base [+]
│  │  │  │  │  └──0.03 MB (02.49%) ── malloc-heap/compartment-tables
│  │  │  │  ├──0.02 MB (02.13%) -- sundries
│  │  │  │  │  ├──0.01 MB (01.23%) ── gc-heap [+]
│  │  │  │  │  └──0.01 MB (00.90%) ── malloc-heap [+]
│  │  │  │  └──0.01 MB (01.02%) ── scripts/gc-heap
│  │  │  ├──0.05 MB (04.19%) ── unused-gc-things [+]
│  │  │  └──0.01 MB (00.79%) ++ sundries
│  │  ├──0.18 MB (16.01%) -- zone(0xb20c2800)
│  │  │  ├──0.13 MB (11.95%) -- compartment(web-worker)
│  │  │  │  ├──0.05 MB (04.50%) -- objects
│  │  │  │  │  ├──0.04 MB (03.29%) -- gc-heap
│  │  │  │  │  │  ├──0.02 MB (02.17%) ── function [+]
│  │  │  │  │  │  └──0.01 MB (01.12%) ── ordinary [+]
│  │  │  │  │  └──0.01 MB (01.21%) ── malloc-heap/slots
│  │  │  │  ├──0.04 MB (03.67%) -- shapes
│  │  │  │  │  ├──0.03 MB (02.78%) -- gc-heap
│  │  │  │  │  │  ├──0.02 MB (02.00%) ── tree/global-parented
│  │  │  │  │  │  └──0.01 MB (00.78%) ── dict [+]
│  │  │  │  │  └──0.01 MB (00.89%) ── malloc-heap/compartment-tables
│  │  │  │  ├──0.02 MB (01.94%) ── scripts/gc-heap
│  │  │  │  └──0.02 MB (01.83%) -- sundries
│  │  │  │     ├──0.01 MB (01.08%) ── malloc-heap [+]
│  │  │  │     └──0.01 MB (00.75%) ── gc-heap [+]
│  │  │  ├──0.04 MB (03.87%) ── unused-gc-things [+]
│  │  │  └──0.00 MB (00.19%) ── sundries/gc-heap
│  │  ├──0.17 MB (15.78%) -- zone(0xb20c5c00)
│  │  │  ├──0.15 MB (13.95%) -- strings
│  │  │  │  ├──0.12 MB (11.12%) -- normal
│  │  │  │  │  ├──0.09 MB (07.88%) ── gc-heap [+]
│  │  │  │  │  └──0.04 MB (03.24%) ── malloc-heap [+]
│  │  │  │  └──0.03 MB (02.84%) ── short-gc-heap [+]
│  │  │  └──0.02 MB (01.82%) ++ (4 tiny)
│  │  └──0.04 MB (03.21%) -- gc-heap
│  │     ├──0.03 MB (02.85%) ── chunk-admin [+]
│  │     └──0.00 MB (00.36%) ── unused-arenas [+]
│  └──0.00 MB (00.00%) ++ (2 tiny)
├──0.21 MB (19.19%) -- heap-overhead
│  ├──0.15 MB (13.85%) ── waste
│  └──0.06 MB (05.35%) ── page-cache
└──-0.12 MB (-10.73%) ++ (9 tiny)
(In reply to Dimi Lee[:dimi][:dlee] from comment #16)
> Test on Nexus4 WITH net_worker.js
> 
> 1.10 MB (100.0%) -- explicit
> ├──1.00 MB (91.54%) -- workers/workers()
> │  ├──1.00 MB (91.54%) -- worker(resource://gre/modules/net_worker.js, 0xNNN)
> │  │  ├──0.38 MB (34.44%) -- runtime
> │  │  │  ├──0.10 MB (08.94%) ── script-sources [+]
> │  │  │  ├──0.09 MB (08.00%) ── script-data [+]
> │  │  │  ├──0.06 MB (05.70%) ── atoms-table [+]
> │  │  │  ├──0.06 MB (05.70%) -- code
> │  │  │  │  ├──0.04 MB (03.62%) ── other [+]
> │  │  │  │  ├──0.01 MB (01.24%) ── baseline [+]
> │  │  │  │  └──0.01 MB (00.84%) ── unused [+]
> │  │  │  ├──0.04 MB (03.56%) ── runtime-object [+]
> │  │  │  ├──0.02 MB (01.43%) ── gc-marker [+]
> │  │  │  └──0.01 MB (01.11%) ++ (4 tiny)
> │  │  ├──0.24 MB (22.11%) -- zone(0xb163c800)
> │  │  │  ├──0.19 MB (17.14%) -- compartment(web-worker)
> │  │  │  │  ├──0.09 MB (08.21%) -- objects
> │  │  │  │  │  ├──0.05 MB (04.35%) -- malloc-heap
> │  │  │  │  │  │  ├──0.03 MB (02.52%) ── ctypes-data [+]
> │  │  │  │  │  │  └──0.02 MB (01.83%) ── slots [+]
> │  │  │  │  │  └──0.04 MB (03.86%) -- gc-heap
> │  │  │  │  │     ├──0.03 MB (02.31%) ── function [+]
> │  │  │  │  │     └──0.02 MB (01.55%) ── ordinary [+]
> │  │  │  │  ├──0.06 MB (05.78%) -- shapes
> │  │  │  │  │  ├──0.04 MB (03.28%) -- gc-heap
> │  │  │  │  │  │  ├──0.02 MB (01.94%) ── tree/global-parented
> │  │  │  │  │  │  └──0.01 MB (01.34%) ── base [+]
> │  │  │  │  │  └──0.03 MB (02.49%) ── malloc-heap/compartment-tables
> │  │  │  │  ├──0.02 MB (02.13%) -- sundries
> │  │  │  │  │  ├──0.01 MB (01.23%) ── gc-heap [+]
> │  │  │  │  │  └──0.01 MB (00.90%) ── malloc-heap [+]
> │  │  │  │  └──0.01 MB (01.02%) ── scripts/gc-heap
> │  │  │  ├──0.05 MB (04.19%) ── unused-gc-things [+]
> │  │  │  └──0.01 MB (00.79%) ++ sundries
> │  │  ├──0.18 MB (16.01%) -- zone(0xb20c2800)
> │  │  │  ├──0.13 MB (11.95%) -- compartment(web-worker)
> │  │  │  │  ├──0.05 MB (04.50%) -- objects
> │  │  │  │  │  ├──0.04 MB (03.29%) -- gc-heap
> │  │  │  │  │  │  ├──0.02 MB (02.17%) ── function [+]
> │  │  │  │  │  │  └──0.01 MB (01.12%) ── ordinary [+]
> │  │  │  │  │  └──0.01 MB (01.21%) ── malloc-heap/slots
> │  │  │  │  ├──0.04 MB (03.67%) -- shapes
> │  │  │  │  │  ├──0.03 MB (02.78%) -- gc-heap
> │  │  │  │  │  │  ├──0.02 MB (02.00%) ── tree/global-parented
> │  │  │  │  │  │  └──0.01 MB (00.78%) ── dict [+]
> │  │  │  │  │  └──0.01 MB (00.89%) ── malloc-heap/compartment-tables
> │  │  │  │  ├──0.02 MB (01.94%) ── scripts/gc-heap
> │  │  │  │  └──0.02 MB (01.83%) -- sundries
> │  │  │  │     ├──0.01 MB (01.08%) ── malloc-heap [+]
> │  │  │  │     └──0.01 MB (00.75%) ── gc-heap [+]
> │  │  │  ├──0.04 MB (03.87%) ── unused-gc-things [+]
> │  │  │  └──0.00 MB (00.19%) ── sundries/gc-heap
> │  │  ├──0.17 MB (15.78%) -- zone(0xb20c5c00)
> │  │  │  ├──0.15 MB (13.95%) -- strings
> │  │  │  │  ├──0.12 MB (11.12%) -- normal
> │  │  │  │  │  ├──0.09 MB (07.88%) ── gc-heap [+]
> │  │  │  │  │  └──0.04 MB (03.24%) ── malloc-heap [+]
> │  │  │  │  └──0.03 MB (02.84%) ── short-gc-heap [+]
> │  │  │  └──0.02 MB (01.82%) ++ (4 tiny)
> │  │  └──0.04 MB (03.21%) -- gc-heap
> │  │     ├──0.03 MB (02.85%) ── chunk-admin [+]
> │  │     └──0.00 MB (00.36%) ── unused-arenas [+]
> │  └──0.00 MB (00.00%) ++ (2 tiny)
> ├──0.21 MB (19.19%) -- heap-overhead
> │  ├──0.15 MB (13.85%) ── waste
> │  └──0.06 MB (05.35%) ── page-cache
> └──-0.12 MB (-10.73%) ++ (9 tiny)

Sorry wrong comment, this is diff result with/without net_worker.js 
The saving is about 1.1 MB
baseline memory report on Nexus4
Test with mozilla-central and gaia master
change network from js to c++ memory-reports
Test with mozilla central with WIP V1 patch and GAIA master
That's great Dimi! I've seen it take much more in some memory reports (I have one laying around where the net worker eats up 2.5MB).
(In reply to Fabrice Desré [:fabrice] from comment #20)
> That's great Dimi! I've seen it take much more in some memory reports (I
> have one laying around where the net worker eats up 2.5MB).

Hi Fabrice,
 Yes, i did measure memory report several times, sometimes net worker may consume about 2.5Mb
 In that case it saves more memory.
Great job, can this patch land on v1.3 ?
blocking-b2g: --- → 1.3?
Whiteboard: [MemShrink:P1][tarako][~2MB] → [MemShrink:P1][tarako][~2.5MB]
We need this to shrink memory. 1.3+
blocking-b2g: 1.3? → 1.3+
Attached patch WIP V2 (obsolete) — Splinter Review
Attachment #8358321 - Flags: review?(vchang)
Attachment #8358321 - Flags: review?(fabrice)
Attachment #8357629 - Attachment is obsolete: true
(In reply to Steven Yang [:styang] from comment #23)
> We need this to shrink memory. 1.3+

This is a feature request with regression risk likely. I'd like to see more reasoning why this has to be included in 1.3 specifically. This should really be only included in tarako, not 1.3.
blocking-b2g: 1.3+ → 1.3?
We'll take that on tarako, but not in 1.3 - too late & too risky for 1.3.
blocking-b2g: 1.3? → ---
Whiteboard: [MemShrink:P1][tarako][~2.5MB] → [MemShrink:P1][tarako][~2.5MB][POVB]
Dimi, is your patch up to date? I had to patch a bit to get it to build:

diff --git a/dom/webidl/DummyBinding.webidl b/dom/webidl/DummyBinding.webidl
--- a/dom/webidl/DummyBinding.webidl
+++ b/dom/webidl/DummyBinding.webidl
@@ -25,13 +25,15 @@ interface DummyInterface : EventTarget {
   void MmsAttachment(optional MmsAttachment arg);
   void AsyncScrollEventDetail(optional AsyncScrollEventDetail arg);
   void OpenWindowEventDetail(optional OpenWindowEventDetail arg);
   void DOMWindowResizeEventDetail(optional DOMWindowResizeEventDetail arg);
   void WifiOptions(optional WifiCommandOptions arg1,
                    optional WifiResultOptions arg2);
   void AppNotificationServiceOptions(optional AppNotificationServiceOptions arg);
   void AppInfo(optional AppInfo arg1);
+  void NetworkOptions(optional NetworkCommandOptions arg1,
+                      optional NetworkResultOptions arg2);
 };
 
 interface DummyInterfaceWorkers {
   BlobPropertyBag blobBag();
 };
Attached patch WIP networker to c++ v3 (obsolete) — Splinter Review
Attachment #8358321 - Attachment is obsolete: true
Attachment #8358321 - Flags: review?(vchang)
Attachment #8358321 - Flags: review?(fabrice)
Attachment #8359055 - Flags: review?(vchang)
Attachment #8359055 - Flags: review?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #27)
> Dimi, is your patch up to date? I had to patch a bit to get it to build:
> 
> diff --git a/dom/webidl/DummyBinding.webidl b/dom/webidl/DummyBinding.webidl
> --- a/dom/webidl/DummyBinding.webidl
> +++ b/dom/webidl/DummyBinding.webidl
> @@ -25,13 +25,15 @@ interface DummyInterface : EventTarget {
>    void MmsAttachment(optional MmsAttachment arg);
>    void AsyncScrollEventDetail(optional AsyncScrollEventDetail arg);
>    void OpenWindowEventDetail(optional OpenWindowEventDetail arg);
>    void DOMWindowResizeEventDetail(optional DOMWindowResizeEventDetail arg);
>    void WifiOptions(optional WifiCommandOptions arg1,
>                     optional WifiResultOptions arg2);
>    void AppNotificationServiceOptions(optional AppNotificationServiceOptions
> arg);
>    void AppInfo(optional AppInfo arg1);
> +  void NetworkOptions(optional NetworkCommandOptions arg1,
> +                      optional NetworkResultOptions arg2);
>  };
>  
>  interface DummyInterfaceWorkers {
>    BlobPropertyBag blobBag();
>  };

Sorry i missed it. Attach patch v3 to fix this and also fix an compile error on desktop build.
(In reply to Dimi Lee[:dimi][:dlee] from comment #28)
> Created attachment 8359055 [details] [diff] [review]
> WIP networker to c++ v3

There are some comments i forgot to remove in NetworkUtils::ExecuteCommand.
Will remove it in next patch.
Comment on attachment 8359055 [details] [diff] [review]
WIP networker to c++ v3

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

Looks pretty good to me, and works well on device.

::: dom/system/gonk/NetworkUtils.cpp
@@ +243,5 @@
> +    strcpy(result, array[0].get());
> +    for (uint32_t i = 1; i < array.Length(); i++) {
> +      strcat(result, sep);
> +      strcat(result, array[i].get());
> +    }

We have no guarantee to not overrun result in this function. That's a bit scary.

@@ +267,5 @@
> + */
> +#define GET_CHAR(prop) NS_ConvertUTF16toUTF8(aChain->getParams().prop).get()
> +#define GET_FIELD(prop) aChain->getParams().prop
> +
> +void wifiFirmwareReload(CommandChain* aChain, CALLBACK aCallback, NetworkResultOptions& aResult)

aResult is unused in this function (and on many others). Am I missing something?

@@ +596,5 @@
> +
> +#define RUN_CHAIN(param, cmds, err)  uint32_t size = sizeof(cmds) / sizeof(COMMAND);  \
> +                                     CommandChain* chain = new CommandChain(param, cmds, size, err);  \
> +                                     NetworkResultOptions result;  \
> +                                     next(chain, false, result);

nit: align the \ in the macro

@@ +1241,5 @@
> +}
> +
> +void NetworkUtils::dumpParams(NetworkParams& aOptions, const char* aType)
> +{
> +#ifdef USE_DEBUG

why not just #ifdef DEBUG ?

::: dom/system/gonk/NetworkUtils.h
@@ +18,5 @@
> +
> +typedef void (*POSTMESSAGE)(NetworkResultOptions& aResult);
> +typedef void (*CALLBACK)(CommandChain*, bool, NetworkResultOptions& aResult);
> +typedef void (*ERROR_CALLBACK)(NetworkParams& aOptions, NetworkResultOptions& aResult);
> +typedef void (*COMMAND)(CommandChain*, CALLBACK, NetworkResultOptions& aResult);

nit: I'm not a big fan of ALLCAPS here. That make them look like constants.

::: dom/webidl/NetworkOptions.webidl
@@ +43,5 @@
> +  DOMString dns2;                     // for "setWifiTethering".
> +  float rxBytes;                      // for "getNetworkInterfaceStats".
> +  float txBytes;                      // for "getNetworkInterfaceStats".
> +  DOMString date;                     // for "getNetworkInterfaceStats".
> +  long threshold;                     // for "setNetworkInterfaceAlarm", 

nit: trailing whitespace
Attachment #8359055 - Flags: review?(fabrice) → feedback+
(In reply to Fabrice Desré [:fabrice] from comment #31)
> Comment on attachment 8359055 [details] [diff] [review]
> WIP networker to c++ v3
> 
> Review of attachment 8359055 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks pretty good to me, and works well on device.
> 
> ::: dom/system/gonk/NetworkUtils.cpp
> @@ +243,5 @@
> > +    strcpy(result, array[0].get());
> > +    for (uint32_t i = 1; i < array.Length(); i++) {
> > +      strcat(result, sep);
> > +      strcat(result, array[i].get());
> > +    }
> 
> We have no guarantee to not overrun result in this function. That's a bit
> scary.
> 
> @@ +267,5 @@
> > + */
> > +#define GET_CHAR(prop) NS_ConvertUTF16toUTF8(aChain->getParams().prop).get()
> > +#define GET_FIELD(prop) aChain->getParams().prop
> > +
> > +void wifiFirmwareReload(CommandChain* aChain, CALLBACK aCallback, NetworkResultOptions& aResult)
> 
> aResult is unused in this function (and on many others). Am I missing
> something?
> 

Take wifiFirmwareReload for example, it is store in the command chain gWifiEnableChain. And this chain also
contain function wifiTetheringSuccess.
The wifiTetheringSuccess will use aResult to post message to main thread.

And some functions like getTxBytes will also use return result from netd.
Attached patch WIP networker to c++ v4 (obsolete) — Splinter Review
Attachment #8359055 - Attachment is obsolete: true
Attachment #8359055 - Flags: review?(vchang)
Attachment #8359583 - Flags: review?(vchang)
Attachment #8359583 - Flags: review?(fabrice)
Comment on attachment 8359583 [details] [diff] [review]
WIP networker to c++ v4

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

Thanks for your help on this bug. The overall patches looks good. Please see my commands below.

::: dom/system/gonk/NetworkUtils.cpp
@@ +98,5 @@
> +inline bool isProceeding(uint32_t code)
> +{
> +  uint32_t type = netdResponseType(code);
> +  return type == NETD_COMMAND_PROCEEDING;
> +}

Why these functions are declared as global scope ? Please declare they as private member functions if they are used in NetworkUtils class only.

@@ +117,5 @@
> +static CommandChain* gCurrentChain = nullptr;
> +static bool gPending = false;
> +static nsTArray<nsCString> gReason;
> +
> +static void next(CommandChain* aChain, bool aError, NetworkResultOptions& aResult)

Please add const if aResult is not modified in this function.

@@ +135,5 @@
> +    return;
> +  }
> +
> +  (*f)(aChain, next, aResult);
> +}

How do you think if we move this to a member function of CommandChain class ?

@@ +146,5 @@
> +  if (gCommandQueue.IsEmpty() || gPending) {
> +    return;
> +  }
> +  QueueData data = gCommandQueue[0];
> +  gCommandQueue.RemoveElementAt(0);

You remove the queue here and add the queue in doCommand() function. 
Please make sure these two functions are called in the same thread, or you should  
protect gCommandQueue. Add MOZ_ASSERT(is net worker thread) should be a good idea.

@@ +153,5 @@
> +  gCurrentCallback = data.b;
> +  gCurrentChain = data.c;
> +
> +  gPending = true;
> +

The command chain ownership is passed to gCurrentxxx so that you can pop out the command queue here. gCurrentxxx are global variables and protected by gPending flag. It would be nicer to minimize the use of global variables if we move these global to gCommandQueue.

@@ +173,5 @@
> +  }
> +  netdCommand->mSize = strlen((char*)netdCommand->mData) + 1;
> +
> +  gCommandQueue.AppendElement(QueueData(netdCommand, aCallback, aChain));
> +

Ditto, please make sure this function is called in the same thread as nextNedCommand(). 
Add MOZ_ASSERT(is net worker thread) should be a good idea.

@@ +179,5 @@
> +}
> +
> +static void postMessage(NetworkResultOptions& aResult)
> +{
> +  (*(gNetworkUtils->mMessageCallback))(aResult);

This is static function and can be misuse even if gNetworkUtils is not assigned a initial value. 
add MOZ_ASSERT(gNetworkUtils, "xxxx") and MOZ_ASSERT(gNetworkUtils->mPostCallback, "xxxx").

@@ +185,5 @@
> +
> +static void postMessage(NetworkParams& aOptions, NetworkResultOptions& aResult)
> +{
> +  aResult.mId = aOptions.mId;
> +  (*(gNetworkUtils->mMessageCallback))(aResult);

Ditto: Please add the asserts
MOZ_ASSERT(gNetworkUtils)
MOZ_ASSERT(gNetworkUtils->mPostCallback)

@@ +189,5 @@
> +  (*(gNetworkUtils->mMessageCallback))(aResult);
> +}
> +
> +static void sendBroadcastMessage(uint32_t code, char* reason)
> +{

Why do you declare this function static scope ? It seems only used in NetworkUtils class.

@@ +211,5 @@
> +/**
> + * Helper function to get the bit length from given mask.
> + */
> +static uint32_t getMaskLength(const uint32_t mask)
> +{

Ditto, can this be a member function of NetworkUtils class ?

@@ +236,5 @@
> +
> +/**
> + * Helper function that implement join function.
> + */
> +static void join(nsTArray<nsCString>& array, const char* sep, const uint32_t maxlen, char* result)

Ditto, can this be a member function of NetworkUtils class ?

@@ +281,5 @@
> + */
> +#define GET_CHAR(prop) NS_ConvertUTF16toUTF8(aChain->getParams().prop).get()
> +#define GET_FIELD(prop) aChain->getParams().prop
> +
> +void wifiFirmwareReload(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)

Can you add the comments for this function ? Also these netd command functions are used in CommandChain, can we move them to CommandChain Class ?

@@ +290,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void startAccessPointDriver(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +306,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void stopAccessPointDriver(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +361,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void cleanUpStream(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +369,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void createUpStream(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +377,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void startSoftAP(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +383,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void stopSoftAP(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +389,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void getRxBytes(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +397,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void getTxBytes(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +408,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void enableAlarm(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +414,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void disableAlarm(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +420,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void setQuota(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +428,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void removeQuota(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +436,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void setAlarm(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +444,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void setInterfaceUp(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +463,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void tetherInterface(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +471,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void preTetherInterfaceList(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +483,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void postTetherInterfaceList(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +497,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void setIpForwardingEnabled(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +516,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void tetheringStatus(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +522,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void stopTethering(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +537,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void startTethering(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +559,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void untetherInterface(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +567,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void setDnsForwarders(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +575,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void enableNat(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +583,5 @@
> +  doCommand(command, aChain, aCallback);
> +}
> +
> +void disableNat(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +596,5 @@
> +
> +static CommandFunc gWifiFailChain[] = {stopSoftAP,
> +                                   setIpForwardingEnabled,
> +                                   stopTethering};
> +

Can you modify the coding style here and below ? 

static CommandFunc gWifiFailChain[] = {
  stopSoftAP,
  .....
}

@@ +600,5 @@
> +
> +static CommandFunc gUSBFailChain[] = {stopSoftAP,
> +                                  setIpForwardingEnabled,
> +                                  stopTethering};
> +

Ditto.

@@ +610,5 @@
> +
> +#define RUN_CHAIN(param, cmds, err)  uint32_t size = sizeof(cmds) / sizeof(CommandFunc);              \
> +                                     CommandChain* chain = new CommandChain(param, cmds, size, err);  \
> +                                     NetworkResultOptions result;                                     \
> +                                     next(chain, false, result);

Move to new line. 

#define RUN_CHAIN(param, cmds, err)                \
  uint32_t size = sizeof(cmds)/ sizeof(COMMAND);  \
  ....

If you move next to CommandChain, this will become chain->next(...)

@@ +612,5 @@
> +                                     CommandChain* chain = new CommandChain(param, cmds, size, err);  \
> +                                     NetworkResultOptions result;                                     \
> +                                     next(chain, false, result);
> +
> +void wifiTetheringFail(NetworkParams& aOptions, NetworkResultOptions& aResult)

Add the const to reference to object argument declaration here and below if they are not modified.  
void wifiTetheringFail(const NetworkParams& aOptions, const NetworkResultOptions& aResult)

@@ +624,5 @@
> +  RUN_CHAIN(aOptions, gWifiFailChain, nullptr)
> +}
> +
> +void wifiTetheringSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +630,5 @@
> +  postMessage(aChain->getParams(), aResult);
> +}
> +
> +void usbTetheringFail(NetworkParams& aOptions, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +651,5 @@
> +  }
> +}
> +
> +void usbTetheringSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +657,5 @@
> +  postMessage(aChain->getParams(), aResult);
> +}
> +
> +void networkInterfaceStatsFail(NetworkParams& aOptions, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +662,5 @@
> +  postMessage(aOptions, aResult);
> +}
> +
> +void networkInterfaceStatsSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +669,5 @@
> +  postMessage(aChain->getParams(), aResult);
> +}
> +
> +void networkInterfaceAlarmFail(NetworkParams& aOptions, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +674,5 @@
> +  postMessage(aOptions, aResult);
> +}
> +
> +void networkInterfaceAlarmSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +680,5 @@
> +  // params.error = parseFloat(params.resultReason);
> +  postMessage(aChain->getParams(), aResult);
> +}
> +
> +void updateUpStreamFail(NetworkParams& aOptions, NetworkResultOptions& aResult)

Ditto.

@@ +686,5 @@
> +  postMessage(aOptions, aResult);
> +}
> +
> +void updateUpStreamSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +693,5 @@
> +  postMessage(aChain->getParams(), aResult);
> +}
> +
> +void setDhcpServerFail(NetworkParams& aOptions, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +699,5 @@
> +  postMessage(aOptions, aResult);
> +}
> +
> +void setDhcpServerSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +705,5 @@
> +  postMessage(aChain->getParams(), aResult);
> +}
> +
> +void wifiOperationModeFail(NetworkParams& aOptions, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +710,5 @@
> +  postMessage(aOptions, aResult);
> +}
> +
> +void wifiOperationModeSuccess(CommandChain* aChain, CommandCallback aCallback, NetworkResultOptions& aResult)
> +{

Ditto.

@@ +724,5 @@
> +                                        tetherInterface,
> +                                        tetheringStatus,
> +                                        startTethering,
> +                                        setDnsForwarders,
> +                                        usbTetheringSuccess};

Move to new line. 

static CommandFunc gUSBEnableChain[] = {
  setInterfaceUp,
  ...
};

@@ +733,5 @@
> +                                         disableNat,
> +                                         setIpForwardingEnabled,
> +                                         stopTethering,
> +                                         usbTetheringSuccess};
> +

Ditto.

@@ +746,5 @@
> +                                         startTethering,
> +                                         setDnsForwarders,
> +                                         enableNat,
> +                                         wifiTetheringSuccess};
> +

Ditto.

@@ +756,5 @@
> +                                          postTetherInterfaceList,
> +                                          disableNat,
> +                                          setIpForwardingEnabled,
> +                                          stopTethering,
> +                                          wifiTetheringSuccess};

Ditto.

@@ +760,5 @@
> +                                          wifiTetheringSuccess};
> +
> +static CommandFunc gStartDhcpServerChain[] = {setInterfaceUp,
> +                                              startTethering,
> +                                              setDhcpServerSuccess};

Ditto.

@@ +764,5 @@
> +                                              setDhcpServerSuccess};
> +
> +static CommandFunc gStopDhcpServerChain[] = {stopTethering,
> +                                             setDhcpServerSuccess};
> +

Ditto.

@@ +768,5 @@
> +
> +static CommandFunc gNetworkInterfaceStatsChain[] = {getRxBytes,
> +                                                    getTxBytes,
> +                                                    networkInterfaceStatsSuccess};
> +

Ditto.

@@ +773,5 @@
> +static CommandFunc gNetworkInterfaceEnableAlarmChain[] = {enableAlarm,
> +                                                          setQuota,
> +                                                          setAlarm,
> +                                                      networkInterfaceAlarmSuccess};
> +

Ditto.

@@ +776,5 @@
> +                                                      networkInterfaceAlarmSuccess};
> +
> +static CommandFunc gNetworkInterfaceDisableAlarmChain[] = {removeQuota,
> +                                                           disableAlarm,
> +                                                           networkInterfaceAlarmSuccess};

Ditto.

@@ +780,5 @@
> +                                                           networkInterfaceAlarmSuccess};
> +
> +static CommandFunc gNetworkInterfaceSetAlarmChain[] = {setAlarm,
> +                                                       networkInterfaceAlarmSuccess};
> +

Ditto.

@@ +782,5 @@
> +static CommandFunc gNetworkInterfaceSetAlarmChain[] = {setAlarm,
> +                                                       networkInterfaceAlarmSuccess};
> +
> +static CommandFunc gWifiOperationModeChain[] = {wifiFirmwareReload,
> +                                                wifiOperationModeSuccess};

Ditto.

@@ +787,5 @@
> +
> +static CommandFunc gUpdateUpStreamChain[] = {cleanUpStream,
> +                                             createUpStream,
> +                                             updateUpStreamSuccess};
> +

Ditto.
Attachment #8359583 - Flags: review?(vchang)
Attached patch WIP networker to c++ v5 (obsolete) — Splinter Review
Attachment #8359583 - Attachment is obsolete: true
Attachment #8359583 - Flags: review?(fabrice)
Attachment #8360239 - Flags: review?(vchang)
Attachment #8360239 - Flags: review?(fabrice)
Comment on attachment 8360239 [details] [diff] [review]
WIP networker to c++ v5

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

This looks pretty good overall. Just a couple of nits. I will try to do some test manually tomorrow. If everything goes well, I'll r+ the patch.

::: dom/system/gonk/NetworkUtils.cpp
@@ +19,5 @@
> +#include <cutils/properties.h>
> +#include "mozilla/dom/network/NetUtils.h"
> +
> +#define _DEBUG 1
> +

Nit: Please remember to set _DEBUG to zero when you commit the patches.

@@ +323,5 @@
> +  snprintf(gCurrentCommand.command, MAX_COMMAND_SIZE - 1, "%s", GET_CURRENT_COMMAND);
> +
> +  DEBUG("Sending \'%s\' command to netd.", gCurrentCommand.command);
> +  SendNetdCommand(GET_CURRENT_NETD_COMMAND);
> + 

Nit: extra space.
Blocks: 928289
Comment on attachment 8360239 [details] [diff] [review]
WIP networker to c++ v5

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

I tested locally, and could get both wifi and data connections working.
Attachment #8360239 - Flags: review?(fabrice) → review+
Comment on attachment 8360239 [details] [diff] [review]
WIP networker to c++ v5

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

I have tested wifi/wifi hotspot/usb tethering/3G data connection/cost control, and these functions work pretty well. r=me with small nits addressed.
Attachment #8360239 - Flags: review?(vchang) → review+
Attached patch networker to c++ v6 (obsolete) — Splinter Review
Attachment #8360239 - Attachment is obsolete: true
Hi Dimi, how about the try server result here ? If everything is fine, I would like this patch be landed soon.
Flags: needinfo?(dlee)
(In reply to Vincent Chang[:vchang] from comment #40)
> Hi Dimi, how about the try server result here ? If everything is fine, I
> would like this patch be landed soon.

Hi Vincent,
  It seems there are some error on Mochitest 
  https://tbpl.mozilla.org/?tree=Try&rev=8b800382f044

  But because i am at NFC work week, i might not be able to check it soon :(
Flags: needinfo?(dlee)
Vincent, would you like to help here? We need this to land soon.
(In reply to James Ho from comment #42)
> Vincent, would you like to help here? We need this to land soon.

I am helping here and trying to fix the try server error right now.
Fixed xpcshell errors with local test using below command. 
./test.sh xpcshell --test-path dom/network/tests/unit_stats/

push to try server again....

https://tbpl.mozilla.org/?tree=Try&rev=d63fed079bd9
Attached patch networker to c++ v7 (obsolete) — Splinter Review
Hi Fabrice,
This patch fixs xpcshell test failure and all the fix are in NetworkService.js.
The problem is that asynchronous call is being called when xpcom is shutdown,
so the callback may cause crash when returned.
Attachment #8361508 - Attachment is obsolete: true
Attachment #8365854 - Flags: review?(fabrice)
Attachment #8365854 - Attachment is obsolete: true
Attachment #8365854 - Flags: review?(fabrice)
Attached patch networker to c++ v7 (obsolete) — Splinter Review
Comment on attachment 8365862 [details] [diff] [review]
networker to c++ v7

Re-attach since some style error, please see Comment 46.
Attachment #8365862 - Flags: review?(fabrice)
Attachment #8365862 - Flags: review?(fabrice) → review+
Keywords: checkin-needed
Comment on attachment 8365862 [details] [diff] [review]
networker to c++ v7

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

::: dom/system/gonk/NetworkWorker.cpp
@@ +148,5 @@
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +  MOZ_ASSERT(aListener);
> +
> +  WARN("[Dimi]NetworkWorker - Start");

I guess we don't need this.

Same for below.
Attached patch network to c++ v8 (obsolete) — Splinter Review
Attachment #8367100 - Attachment is obsolete: true
Attached patch networker to c++ v8 (obsolete) — Splinter Review
Attachment #8365862 - Attachment is obsolete: true
Keywords: checkin-needed
I got conflict when pushing to inbound
please rebase again.
Keywords: checkin-needed
Hi Yoshi,
  I try rebase on in-bound and there is no conflict, attach a new patch merged with in-bound.
  Could you help check if it is ok ? Thanks!
Attachment #8367102 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/496458acf82a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
use 1.3T+ flag instead of [tarako][POVB] in whiteboard
blocking-b2g: --- → 1.3T+
Whiteboard: [MemShrink:P1][tarako][~2.5MB][POVB] → [MemShrink:P1][~2.5MB]
Whiteboard: [MemShrink:P1][~2.5MB] → [tarako][MemShrink:P1][~2.5MB]
Dimi, can you rebase this patch on 1.3t (and the couple of followups we had too)? thanks!
Flags: needinfo?(dlee)
using status-b2g-v1.3T, remove [tarako]
Whiteboard: [tarako][MemShrink:P1][~2.5MB] → [MemShrink:P1][~2.5MB]
(In reply to Fabrice Desré [:fabrice] from comment #58)
> Dimi, can you rebase this patch on 1.3t (and the couple of followups we had
> too)? thanks!

I am working on it :)
Flags: needinfo?(dlee)
Depends on: 966175
Blocks: 977474
Depends on: 989135
Depends on: 1031647
Blocks: 1041397
Depends on: 1075800
Depends on: 1117592
Depends on: 1168938
You need to log in before you can comment on or make changes to this bug.