New issue
Advanced search Search tips

Issue 910767 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

Remove carrier mobile data promo notifications

Project Member Reported by tbarzic@chromium.org, Nov 30

Issue description

Ash currently supports notification for mobile data carrier promotions, which are shown when the user first connects to a specific cellular network
(see chrome/browser/ui/ash/network/data_promo_notification.h).

The promotions are fetched from mobile_config.json files installed by chromeos-assets package - and yes, all the promotions contained in the file have expired some time mid 2012  - we should remove support for this notification.
(and probably mobile_config.json all together, it doesn't seem to be used for anything else, at least not in Chrome)
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e29d459ca52cba12ffe2026ea83cd50f06c869fc

commit e29d459ca52cba12ffe2026ea83cd50f06c869fc
Author: Tony de Luna <tonydeluna@chromium.org>
Date: Mon Dec 10 19:39:53 2018

Remove support for mobile data promos

Removes logic for displaying mobile data promos. We haven't had mobile
data promos since 2012.

Refactors notification displaying logic, renames variables and method
names to avoid the promo concept.

I'll follow up with a cl that renames this class and all references
to it.

BUG=910767

Change-Id: I4deda2407b268729405af734241b8f36de2d2de9
Reviewed-on: https://chromium-review.googlesource.com/c/1359825
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615204}
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/browser/chromeos/login/ui/login_display_host_common.cc
[delete] https://crrev.com/7c69b5099532dcbf5269c3016d20bd3c43c607a4/chrome/browser/chromeos/mobile_config.cc
[delete] https://crrev.com/7c69b5099532dcbf5269c3016d20bd3c43c607a4/chrome/browser/chromeos/mobile_config.h
[delete] https://crrev.com/7c69b5099532dcbf5269c3016d20bd3c43c607a4/chrome/browser/chromeos/mobile_config_unittest.cc
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/browser/ui/ash/network/data_promo_notification.cc
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/browser/ui/ash/network/data_promo_notification.h
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/common/pref_names.cc
[modify] https://crrev.com/e29d459ca52cba12ffe2026ea83cd50f06c869fc/chrome/common/pref_names.h

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4ae7da177ada33215ecb61969ade5a6570aa349

commit a4ae7da177ada33215ecb61969ade5a6570aa349
Author: Tony de Luna <tonydeluna@chromium.org>
Date: Wed Dec 12 22:13:20 2018

Rename DataPromoNotification to MobileDataNotifications

Notifications for displaying data promotions are no longer supported.

Class is now in charge of displaying mobile data warnings and prompts
user to install data saver extension.

BUG=910767

Change-Id: I74c45001158194788fb335be83e040b95e3271be
Reviewed-on: https://chromium-review.googlesource.com/c/1362211
Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616076}
[modify] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.h
[rename] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/browser/ui/ash/network/mobile_data_notifications.cc
[rename] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/browser/ui/ash/network/mobile_data_notifications.h
[rename] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/browser/ui/ash/network/mobile_data_notifications_unittest.cc
[modify] https://crrev.com/a4ae7da177ada33215ecb61969ade5a6570aa349/chrome/test/BUILD.gn

Owner: tonydeluna@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49d89d55ff609a1adc9511866e1b2e0358cd391f

commit 49d89d55ff609a1adc9511866e1b2e0358cd391f
Author: Tony de Luna <tonydeluna@chromium.org>
Date: Fri Dec 14 19:17:34 2018

cros: Remove data saver extension notification

Stop showing chromeos users a notification prompting them to install a
data saver extension the first time they connect with a cellular
connection. Extension was recently removed.

Remove flag 'enable-datasaver-prompt' and deprecate
prefs::kDataSaverPromptsShown.

Add more tests to validate that notifications are only shown once.

Bug:  914470 , 910767
Change-Id: I0c7799d9a57fcb207fcea946a4454b4edc466ef4
Reviewed-on: https://chromium-review.googlesource.com/c/1374581
Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616779}
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/about_flags.cc
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/ui/ash/network/mobile_data_notifications.cc
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/ui/ash/network/mobile_data_notifications.h
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chrome/browser/ui/ash/network/mobile_data_notifications_unittest.cc
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chromeos/chromeos_switches.cc
[modify] https://crrev.com/49d89d55ff609a1adc9511866e1b2e0358cd391f/chromeos/chromeos_switches.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/feec5c2fb91ff5d802ace328c18d2700720fc86e

commit feec5c2fb91ff5d802ace328c18d2700720fc86e
Author: Tony de Luna <tonydeluna@chromium.org>
Date: Tue Dec 18 01:05:08 2018

Display mobile data notifications as system notifications

Bug: 910767
Change-Id: Icb5adce6672a9a235df2b8b0c084c5ef9519a00a
Reviewed-on: https://chromium-review.googlesource.com/c/1380795
Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617321}
[modify] https://crrev.com/feec5c2fb91ff5d802ace328c18d2700720fc86e/chrome/browser/ui/ash/network/mobile_data_notifications.cc
[modify] https://crrev.com/feec5c2fb91ff5d802ace328c18d2700720fc86e/chrome/browser/ui/ash/network/mobile_data_notifications_unittest.cc

As part of this code we're showing a warning notification to the user the first time they've used cellular data on their device.

Once shown, the notification is there until the user closes it. 


Should we be hiding this notification if the user switches back to WiFi/Ethernet? 

Maybe we can let it be shown for 10 seconds and hide it? 

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c07af6a5606da8c5c68df653543d8e8290d75d3b

commit c07af6a5606da8c5c68df653543d8e8290d75d3b
Author: Tony de Luna <tonydeluna@chromium.org>
Date: Mon Jan 14 23:34:03 2019

mobile notifications: update observers

Stop subscribing to all network property updates.

Subscribe to default network changes to show a notification when
cellular becomes the default network.

Subscribe to session updates to show the notification if cellular
is the default network after a session change.

We avoid showing the notification if there are other network
connections pending. Subscribe to network connections changes to make
sure we don't accidentally miss legitimate updates.

Bug: 910767
Change-Id: I189b16cb5fc19428c13848e7eb193952ca6655ac
Reviewed-on: https://chromium-review.googlesource.com/c/1399428
Commit-Queue: Tony De Luna <tonydeluna@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622638}
[modify] https://crrev.com/c07af6a5606da8c5c68df653543d8e8290d75d3b/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc
[modify] https://crrev.com/c07af6a5606da8c5c68df653543d8e8290d75d3b/chrome/browser/ui/ash/network/mobile_data_notifications.cc
[modify] https://crrev.com/c07af6a5606da8c5c68df653543d8e8290d75d3b/chrome/browser/ui/ash/network/mobile_data_notifications.h
[modify] https://crrev.com/c07af6a5606da8c5c68df653543d8e8290d75d3b/chrome/browser/ui/ash/network/mobile_data_notifications_unittest.cc
[modify] https://crrev.com/c07af6a5606da8c5c68df653543d8e8290d75d3b/components/user_manager/user_manager_base.cc
[modify] https://crrev.com/c07af6a5606da8c5c68df653543d8e8290d75d3b/components/user_manager/user_manager_base.h

Sign in to add a comment