Added flags to prevent messing with network connections while 3G device is be...

Repackeged mobile marketing content as component extension.

BUG=chromium-os:11454
TEST=make sure marketing message shows up on chrome:mobilesetup page again

[Charlie Lee, 2011-01-20 20:05:43]

A couple of main issues:

1) If we are only using lock/unlock for mobile activation, I suggest we rename
lock/unlock to be specific... something like SetActivatingMobile(bool) and
IsActivatingMobile(). That way we won't be tempted to use these methods for
other situations where we want to lock down the network library. Also, we should
move the DisableOtherNetworks/ReenableOtherNetworks methods into the network
library. And maybe you don't need the check in the Enable*Network methods.

2) Could we get into the state where the network is locked down but the user
cannot cancel the mobile activation and the activation hangs for some reason? Is
there any case where the mobile activation tab is waiting for user action and
the network is locked down?

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:957: is_locked_ = false;
What's the reason for setting this twice?

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:454: if (lib->IsLocked())
probably want to remove network manager observer before calling unlock since
unlock will trigger a network manager changed event.

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:468: if
(!chromeos::CrosLibrary::Get()->GetNetworkLibrary()->IsLocked())
if we are using locked only for network activation. maybe we should rename it to
that instead. that way we won't use the lock/unlock for something else that
wants to lock down the network library.

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:1023: lib->Unlock();
same here. move this below remove network manager observer?

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/network_menu_button.cc (right):

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu_button.cc:121: if (!cros ||
!CrosLibrary::Get()->EnsureLoaded() || cros->IsLocked()) {
we probably want a different tool tip when it's locked.

[zel, 2011-01-20 22:06:14]

1) See my other comments on this subject
2) Yes, the activation can still get stuck for whatever reasons (i.e. camping on
payment page), but as long as the user closes the tab, we will unlock network
access again. While we are stuck in such states, we still can not let other
network activity to be present since it might affect the outcome of our
processing (i.e. restricted IP pool detection).

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/cros...
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/cros...
chrome/browser/chromeos/cros/network_library.cc:957: is_locked_ = false;
On 2011/01/20 20:05:43, Charlie Lee wrote:
> What's the reason for setting this twice?

Done.

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
File chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc (right):

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:454: if (lib->IsLocked())
On 2011/01/20 20:05:43, Charlie Lee wrote:
> probably want to remove network manager observer before calling unlock since
> unlock will trigger a network manager changed event.

Done.

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:468: if
(!chromeos::CrosLibrary::Get()->GetNetworkLibrary()->IsLocked())
On 2011/01/20 20:05:43, Charlie Lee wrote:
> if we are using locked only for network activation. maybe we should rename it
to
> that instead. that way we won't use the lock/unlock for something else that
> wants to lock down the network library.

Actually, I wanted to keep this generic at least from API side. Ideally, we
should be able to solve activation problems with flimflam communication channels
instead of locking, but there are other cases where we might want to freeze
operations that enable/disable particular types of connections.

http://codereview.chromium.org/6292008/diff/8002/chrome/browser/chromeos/dom_...
chrome/browser/chromeos/dom_ui/mobile_setup_ui.cc:1023: lib->Unlock();
On 2011/01/20 20:05:43, Charlie Lee wrote:
> same here. move this below remove network manager observer?

Done.

[Charlie Lee, 2011-01-21 00:24:37]

LGTM but one UI concern. When the network is locked and the mobile setup tab is
stuck, the user may not realize that they can close the tab to unlock the
network. So some users will be stuck in this state and become frustrated. I'm
not sure what a good solution is though.

http://codereview.chromium.org/6292008/diff/21002/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/network_menu_button.cc (right):

http://codereview.chromium.org/6292008/diff/21002/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/network_menu_button.cc:125:
IDS_STATUSBAR_NETWORK_NO_NETWORK_TOOLTIP)));
fix tooltip to be more clear?

[zel, 2011-01-21 19:14:49]

http://codereview.chromium.org/6292008/diff/21002/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/network_menu_button.cc (right):

http://codereview.chromium.org/6292008/diff/21002/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/network_menu_button.cc:125:
IDS_STATUSBAR_NETWORK_NO_NETWORK_TOOLTIP)));
On 2011/01/21 00:24:37, Charlie Lee wrote:
> fix tooltip to be more clear?

On the second thought, I will better leave the icon and tooltip as they are
right now and just reflect locked state in the menu. Done.

[Charlie Lee, 2011-01-21 19:25:15]

LGTM

On 2011/01/21 19:14:49, zel wrote:
>
http://codereview.chromium.org/6292008/diff/21002/chrome/browser/chromeos/sta...
> File chrome/browser/chromeos/status/network_menu_button.cc (right):
> 
>
http://codereview.chromium.org/6292008/diff/21002/chrome/browser/chromeos/sta...
> chrome/browser/chromeos/status/network_menu_button.cc:125:
> IDS_STATUSBAR_NETWORK_NO_NETWORK_TOOLTIP)));
> On 2011/01/21 00:24:37, Charlie Lee wrote:
> > fix tooltip to be more clear?
> 
> On the second thought, I will better leave the icon and tooltip as they are
> right now and just reflect locked state in the menu. Done.