[Wifi]: onconnected() function is not called when receiving CTRL-EVENT-CONNECTED event

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: vchang, Assigned: vchang)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

If we receive CTRL-EVENT-CONNECTED when manager.state is equal to COMPLETE, the onconnected() function will not be called. The reason is that  notifyStateChange() returns false in this case. So runDhcp() function will not have a chance to be called. This is the case reported from one of our partner. To get wider support of our framework, we should make it works in this case, too.

Comment 1

6 years ago
This is a great report. Please thank the partner. mrbkap, can you take this?
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
(Assignee)

Comment 2

6 years ago
Hi Blake, I would like to take this.
Assignee: nobody → vchang
(Assignee)

Comment 3

6 years ago
Created attachment 677315 [details] [diff] [review]
Patch, call onconnected() function when receiving CONNECTED event.
Attachment #677315 - Flags: review?(mrbkap)
Comment on attachment 677315 [details] [diff] [review]
Patch, call onconnected() function when receiving CONNECTED event.

Won't this let the manager's state end up in the "CONNECTED" state, meaning that we'll be kicked off and confused by reauth events later?

It seems like we should be tracking whether or not we've started DHCP separately. Either that, or we should ignore the CONNECTED state entirely and rely only on the COMPLETED message to start DHCP and friends. What do you think?
Attachment #677315 - Flags: review?(mrbkap)
(Assignee)

Comment 5

6 years ago
(In reply to Blake Kaplan (:mrbkap) from comment #4)
> Comment on attachment 677315 [details] [diff] [review]
> Patch, call onconnected() function when receiving CONNECTED event.
> 
> Won't this let the manager's state end up in the "CONNECTED" state, meaning
> that we'll be kicked off and confused by reauth events later?

Yeah, you are right. 

> It seems like we should be tracking whether or not we've started DHCP
> separately. Either that, or we should ignore the CONNECTED state entirely
> and rely only on the COMPLETED message to start DHCP and friends. What do
> you think?

Wpa_supplicant sends us CONNECTED event when wpa_state is "COMPLETED" and new_connection flag is 1. The new_connection flag is set to 1 when wpa_state is in one of (WPA_DISCONNECTED, WPA_ASSOCIATING, WPA_ASSOCIATED) state. So we receive CONNECTED event for a new connection once, but may receive COMPLETED event several times for a CONNECTION(Not sure if we really have this case, maybe renew key make it happen ?). 
So the question becomes that should we run dhcp whenever we receive a COMPLETED event, or we just need to run dhcp once for a new connection ? Any comments ?
(In reply to Vincent Chang[:vchang] from comment #5)
> So the question becomes that should we run dhcp whenever we receive a
> COMPLETED event, or we just need to run dhcp once for a new connection ? Any
> comments ?

Well, we might receive multiple COMPLETED events due to reauth, but that's taken care of by notifyStateChange, right? So we should be able to say (in CTRL-EVENT-STATE-CHANGE):

if (fields.state === "COMPLETED" && notifyStateChange(fields))
  onconnected();

and then we can safely ignore CTRL-EVENT-CONNECTED.

Am I missing anything?
(Assignee)

Comment 7

6 years ago
Created attachment 679054 [details] [diff] [review]
Patch v1.1

This patch addresses the comment 6.
Attachment #677315 - Attachment is obsolete: true
Attachment #679054 - Flags: review?(mrbkap)

Updated

6 years ago
Attachment #679054 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Comment 9

6 years ago
https://hg.mozilla.org/mozilla-central/rev/7c695905792e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c50f70664ba
status-firefox18: --- → fixed
status-firefox19: --- → fixed
You need to log in before you can comment on or make changes to this bug.