Show notification when activating with no connection

Show notification when activating with no connection

This also removes some code that prevented notifications from showing while the detailed view was shown. This behavior is really only desired for SMS, we would actually like to see network/VPN errors. The SMS logic was changed to supress / hide notifications when the detailed view is visible / shown.

BUG=172528
For chrome/browser/ui/ash/:
TBR=sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182645

[jennyz, 2013-02-14 23:26:44]

https://chromiumcodereview.appspot.com/12260046/diff/2001/ash/system/chromeos...
File ash/system/chromeos/network/tray_sms.cc (right):

https://chromiumcodereview.appspot.com/12260046/diff/2001/ash/system/chromeos...
ash/system/chromeos/network/tray_sms.cc:305: HideNotificationView();
Why do you need to HideNotificationView twice?

[rkc, 2013-02-14 23:32:12]

https://chromiumcodereview.appspot.com/12260046/diff/2001/chrome/browser/ui/a...
File chrome/browser/ui/ash/chrome_shell_delegate.cc (right):

https://chromiumcodereview.appspot.com/12260046/diff/2001/chrome/browser/ui/a...
chrome/browser/ui/ash/chrome_shell_delegate.cc:262: if (cellular &&
!cellular->activate_over_non_cellular_network() &&
so I'm reading this as,
1.) (cellular &&) if we have a cellular network and
2.) (!cellular->activate_over_non_cellular_network() &&) the cellular network
does not support activation over non-cellular-networks and
3.) we're not online on a network

then show the notification - all of it makes sense except #2. Won't LTE networks
return true for cros->activate_over_non_cellular_network() since the only
possible activation for LTE networks is to activate over non-cellular?

[stevenjb, 2013-02-14 23:32:25]

https://chromiumcodereview.appspot.com/12260046/diff/2001/ash/system/chromeos...
File ash/system/chromeos/network/tray_sms.cc (right):

https://chromiumcodereview.appspot.com/12260046/diff/2001/ash/system/chromeos...
ash/system/chromeos/network/tray_sms.cc:305: HideNotificationView();
On 2013/02/14 23:26:45, jennyz wrote:
> Why do you need to HideNotificationView twice? 
Oops, didn't see that we already did that. Fixed.

[jennyz, 2013-02-14 23:35:12]

lgtm to files under ash/system.

[stevenjb, 2013-02-14 23:43:17]

https://chromiumcodereview.appspot.com/12260046/diff/2001/chrome/browser/ui/a...
File chrome/browser/ui/ash/chrome_shell_delegate.cc (right):

https://chromiumcodereview.appspot.com/12260046/diff/2001/chrome/browser/ui/a...
chrome/browser/ui/ash/chrome_shell_delegate.cc:262: if (cellular &&
!cellular->activate_over_non_cellular_network() &&
On 2013/02/14 23:32:12, Rahul Chaturvedi wrote:
> so I'm reading this as,
> 1.) (cellular &&) if we have a cellular network and
> 2.) (!cellular->activate_over_non_cellular_network() &&) the cellular network
> does not support activation over non-cellular-networks and
> 3.) we're not online on a network
> 
> then show the notification - all of it makes sense except #2. Won't LTE
networks
> return true for cros->activate_over_non_cellular_network() since the only
> possible activation for LTE networks is to activate over non-cellular?

Thank you, yes, I got the logic backward (test image is still building...)

[rkc, 2013-02-15 00:01:33]

lgtm

[commit-bot: I haz the power, 2013-02-15 02:40:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/12260046/7

[commit-bot: I haz the power, 2013-02-15 02:40:51]

Presubmit check for 12260046-7 failed and returned exit status 1.

INFO:root:Found 5 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ash/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/ash

Presubmit checks took 1.8s to calculate.

[commit-bot: I haz the power, 2013-02-15 02:43:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/12260046/7

[commit-bot: I haz the power, 2013-02-15 02:43:52]

Presubmit check for 12260046-7 failed and returned exit status 1.

INFO:root:Found 5 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ash/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/ash

Presubmit checks took 1.1s to calculate.

[commit-bot: I haz the power, 2013-02-15 02:46:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/12260046/7

[commit-bot: I haz the power, 2013-02-15 02:46:51]

Presubmit check for 12260046-7 failed and returned exit status 1.

INFO:root:Found 5 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/ash/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/ash

[commit-bot: I haz the power, 2013-02-15 02:52:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/12260046/7

[commit-bot: I haz the power, 2013-02-15 08:02:32]

Change committed as 182645