Commit b671af6d authored by Naoki Fukino's avatar Naoki Fukino Committed by Commit Bot

Remove "OS update successful." notification.

We have been showing the notification after successful ext4
migration. This notification is getting less useful since most
users have already migrated to ext4, and we hear several feedback
reports that the notification was shown at unexpected timing.

Let's remove the notification as a simple fix and a clean-up.
(Removing the notification was approved by PM.)

Bug: 894707
Test: Manually confirmed that the notification was not shown after migration on Kevin.
Change-Id: Ide3237a57a127974eed6f1619fc5e096f911f72a
Reviewed-on: https://chromium-review.googlesource.com/c/1278666
Commit-Queue: Naoki Fukino <fukino@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarYusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599568}
parent 90d6b85f
......@@ -3528,12 +3528,6 @@
<message name="IDS_ARC_MIGRATE_ENCRYPTION_NOTIFICATION_RESTART_BUTTON" desc="Label for the button in the notification that navigates the user to sign-out for completing the encryption update step.">
Sign out &amp; sign back in
</message>
<message name="IDS_ARC_MIGRATE_ENCRYPTION_SUCCESS_TITLE" desc="Title of the toast shown on the first sign-in after the successfully completed migration needed for using Android apps.">
OS update successful
</message>
<message name="IDS_ARC_MIGRATE_ENCRYPTION_SUCCESS_MESSAGE" desc="Message of the toast shown on the first sign-in after the successfully completed migration needed for using Android apps.">
You can now use Android apps.
</message>
<message name="IDS_ARC_USB_PERMISSION_TITLE" desc="Titlebar of Android app USB permissions prompt window">
Confirm USB Permission
</message>
......
......@@ -9,8 +9,6 @@
#include "ash/public/cpp/vector_icons/vector_icons.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "chrome/browser/chromeos/arc/arc_migration_constants.h"
#include "chrome/browser/chromeos/arc/arc_util.h"
#include "chrome/browser/lifetime/application_lifetime.h"
......@@ -21,9 +19,6 @@
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
#include "chromeos/dbus/power_manager_client.h"
#include "components/account_id/account_id.h"
#include "components/arc/arc_prefs.h"
#include "components/user_manager/known_user.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/chromeos/devicetype_utils.h"
#include "ui/gfx/color_palette.h"
......@@ -36,37 +31,6 @@ namespace {
constexpr char kNotifierId[] = "arc_fs_migration";
constexpr char kSuggestNotificationId[] = "arc_fs_migration/suggest";
constexpr char kSuccessNotificationId[] = "arc_fs_migration/success";
constexpr base::TimeDelta kSuccessNotificationDelay =
base::TimeDelta::FromSeconds(3);
void DoShowArcMigrationSuccessNotification(Profile* profile) {
message_center::NotifierId notifier_id(
message_center::NotifierId::SYSTEM_COMPONENT, kNotifierId);
notifier_id.profile_id =
multi_user_util::GetAccountIdFromProfile(profile).GetUserEmail();
auto delegate =
base::MakeRefCounted<message_center::HandleNotificationClickDelegate>(
base::BindRepeating(
[](Profile* profile) {
arc::SetArcPlayStoreEnabledForProfile(profile, true);
},
profile));
std::unique_ptr<message_center::Notification> notification =
message_center::Notification::CreateSystemNotification(
message_center::NOTIFICATION_TYPE_SIMPLE, kSuccessNotificationId,
l10n_util::GetStringUTF16(IDS_ARC_MIGRATE_ENCRYPTION_SUCCESS_TITLE),
l10n_util::GetStringUTF16(IDS_ARC_MIGRATE_ENCRYPTION_SUCCESS_MESSAGE),
base::string16(), GURL(), notifier_id,
message_center::RichNotificationData(), std::move(delegate),
ash::kNotificationSettingsIcon,
message_center::SystemNotificationWarningLevel::NORMAL);
NotificationDisplayService::GetForProfile(profile)->Display(
NotificationHandler::Type::TRANSIENT, *notification);
}
} // namespace
......@@ -111,42 +75,4 @@ void ShowArcMigrationGuideNotification(Profile* profile) {
NotificationHandler::Type::TRANSIENT, *notification);
}
void ShowArcMigrationSuccessNotificationIfNeeded(Profile* profile) {
const AccountId account_id =
multi_user_util::GetAccountIdFromProfile(profile);
int pref_value = kFileSystemIncompatible;
user_manager::known_user::GetIntegerPref(
account_id, prefs::kArcCompatibleFilesystemChosen, &pref_value);
// Show notification only when the pref value indicates the file system is
// compatible, but not yet notified.
if (pref_value != kFileSystemCompatible)
return;
if (profile->IsNewProfile()) {
// If this is a newly created profile, the filesystem was compatible from
// the beginning, not because of migration. Skip showing the notification.
} else if (!arc::IsArcAllowedForProfile(profile)) {
// TODO(kinaba; crbug.com/721631): the current message mentions ARC,
// which is inappropriate for users disallowed running ARC.
// Log a warning message because, for now, this should not basically happen
// except for some exceptional situation or due to some bug.
LOG(WARNING) << "Migration has happened for an ARC-disallowed user.";
} else {
// Delay the notification to make sure that it is not hidden behind windows
// which are shown at the beginning of user session (e.g. Chrome).
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&DoShowArcMigrationSuccessNotification, profile),
kSuccessNotificationDelay);
}
// Mark as notified.
user_manager::known_user::SetIntegerPref(
account_id, prefs::kArcCompatibleFilesystemChosen,
arc::kFileSystemCompatibleAndNotified);
}
} // namespace arc
......@@ -14,12 +14,6 @@ namespace arc {
// due to the file system incompatibility.
void ShowArcMigrationGuideNotification(Profile* profile);
// Shows a one-time notification when this is the first sign-in after the
// migration has happened successfully. It records the fact to the pref that no
// more notification is necessary, either when it showed the notification or
// when the profile is newly created on the compatible filesystem.
void ShowArcMigrationSuccessNotificationIfNeeded(Profile* profile);
} // namespace arc
#endif // CHROME_BROWSER_CHROMEOS_ARC_ARC_MIGRATION_GUIDE_NOTIFICATION_H_
......@@ -39,7 +39,10 @@ enum FileSystemCompatibilityState : int32_t {
// Migration has happened. New filesystem is in use.
kFileSystemCompatible = 1,
// Migration has happened, and a notification about it was already shown.
kFileSystemCompatibleAndNotified = 2,
// This pref value will not be written anymore since we stopped showing the
// notification, but existing profiles which had shown the notification can
// have this value in their pref.
kFileSystemCompatibleAndNotifiedDeprecated = 2,
// Existing code assumes that kFileSystemIncompatible is the only state
// representing incompatibility and other values are all variants of
......
......@@ -445,7 +445,7 @@ TEST_F(ChromeArcUtilTest, IsArcCompatibleFileSystemUsedForProfile) {
// New FS (User notified) + Old ARC
user_manager::known_user::SetIntegerPref(
id, prefs::kArcCompatibleFilesystemChosen,
kFileSystemCompatibleAndNotified);
kFileSystemCompatibleAndNotifiedDeprecated);
base::SysInfo::SetChromeOSVersionInfoForTest(
"CHROMEOS_ARC_ANDROID_SDK_VERSION=23", base::Time::Now());
EXPECT_TRUE(IsArcCompatibleFileSystemUsedForUser(user));
......
......@@ -2045,10 +2045,6 @@ void UserSessionManager::DoBrowserLaunchInternal(Profile* profile,
// and show the message accordingly.
tpm_firmware_update::ShowNotificationIfNeeded(profile);
// Show the one-time notification and update the relevant pref about the
// completion of the file system migration necessary for ARC, when needed.
arc::ShowArcMigrationSuccessNotificationIfNeeded(profile);
if (should_launch_browser_)
SyncConsentScreen::MaybeLaunchSyncConstentSettings(profile);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment