Hooked mobile activation UI with the new libcros additions

Finished mobile activation UI. Notification logic need to be tested.

BUG=chromium-os:4739, chromium-os:6949
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62041

[zel, 2010-10-08 06:00:59]

This CL includes:
- final UI for activation
- JS logic for displaying different UI states
- activation error detection changes based on spreadsheet from Eric and Jason

[dhg, 2010-10-08 17:28:26]

http://codereview.chromium.org/3606005/diff/14001/7009
File chrome/browser/resources/mobile_setup.js (right):

http://codereview.chromium.org/3606005/diff/14001/7009#newcode106
chrome/browser/resources/mobile_setup.js:106:
$('system-status').classList.add('hidden');
Could we do all these hidings, etc with a attribute on a parent div?  
i.e. the css rule:
parent['payment'] > .child {
  display: none
}

[Eric Shienbrood, 2010-10-08 17:39:40]

Most of these are comment issues, so feel free to ignore those for now.

http://codereview.chromium.org/3606005/diff/14001/7003
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3606005/diff/14001/7003#newcode4077
chrome/app/generated_resources.grd:4077: Activating your connection
"Activating your data service" ??

http://codereview.chromium.org/3606005/diff/14001/7003#newcode4083
chrome/app/generated_resources.grd:4083: Your cellular connection is ready to
use
I'm not sure whether this text reflects the actual wording that will be used,
but I wonder whether using the word "connection" is confusing here. This is
about setting up the user's data plan, and maybe the messages should say that.

http://codereview.chromium.org/3606005/diff/14001/7005
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/3606005/diff/14001/7005#newcode134
chrome/browser/chromeos/cros/network_library.h:134: const std::string&
operator_code() const { return operator_code_; }
These are only set for GSM networks, so don't depend on them.

http://codereview.chromium.org/3606005/diff/14001/7006
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/3606005/diff/14001/7006#newcode57
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:57: // Error codes maching
codes defined in cellular config file from
truncated comment?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode143
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:143: // Converts device the
current CelularNetork into JS object.
Not sure how this comment should read. "Converts the current CellularNetwork
device into a JS object"?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode326
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:326: const
chromeos::CellularNetwork& network = *(cell_networks.begin());
Maybe leave a comment that we assume a single cellular network?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode366
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:366: // reporting might not
have catch up with us yet.
s/catch/caught/

http://codereview.chromium.org/3606005/diff/14001/7006#newcode378
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:378: // We don't
partial comment?

[stevenjb, 2010-10-08 17:57:16]

http://codereview.chromium.org/3606005/diff/14001/7006
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/3606005/diff/14001/7006#newcode58
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:58: // /
nit: incomplete comment?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode237
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:237:
l10n_util::GetStringUTF16(IDS_MOBILE_ACTIVATING_HEADER));
Duplicate activating_header

http://codereview.chromium.org/3606005/diff/14001/7006#newcode352
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:352: const
chromeos::CellularNetwork& network = cell_networks.at(0);
Add comment re: eventual support for multiple cellular networks, e.g.
// TODO: handle multiple cellular networks. 
Do we know the path for the network we care about at this point?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode366
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:366: // reporting might not
have catch up with us yet.
nit: s/catch/caught/

http://codereview.chromium.org/3606005/diff/14001/7006#newcode378
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:378: // We don't
nit: incomplete comment?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode386
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:386: bool gotError = false;
nit: got_error

http://codereview.chromium.org/3606005/diff/14001/7006#newcode467
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:467: kCellularConfigPath;
Fatal here, or DCHECK? Might bite devs?

[zel, 2010-10-08 18:51:20]

http://codereview.chromium.org/3606005/diff/14001/7003
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3606005/diff/14001/7003#newcode4077
chrome/app/generated_resources.grd:4077: Activating your connection
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> "Activating your data service" ??

Done.

http://codereview.chromium.org/3606005/diff/14001/7003#newcode4083
chrome/app/generated_resources.grd:4083: Your cellular connection is ready to
use
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> I'm not sure whether this text reflects the actual wording that will be used,
> but I wonder whether using the word "connection" is confusing here. This is
> about setting up the user's data plan, and maybe the messages should say that.

