Update icons to show lowdata and very lowdata for 3G data....

Update icons to show lowdata and very lowdata for 3G data.
BUG=chromium-os:5608
TEST=manual

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

[stevenjb, 2010-10-14 17:58:43]

http://codereview.chromium.org/3748005/diff/27001/28023
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28023#newcode481
chrome/browser/chromeos/cros/network_library.cc:481: }
See comment in header. I think the code will be easier to read (less
duplication) if these were merged into a single function.

http://codereview.chromium.org/3748005/diff/27001/28024
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/3748005/diff/27001/28024#newcode338
chrome/browser/chromeos/cros/network_library.h:338: virtual bool
cellular_nodata() const = 0;
Maintaining network_library functions is something of a pain; if any of these
ever get called during initialization,
CrosMock::SetNetworkLibraryStatusAreaExpectations needs to be changed with
expectations for each of them (I just ran into this).
Can we make this a single function that returns an enum, something like
cellular_data_plan_status()? 
This would also make it more clear that the function is associated with the data
plan(s), not the connected cellular network (as opposed to
cellular_connected()/connecting())

http://codereview.chromium.org/3748005/diff/27001/28025
File chrome/browser/chromeos/dom_ui/internet_options_handler.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28025#newcode409
chrome/browser/chromeos/dom_ui/internet_options_handler.cc:409: const
chromeos::CellularDataPlan& plan) {
This logic seems very similar to the logic now in network_library. Can we use
the same logic instead of just the same constants?

http://codereview.chromium.org/3748005/diff/27001/28026
File chrome/browser/chromeos/network_message_observer.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28026#newcode181
chrome/browser/chromeos/network_message_observer.cc:181: if (plan.plan_type ==
CELLULAR_DATA_PLAN_UNLIMITED) {
This also seems like logic that should be in network_library.

http://codereview.chromium.org/3748005/diff/27001/28028
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28028#newcode376
chrome/browser/chromeos/status/network_menu.cc:376: const int images[]) {
Since the image arrays are all declared here, can we make them local and pass an
enum to this function, rather than passing a global pointer to one of the
arrays?

[Charlie Lee, 2010-10-19 21:02:19]

http://codereview.chromium.org/3748005/diff/27001/28023
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28023#newcode481
chrome/browser/chromeos/cros/network_library.cc:481: }
On 2010/10/14 17:58:43, Steven Bennetts wrote:
> See comment in header. I think the code will be easier to read (less
> duplication) if these were merged into a single function.
> 

Done.

http://codereview.chromium.org/3748005/diff/27001/28024
File chrome/browser/chromeos/cros/network_library.h (right):

http://codereview.chromium.org/3748005/diff/27001/28024#newcode338
chrome/browser/chromeos/cros/network_library.h:338: virtual bool
cellular_nodata() const = 0;
Moved this to CellularNetwork.

On 2010/10/14 17:58:43, Steven Bennetts wrote:
> Maintaining network_library functions is something of a pain; if any of these
> ever get called during initialization,
> CrosMock::SetNetworkLibraryStatusAreaExpectations needs to be changed with
> expectations for each of them (I just ran into this).
> Can we make this a single function that returns an enum, something like
> cellular_data_plan_status()? 
> This would also make it more clear that the function is associated with the
data
> plan(s), not the connected cellular network (as opposed to
> cellular_connected()/connecting())

http://codereview.chromium.org/3748005/diff/27001/28025
File chrome/browser/chromeos/dom_ui/internet_options_handler.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28025#newcode409
chrome/browser/chromeos/dom_ui/internet_options_handler.cc:409: const
chromeos::CellularDataPlan& plan) {
Hard to move this logic right now. Will look into this later.

On 2010/10/14 17:58:43, Steven Bennetts wrote:
> This logic seems very similar to the logic now in network_library. Can we use
> the same logic instead of just the same constants?
>

http://codereview.chromium.org/3748005/diff/27001/28026
File chrome/browser/chromeos/network_message_observer.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28026#newcode181
chrome/browser/chromeos/network_message_observer.cc:181: if (plan.plan_type ==
CELLULAR_DATA_PLAN_UNLIMITED) {
On 2010/10/14 17:58:43, Steven Bennetts wrote:
> This also seems like logic that should be in network_library.
> 

Done.

http://codereview.chromium.org/3748005/diff/27001/28028
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28028#newcode376
chrome/browser/chromeos/status/network_menu.cc:376: const int images[]) {
On 2010/10/14 17:58:43, Steven Bennetts wrote:
> Since the image arrays are all declared here, can we make them local and pass
an
> enum to this function, rather than passing a global pointer to one of the
> arrays? 
> 

Done.

[stevenjb, 2010-10-19 21:28:09]

LGTM

http://codereview.chromium.org/3748005/diff/27001/28025
File chrome/browser/chromeos/dom_ui/internet_options_handler.cc (right):

http://codereview.chromium.org/3748005/diff/27001/28025#newcode409
chrome/browser/chromeos/dom_ui/internet_options_handler.cc:409: const
chromeos::CellularDataPlan& plan) {
On 2010/10/19 21:02:19, Charlie Lee wrote:
> Hard to move this logic right now. Will look into this later.
> 
> On 2010/10/14 17:58:43, Steven Bennetts wrote:
> > This logic seems very similar to the logic now in network_library. Can we
use
> > the same logic instead of just the same constants?
> > 
> 
> 

Fair enough.