1116458 - Some functions of nsINetworkService cannot be called interleavingly.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Henry Chang [:hchang]]

nsINetworkService provides a set of functions about networking but some of them which require netd command chain cannot be called interleavingly. That means, one function has to be called after the previous one is called back. It is inconvenient for calling nsINetworkSerive functions in a row.

Comment 1

[Henry Chang [:hchang]]

Attachment: Bug1116458.patch

Comment 2

[Dimi Lee [ooo 16.04 - 21.04］]

Comment on attachment 8543843 [details] [diff] [review]
Bug1116458.patch

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

Looks good, thanks

::: dom/system/gonk/NetworkUtils.cpp
@@ +126,1 @@
>  

I think we need to clear this when NetworkUtils is destroyed

@@ +999,5 @@
>      MOZ_ASSERT(!gWifiTetheringParms);
>      gWifiTetheringParms = new NetworkParams(aChain->getParams());
>    }
>    postMessage(aChain->getParams(), aResult);
> +  finalizeSuccess(aChain, aResult);

Is it possible we move this into PostMessage and check the |aResult| value ?
Only call finalize when result is success. I am not sure if there is any chance
that result is success but we don't have to call finalize

Comment 3

[Henry Chang [:hchang]]

Thanks for your review!

(In reply to Dimi Lee[:dimi][:dlee] from comment #2)
> Comment on attachment 8543843 [details] [diff] [review]
> Bug1116458.patch
> 
> Review of attachment 8543843 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, thanks
> 
> ::: dom/system/gonk/NetworkUtils.cpp
> @@ +126,1 @@
> >  
> 
> I think we need to clear this when NetworkUtils is destroyed
> 
> @@ +999,5 @@
> >      MOZ_ASSERT(!gWifiTetheringParms);
> >      gWifiTetheringParms = new NetworkParams(aChain->getParams());
> >    }
> >    postMessage(aChain->getParams(), aResult);
> > +  finalizeSuccess(aChain, aResult);
> 
> Is it possible we move this into PostMessage and check the |aResult| value ?
> Only call finalize when result is success. I am not sure if there is any
> chance
> that result is success but we don't have to call finalize

Since there's no standard way to tell if a operation is successful
or not (some uses mRet and some uses mSuccess), it's difficult to knwo 
when we should call to finalize in PostMessage whereas xxxxSuccess 
explicitly suggests success.


By the way, I forgot to add finalizeSuccess to updateUpStreamSuccess

Comment 4

[Henry Chang [:hchang]]

Attachment: Bug1116458 (v2, r+'d)

Comment 5

[Henry Chang [:hchang]]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/67ea86c1f2e9

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/67ea86c1f2e9