changed to "Your data service is activated and ready to use" to make it
consistent with other messages

http://codereview.chromium.org/3606005/diff/14001/7005
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/3606005/diff/14001/7005#newcode134
chrome/browser/chromeos/cros/network_library.h:134: const std::string&
operator_code() const { return operator_code_; }
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> These are only set for GSM networks, so don't depend on them.

I will keep them in the library, but won't display them unless we have GSM
connection.

http://codereview.chromium.org/3606005/diff/14001/7006
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/3606005/diff/14001/7006#newcode57
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:57: // Error codes maching
codes defined in cellular config file from
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> truncated comment?

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode58
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:58: // /
On 2010/10/08 17:57:16, Steven Bennetts wrote:
> nit: incomplete comment?

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode143
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:143: // Converts device the
current CelularNetork into JS object.
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> Not sure how this comment should read. "Converts the current CellularNetwork
> device into a JS object"?

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode237
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:237:
l10n_util::GetStringUTF16(IDS_MOBILE_ACTIVATING_HEADER));
On 2010/10/08 17:57:16, Steven Bennetts wrote:
> Duplicate activating_header
> 

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode326
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:326: const
chromeos::CellularNetwork& network = *(cell_networks.begin());
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> Maybe leave a comment that we assume a single cellular network?

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode352
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:352: const
chromeos::CellularNetwork& network = cell_networks.at(0);
On 2010/10/08 17:57:16, Steven Bennetts wrote:
> Add comment re: eventual support for multiple cellular networks, e.g.
> // TODO: handle multiple cellular networks. 
> Do we know the path for the network we care about at this point?

I am not sure about this one - Eric, do I need to prepare the code for having
more than one cellular network to deal with?

http://codereview.chromium.org/3606005/diff/14001/7006#newcode366
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:366: // reporting might not
have catch up with us yet.
On 2010/10/08 17:57:16, Steven Bennetts wrote:
> nit: s/catch/caught/

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode366
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:366: // reporting might not
have catch up with us yet.
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> s/catch/caught/

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode378
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:378: // We don't
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> partial comment?

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode378
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:378: // We don't
On 2010/10/08 17:39:40, Eric Shienbrood wrote:
> partial comment?

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode386
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:386: bool gotError = false;
On 2010/10/08 17:57:16, Steven Bennetts wrote:
> nit: got_error

Done.

http://codereview.chromium.org/3606005/diff/14001/7006#newcode467
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:467: kCellularConfigPath;
On 2010/10/08 17:57:16, Steven Bennetts wrote:
> Fatal here, or DCHECK? Might bite devs?

Yep, it will surely bite devs, so I don't want to make this fatal event for now.

http://codereview.chromium.org/3606005/diff/14001/7009
File chrome/browser/resources/mobile_setup.js (right):

http://codereview.chromium.org/3606005/diff/14001/7009#newcode106
chrome/browser/resources/mobile_setup.js:106:
$('system-status').classList.add('hidden');
On 2010/10/08 17:28:26, dhg wrote:
> Could we do all these hidings, etc with a attribute on a parent div?  
> i.e. the css rule:
> parent['payment'] > .child {
>   display: none
> }
> 

Done. Visual state is now handled by CSS selectors over document.body.state

[stevenjb, 2010-10-08 19:07:16]

LGTM

http://codereview.chromium.org/3606005/diff/14001/7006
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/3606005/diff/14001/7006#newcode467
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:467: kCellularConfigPath;
On 2010/10/08 18:51:20, zel wrote:
> On 2010/10/08 17:57:16, Steven Bennetts wrote:
> > Fatal here, or DCHECK? Might bite devs?
> 
> Yep, it will surely bite devs, so I don't want to make this fatal event for
now.

Again, my bad, misunderstood LOG(ERROR).

[Eric Shienbrood, 2010-10-08 19:44:06]

Just more questions about "connection" vs. "activation" in the messages.

