Replace network combobox with network dropdown button

Replace network combobox with network dropdown button

BUG=chromium-os:4023, chromium-os:4117, chromium-os:4117
TEST=Manual test first OOBE screen

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

[Nikita (slow), 2010-09-13 11:03:36]

Please add chocobo@ to the reviewer list too.

http://codereview.chromium.org/3166028/diff/10001/11002
File chrome/browser/chromeos/login/network_screen.cc (right):

http://codereview.chromium.org/3166028/diff/10001/11002#newcode60
chrome/browser/chromeos/login/network_screen.cc:60: UpdateStatus();
Might pass network_lib as a parameter to UpdateStatus().

http://codereview.chromium.org/3166028/diff/10001/11003
File chrome/browser/chromeos/login/network_screen.h (right):

http://codereview.chromium.org/3166028/diff/10001/11003#newcode68
chrome/browser/chromeos/login/network_screen.h:68: void
StopWaitingForConnection(const string16& network_id_);
Should be named network_id.

http://codereview.chromium.org/3166028/diff/10001/11003#newcode71
chrome/browser/chromeos/login/network_screen.h:71: void WaitForConnection(const
string16& network_id_);
Should be named network_id.

http://codereview.chromium.org/3166028/diff/10001/11004
File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):

http://codereview.chromium.org/3166028/diff/10001/11004#newcode203
chrome/browser/chromeos/login/network_screen_browsertest.cc:203: #if 0
Please file an issue to re-enable these tests.

[Dmitry Polukhin, 2010-09-13 12:17:39]

http://codereview.chromium.org/3166028/diff/10001/11002
File chrome/browser/chromeos/login/network_screen.cc (right):

http://codereview.chromium.org/3166028/diff/10001/11002#newcode60
chrome/browser/chromeos/login/network_screen.cc:60: UpdateStatus();
On 2010/09/13 11:03:36, Nikita Kostylev wrote:
> Might pass network_lib as a parameter to UpdateStatus().

Done.

http://codereview.chromium.org/3166028/diff/10001/11003
File chrome/browser/chromeos/login/network_screen.h (right):

http://codereview.chromium.org/3166028/diff/10001/11003#newcode68
chrome/browser/chromeos/login/network_screen.h:68: void
StopWaitingForConnection(const string16& network_id_);
On 2010/09/13 11:03:36, Nikita Kostylev wrote:
> Should be named network_id.

Now it's just name. It would be good to create user friendly network names that
can be parsed unambiguously, something like WiFi/SSID, 3G/CarrierID or Ethernet.

http://codereview.chromium.org/3166028/diff/10001/11004
File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):

http://codereview.chromium.org/3166028/diff/10001/11004#newcode203
chrome/browser/chromeos/login/network_screen_browsertest.cc:203: #if 0
On 2010/09/13 11:03:36, Nikita Kostylev wrote:
> Please file an issue to re-enable these tests.

Done.

[Dmitry Polukhin, 2010-09-13 14:48:04]

Charlie, could you please take a look to this CL?

I know you are working on network menu refactoring so you opinion is very
important. I hope this CL won't make serious collision with your changes. It's
first step to use the same code in both cases.

[Dmitry Polukhin, 2010-09-13 16:02:13]

Oshima, as far as I know you are making experiment with DOM based network menu.

[Paweł Hajdan Jr., 2010-09-13 17:57:37]

Drive-by with a test comment.

http://codereview.chromium.org/3166028/diff/18001/19004
File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):

http://codereview.chromium.org/3166028/diff/18001/19004#newcode203
chrome/browser/chromeos/login/network_screen_browsertest.cc:203: #if 0
Why?

[oshima, 2010-09-13 18:21:26]

On 2010/09/13 16:02:13, Dmitry Polukhin wrote:
> Oshima, as far as I know you are making experiment with DOM based network
menu.

Yes, I'm playing with it right now. Thank you for sending this to me as well,
but I'm probably not right person to review this, especially you already have
two right person on this.

[Charlie Lee, 2010-09-13 18:29:35]

lgtm

[Dmitry Polukhin, 2010-09-13 18:47:39]

On 2010/09/13 17:57:37, Paweł Hajdan Jr. wrote:
> Drive-by with a test comment.
> 
> http://codereview.chromium.org/3166028/diff/18001/19004
> File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):
> 
> http://codereview.chromium.org/3166028/diff/18001/19004#newcode203
> chrome/browser/chromeos/login/network_screen_browsertest.cc:203: #if 0
> Why?

Original tests are completely irrelevant after my changes and needs to be fully
rewritten. Since we are short in time I would prefer to commit main
functionality as soon as it works. As for tests I created separate issue to
rewrite tests http://code.google.com/p/chromium-os/issues/detail?id=6672

[Dmitry Polukhin, 2010-09-13 18:52:23]

On 2010/09/13 18:21:26, oshima wrote:
> On 2010/09/13 16:02:13, Dmitry Polukhin wrote:
> > Oshima, as far as I know you are making experiment with DOM based network
> menu.
> 
> Yes, I'm playing with it right now. Thank you for sending this to me as well,
> but I'm probably not right person to review this, especially you already have
> two right person on this.

It should be "CC" instead of "To" - it was my mistake. Main reason for adding
you to this CL is to let you know that I changed it. To take into account not
only status bar use case but also OOBE. We don't want to have two independent
implementations.

[oshima, 2010-09-13 19:43:39]

On Mon, Sep 13, 2010 at 11:52 AM, <dpolukhin@chromium.org> wrote:

> On 2010/09/13 18:21:26, oshima wrote:
>
>> On 2010/09/13 16:02:13, Dmitry Polukhin wrote:
>> > Oshima, as far as I know you are making experiment with DOM based
>> network
>> menu.
>>
>
>  Yes, I'm playing with it right now. Thank you for sending this to me as
>> well,
>> but I'm probably not right person to review this, especially you already
>> have
>> two right person on this.
>>
>
> It should be "CC" instead of "To" - it was my mistake. Main reason for
> adding
> you to this CL is to let you know that I changed it. To take into account
> not
> only status bar use case but also OOBE. We don't want to have two
> independent
> implementations.


 Thanks for clarification. and yes, even if we go with domui path,
we should try to share the code as much as possible.

- oshima

>
>
> http://codereview.chromium.org/3166028/show
>

[Nikita (slow), 2010-09-14 06:14:45]

LGTM