Closed Bug 1116458 Opened 6 years ago Closed 6 years ago

Some functions of nsINetworkService cannot be called interleavingly.

Categories

(Firefox OS Graveyard :: Wifi, defect)

x86_64
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.2 S3 (9jan)

People

(Reporter: hchang, Assigned: hchang)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → hchang
Attached patch Bug1116458.patch (obsolete) — Splinter Review
Blocks: 1104664
Attachment #8543843 - Flags: review?(dlee)
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
Attachment #8543843 - Flags: review?(dlee) → review+
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
Attachment #8543843 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/67ea86c1f2e9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S3 (9jan)
You need to log in before you can comment on or make changes to this bug.