http://codereview.chromium.org/3606005/diff/14001/7006
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/3606005/diff/14001/7006#newcode352
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:352: const
chromeos::CellularNetwork& network = cell_networks.at(0);
On 2010/10/08 18:51:20, zel wrote:
> On 2010/10/08 17:57:16, Steven Bennetts wrote:
> > Add comment re: eventual support for multiple cellular networks, e.g.
> > // TODO: handle multiple cellular networks. 
> > Do we know the path for the network we care about at this point?
> 
> I am not sure about this one - Eric, do I need to prepare the code for having
> more than one cellular network to deal with?

We won't be dealing with multiple networks at the same time in the foreseeable
future, so don't worry about it.

http://codereview.chromium.org/3606005/diff/29001/30001
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4068
chrome/app/generated_resources.grd:4068: Cellular connection management
Should this be "Cellular data service activation management" or something
similar? I'm not sure of the context in which it's being used.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4071
chrome/app/generated_resources.grd:4071: Connecting to <ph
name="PROVIDER_NAME">$1<ex>Acme Mobile</ex></ph>
Is this message used only when connecting to the carrier's portal? If so, then
it's OK to use "connecting" here rather than "activation".

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4074
chrome/app/generated_resources.grd:4074: Connection error:
Should this refer to activation rather than connection? I'm not sure of the
context in which it's used.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4076
chrome/app/generated_resources.grd:4076: <message
name="IDS_MOBILE_ACTIVATING_HEADER" desc="Message header when cellular
connection is being activated">
Should desc be "Message header when cellular data service is being activated"?

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4079
chrome/app/generated_resources.grd:4079: <message
name="IDS_MOBILE_COMPLETED_HEADER" desc="Message header when cellular connection
is completed">
Should desc be "Message header when data service activation is completed?"

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4080
chrome/app/generated_resources.grd:4080: Connection complete
Should this be "Activation complete"?

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4082
chrome/app/generated_resources.grd:4082: <message
name="IDS_MOBILE_COMPLETED_TEXT" desc="Message when cellular connection is
completed">
Should desc be "Message when data service activation is completed?"

[zel, 2010-10-08 20:37:52]

Kan looked over the remaining texts and blessed them.

http://codereview.chromium.org/3606005/diff/29001/30001
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4068
chrome/app/generated_resources.grd:4068: Cellular connection management
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Should this be "Cellular data service activation management" or something
> similar? I'm not sure of the context in which it's being used.

Done.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4071
chrome/app/generated_resources.grd:4071: Connecting to <ph
name="PROVIDER_NAME">$1<ex>Acme Mobile</ex></ph>
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Is this message used only when connecting to the carrier's portal? If so, then
> it's OK to use "connecting" here rather than "activation".

Kan is ok with the message as it is.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4074
chrome/app/generated_resources.grd:4074: Connection error:
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Should this refer to activation rather than connection? I'm not sure of the
> context in which it's used.

Kan is ok with the message as it is.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4076
chrome/app/generated_resources.grd:4076: <message
name="IDS_MOBILE_ACTIVATING_HEADER" desc="Message header when cellular
connection is being activated">
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Should desc be "Message header when cellular data service is being activated"?

Done.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4079
chrome/app/generated_resources.grd:4079: <message
name="IDS_MOBILE_COMPLETED_HEADER" desc="Message header when cellular connection
is completed">
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Should desc be "Message header when data service activation is completed?"

Done.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4080
chrome/app/generated_resources.grd:4080: Connection complete
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Should this be "Activation complete"?

Done.

http://codereview.chromium.org/3606005/diff/29001/30001#newcode4082
chrome/app/generated_resources.grd:4082: <message
name="IDS_MOBILE_COMPLETED_TEXT" desc="Message when cellular connection is
completed">
On 2010/10/08 19:44:06, Eric Shienbrood wrote:
> Should desc be "Message when data service activation is completed?"

Done.

[Eric Shienbrood, 2010-10-08 20:52:47]

LGTM with one minor typo.

http://codereview.chromium.org/3606005/diff/1007/41001
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3606005/diff/1007/41001#newcode4079
chrome/app/generated_resources.grd:4079: <message
name="IDS_MOBILE_COMPLETED_HEADER" desc="Message header when data servicen is
completed">
typo: s/servicen/service/