1050678 - [Wifi] Send network info along with state change event: disconnected and connectingfailed

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Some state change event like disconnected and connectingfailed doesn't contain network info, so app has to get network data from wifiManager.connectionInformation now.
But it doesn't contain network information, also current network info might be different from the network triggers the state change(especially disconnected and connectingfailed state).
We should revise and make sure we provided as much network info as possible with ever state change event.

Comment 1

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - Provide network info for state change event.

Comment 2

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - Provide network info for state change event.

Provide network information on each wifi state change, so Gaia can take these info to control UI more precisely.
WPS state is not included.

Comment 3

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8475030 [details] [diff] [review]
WIP - Provide network info for state change event.

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

::: dom/wifi/DOMWifiManager.js
@@ +364,4 @@
>          this._connectionStatus = "disconnected";
>          this._lastConnectionInfo = null;
>          this._fireStatusChangeEvent();
> +        this._currentNetwork = null;

Can we use msg.network directly?

Comment 4

[Henry Chang [:hchang]]

Comment on attachment 8475030 [details] [diff] [review]
WIP - Provide network info for state change event.

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

The idea generally looks good to me! Thanks!

::: dom/wifi/WifiWorker.js
@@ +2004,5 @@
>        if (netId) {
>          WifiManager.disableNetwork(netId, function() {});
>        }
>      });
> +    self._fireEvent("onconnectingfailed", {network: netToDOM(self.currentNetwork)});

Does it mean the original version is not working?

@@ +2050,5 @@
>          self.currentNetwork.netId = this.id;
> +        WifiManager.getNetworkConfiguration(self.currentNetwork, function (){
> +          // Notify again because we get complete network information.
> +          self._fireEvent("onconnecting", { network: netToDOM(self.currentNetwork) });
> +        }.bind(self));

No need to bind |self| since the callback function doesn't use |this|.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Henry Chang [:henry] from comment #4)
> ::: dom/wifi/WifiWorker.js
> @@ +2004,5 @@
> >        if (netId) {
> >          WifiManager.disableNetwork(netId, function() {});
> >        }
> >      });
> > +    self._fireEvent("onconnectingfailed", {network: netToDOM(self.currentNetwork)});
> 
> Does it mean the original version is not working?
> 

Yes, but Gaia doesn't use event.network now, so it doesn't affect them now.

Comment 6

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - Provide network info for state change event.

Address feedback comments.

Comment 7

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Provide network info for state change event.

Comment 8

[Henry Chang [:hchang]]

Comment on attachment 8487748 [details] [diff] [review]
Provide network info for state change event.

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

Good patch and no concern to me. Thanks!

Comment 9

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Provide network info for state change event. r=hchang

Add reviewer's name.

Comment 10

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try looks good, CPP Unit Test Fail is irrelevant because it has been failed several days and I only changes js file : https://tbpl.mozilla.org/?tree=Try&rev=826f124c7898

Comment 11

[Carsten Book [:Tomcat]]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c8b5a6377dad