Commit c4dc65d4 authored by Zufeng Wang's avatar Zufeng Wang Committed by Commit Bot

Workaround to make release notes chip disappear after some views

There's an issue of AppServiceAppResult::OnVisibilityChanged not being
called for suggestion chips, which meant the Decrease function never
ran.
As a result, the release notes suggestion chip never disappeared.

As a temporary workaround, move the Decrease function into
AppServiceAppResult::HandleSuggestionChip. This gets called a bit more
often (including when the session starts and when the launcher input is
focused). Increase kTimesToShowSuggestionChip to compensate for this.

Bug: b/169711884
Change-Id: I1f1827f5c8d4d5a37268074a4af4d2e4840c326b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2442809
Commit-Queue: Zufeng Wang <zufeng@google.com>
Reviewed-by: default avatarNancy Wang <nancylingwang@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814044}
parent 3c3b4c66
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
namespace { namespace {
constexpr int kTimesToShowSuggestionChip = 3; constexpr int kTimesToShowSuggestionChip = 6;
int GetMilestone() { int GetMilestone() {
return version_info::GetVersion().components()[0]; return version_info::GetVersion().components()[0];
......
...@@ -163,7 +163,7 @@ TEST_F(ReleaseNotesStorageTest, ShowSuggestionChipWhenNotificationShown) { ...@@ -163,7 +163,7 @@ TEST_F(ReleaseNotesStorageTest, ShowSuggestionChipWhenNotificationShown) {
release_notes_storage->MarkNotificationShown(); release_notes_storage->MarkNotificationShown();
EXPECT_EQ(3, profile.get()->GetPrefs()->GetInteger( EXPECT_EQ(6, profile.get()->GetPrefs()->GetInteger(
prefs::kReleaseNotesSuggestionChipTimesLeftToShow)); prefs::kReleaseNotesSuggestionChipTimesLeftToShow));
EXPECT_EQ(true, release_notes_storage->ShouldShowSuggestionChip()); EXPECT_EQ(true, release_notes_storage->ShouldShowSuggestionChip());
} }
......
...@@ -155,11 +155,16 @@ IN_PROC_BROWSER_TEST_F(ReleaseNotesSearchBrowserTest, ...@@ -155,11 +155,16 @@ IN_PROC_BROWSER_TEST_F(ReleaseNotesSearchBrowserTest,
->system_web_app_manager() ->system_web_app_manager()
.InstallSystemAppsForTesting(); .InstallSystemAppsForTesting();
GetProfile()->GetPrefs()->SetInteger( GetProfile()->GetPrefs()->SetInteger(
prefs::kReleaseNotesSuggestionChipTimesLeftToShow, 1); prefs::kReleaseNotesSuggestionChipTimesLeftToShow, 3);
SearchAndWaitForProviders("", SearchAndWaitForProviders("",
{ResultType::kInstalledApp, ResultType::kLauncher}); {ResultType::kInstalledApp, ResultType::kLauncher});
// Note: SearchAndWaitForProviders decreases the count multiple times.
// TODO(b/169711884): Decrease times left only when the chip becomes visible.
const int times_left_to_show = GetProfile()->GetPrefs()->GetInteger(
prefs::kReleaseNotesSuggestionChipTimesLeftToShow);
EXPECT_EQ(times_left_to_show, 1);
auto* result = FindResult(chromeos::default_web_apps::kHelpAppId); auto* result = FindResult(chromeos::default_web_apps::kHelpAppId);
ASSERT_TRUE(result); ASSERT_TRUE(result);
// Has Release notes title. // Has Release notes title.
......
...@@ -106,14 +106,6 @@ void AppServiceAppResult::GetContextMenuModel(GetMenuModelCallback callback) { ...@@ -106,14 +106,6 @@ void AppServiceAppResult::GetContextMenuModel(GetMenuModelCallback callback) {
context_menu_->GetMenuModel(std::move(callback)); context_menu_->GetMenuModel(std::move(callback));
} }
void AppServiceAppResult::OnVisibilityChanged(bool visibility) {
if (id() == ash::kReleaseNotesAppId && visibility) {
DCHECK(chromeos::ReleaseNotesStorage(profile()).ShouldShowSuggestionChip());
chromeos::ReleaseNotesStorage(profile())
.DecreaseTimesLeftToShowSuggestionChip();
}
}
ash::SearchResultType AppServiceAppResult::GetSearchResultType() const { ash::SearchResultType AppServiceAppResult::GetSearchResultType() const {
switch (app_type_) { switch (app_type_) {
case apps::mojom::AppType::kArc: case apps::mojom::AppType::kArc:
...@@ -261,7 +253,10 @@ void AppServiceAppResult::HandleSuggestionChip(Profile* profile) { ...@@ -261,7 +253,10 @@ void AppServiceAppResult::HandleSuggestionChip(Profile* profile) {
// Either of these apps could be shown as the release notes suggestion chip. // Either of these apps could be shown as the release notes suggestion chip.
if (id() == ash::kReleaseNotesAppId || if (id() == ash::kReleaseNotesAppId ||
id() == chromeos::default_web_apps::kHelpAppId) { id() == chromeos::default_web_apps::kHelpAppId) {
SetNotifyVisibilityChange(true); // TODO(b/169711884): Decrease times left only when the chip becomes
// visible.
chromeos::ReleaseNotesStorage(profile)
.DecreaseTimesLeftToShowSuggestionChip();
// Make sure that if both Continue Reading and Release Notes are available, // Make sure that if both Continue Reading and Release Notes are available,
// Release Notes shows up first in the suggestion chip container. // Release Notes shows up first in the suggestion chip container.
SetPositionPriority(1.0f); SetPositionPriority(1.0f);
......
...@@ -42,7 +42,6 @@ class AppServiceAppResult : public AppResult { ...@@ -42,7 +42,6 @@ class AppServiceAppResult : public AppResult {
// ChromeSearchResult overrides: // ChromeSearchResult overrides:
void Open(int event_flags) override; void Open(int event_flags) override;
void GetContextMenuModel(GetMenuModelCallback callback) override; void GetContextMenuModel(GetMenuModelCallback callback) override;
void OnVisibilityChanged(bool visibility) override;
AppContextMenu* GetAppContextMenu() override; AppContextMenu* GetAppContextMenu() override;
// AppContextMenuDelegate overrides: // AppContextMenuDelegate overrides:
......
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