Skip to content

Commit

Permalink
[dPWA] Adding support for intent picker UX in WebAppIntegrationTestDr…
Browse files Browse the repository at this point in the history
…iver

This CL adds support for the new intent picker UX to be used in the
WebAppIntegrationTestDriver by:
1. Piping whether the app icon will be shown all the way from the
IntentPickerTabHelper to the test driver so that the latter can
selectively wait if the icon needs to be shown in the intent chip.
2. Adds an observer that observes visibility for the intent chip
button, and smartly determines if it needs to wait for the chip
visibility to change.

Verified by running existing WebAppIntegration tests that were
failing locally once the flag was removed.

Bug: 1412856, 1357905
Change-Id: Ia376bccb2c2dcaaa5c4df35f9d91a3398f0830ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942518
Reviewed-by: Tim Sergeant <tsergeant@chromium.org>
Reviewed-by: Elly FJ <ellyjones@chromium.org>
Commit-Queue: Dibyajyoti Pal <dibyapal@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1211513}
  • Loading branch information
Dp-Goog authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent 2c18705 commit 36f2c72
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ namespace web_app {
WebAppsSyncTestBase::WebAppsSyncTestBase(TestType test_type)
: SyncTest(test_type) {
std::vector<base::test::FeatureRef> disabled_features;

#if BUILDFLAG(IS_CHROMEOS)
// TODO(crbug.com/1357905): Update test driver to work with new UI.
disabled_features.push_back(apps::features::kLinkCapturingUiUpdate);
#endif

#if BUILDFLAG(IS_CHROMEOS_ASH)
// Disable Lacros, so that Web Apps get synced in the Ash browser.
// TODO(crbug.com/1462253): Also test with Lacros flags enabled.
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/ui/intent_picker_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,8 @@ void IntentPickerTabHelper::ShowOrHideIconInternal(bool should_show_icon) {
browser->window()->UpdatePageActionIcon(PageActionIconType::kIntentPicker);

icon_resolved_after_last_navigation_ = true;
if (icon_update_closure_for_testing_) {
std::move(icon_update_closure_for_testing_).Run();
if (icon_update_callback_for_testing_) {
std::move(icon_update_callback_for_testing_).Run(should_show_icon);
}
}

Expand Down Expand Up @@ -410,13 +410,13 @@ void IntentPickerTabHelper::OnIntentPickerClosedMaybeLaunch(
}

void IntentPickerTabHelper::SetIconUpdateCallbackForTesting(
base::OnceClosure callback,
base::OnceCallback<void(bool)> icon_update_callback,
bool include_latest_navigation) {
if (icon_resolved_after_last_navigation_ && include_latest_navigation) {
std::move(callback).Run();
std::move(icon_update_callback).Run(should_show_icon_);
return;
}
icon_update_closure_for_testing_ = std::move(callback);
icon_update_callback_for_testing_ = std::move(icon_update_callback);
}

void IntentPickerTabHelper::DidStartNavigation(
Expand Down
15 changes: 8 additions & 7 deletions chrome/browser/ui/intent_picker_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,13 @@ class IntentPickerTabHelper

const ui::ImageModel& app_icon() const { return current_app_icon_; }


// Sets a OnceClosure callback which will be called next time the icon is
// updated. If include_latest_navigation is true, and the latest navigation
// was finished, the callback is called immediately.
void SetIconUpdateCallbackForTesting(base::OnceClosure callback,
bool include_latest_navigation = false);
// Sets a OnceCallback which is called with the result of whether the intent
// chip button is shown or not. If include_latest_navigation is true, and the
// latest navigation was finished, the callback is called immediately with the
// current value of should_show_icon_.
void SetIconUpdateCallbackForTesting(
base::OnceCallback<void(bool)> icon_update_callback,
bool include_latest_navigation = false);

WEB_CONTENTS_USER_DATA_KEY_DECL();

Expand Down Expand Up @@ -145,7 +146,7 @@ class IntentPickerTabHelper
bool current_app_is_preferred_ = false;
ui::ImageModel current_app_icon_;

base::OnceClosure icon_update_closure_for_testing_;
base::OnceCallback<void(bool)> icon_update_callback_for_testing_;

std::unique_ptr<apps::AppsIntentPickerDelegate> intent_picker_delegate_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class EnableLinkCapturingInfobarBrowserTest
}

testing::AssertionResult ClickIntentPickerAndWaitForBubble() {
base::test::TestFuture<void> future;
base::test::TestFuture<bool> future;
auto* tab_helper =
IntentPickerTabHelper::FromWebContents(GetActiveWebContents(browser()));
tab_helper->SetIconUpdateCallbackForTesting(
Expand Down
14 changes: 7 additions & 7 deletions chrome/browser/ui/views/intent_picker_bubble_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include <string>

#include "base/functional/bind.h"
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
#include "build/build_config.h"
#include "chrome/browser/apps/link_capturing/link_capturing_features.h"
#include "chrome/browser/ui/browser.h"
Expand Down Expand Up @@ -47,11 +47,11 @@ class IntentPickerBrowserTest

template <typename Action>
void DoAndWaitForIntentPickerIconUpdate(Action action) {
base::RunLoop run_loop;
base::test::TestFuture<bool> future;
auto* tab_helper = IntentPickerTabHelper::FromWebContents(GetWebContents());
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
tab_helper->SetIconUpdateCallbackForTesting(future.GetCallback());
action();
run_loop.Run();
EXPECT_TRUE(future.Wait()) << " intent picker icon did not update";
}

content::WebContents* OpenNewTab(const GURL& url,
Expand Down Expand Up @@ -161,12 +161,12 @@ IN_PROC_BROWSER_TEST_P(IntentPickerIconBrowserTest,
auto* tab_helper = IntentPickerTabHelper::FromWebContents(GetWebContents());
NavigateToLaunchingPage(browser());

base::RunLoop run_loop;
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
base::test::TestFuture<bool> future;
tab_helper->SetIconUpdateCallbackForTesting(future.GetCallback());
TestTabActionDoesNotOpenAppWindow(
in_scope_url, base::BindOnce(&ClickLinkAndWait, GetWebContents(),
in_scope_url, LinkTarget::SELF, GetParam()));
run_loop.Run();
EXPECT_TRUE(future.Get());

views::Button* intent_picker_icon = GetIntentPickerIcon();
EXPECT_TRUE(intent_picker_icon->GetVisible());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// Copyright 2020 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/memory/raw_ptr.h"
#include "build/build_config.h"
#include "chrome/browser/ui/views/intent_picker_bubble_view.h"

#include <memory>
Expand All @@ -14,10 +11,13 @@
#include "ash/components/arc/test/arc_util_test_support.h"
#include "ash/components/arc/test/connection_holder_util.h"
#include "ash/components/arc/test/fake_app_instance.h"
#include "base/memory/raw_ptr.h"
#include "base/run_loop.h"
#include "base/scoped_observation.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
#include "build/build_config.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/apps/link_capturing/link_capturing_features.h"
Expand Down Expand Up @@ -362,11 +362,11 @@ class IntentPickerBubbleViewBrowserTestChromeOS : public InProcessBrowserTest,

template <typename Action>
void DoAndWaitForIntentPickerIconUpdate(Action action) {
base::RunLoop run_loop;
base::test::TestFuture<bool> test_future;
auto* tab_helper = IntentPickerTabHelper::FromWebContents(GetWebContents());
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
tab_helper->SetIconUpdateCallbackForTesting(test_future.GetCallback());
action();
run_loop.Run();
EXPECT_TRUE(test_future.Wait()) << " intent picker icon did not update";
}

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <string>

#include "base/test/test_future.h"
#include "chrome/browser/apps/link_capturing/mac_intent_picker_helpers.h"
#include "chrome/browser/ui/intent_picker_tab_helper.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
Expand Down Expand Up @@ -36,11 +37,11 @@ class IntentPickerBubbleViewBrowserTestMac : public InProcessBrowserTest {

template <typename Action>
void DoAndWaitForIntentPickerIconUpdate(Action action) {
base::RunLoop run_loop;
base::test::TestFuture<bool> test_future;
auto* tab_helper = IntentPickerTabHelper::FromWebContents(GetWebContents());
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
tab_helper->SetIconUpdateCallbackForTesting(test_future.GetCallback());
action();
run_loop.Run();
EXPECT_TRUE(test_future.Wait()) << " intent picker icon did not update";
}

size_t GetItemContainerSize(IntentPickerBubbleView* bubble) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/scoped_observation.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/apps/app_service/app_registry_cache_waiter.h"
Expand Down Expand Up @@ -71,11 +72,11 @@ class IntentChipButtonBrowserTest
void DoAndWaitForIntentPickerIconUpdate(Action action) {
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
base::RunLoop run_loop;
base::test::TestFuture<bool> test_future;
auto* tab_helper = IntentPickerTabHelper::FromWebContents(web_contents);
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
tab_helper->SetIconUpdateCallbackForTesting(test_future.GetCallback());
action();
run_loop.Run();
EXPECT_TRUE(test_future.Wait());
}

void OpenNewTab(const GURL& url) {
Expand Down Expand Up @@ -115,10 +116,10 @@ class IntentChipButtonBrowserTest
const GURL& link_url) {
auto* tab_helper = IntentPickerTabHelper::FromWebContents(web_contents);

base::RunLoop run_loop;
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
base::test::TestFuture<bool> test_future;
tab_helper->SetIconUpdateCallbackForTesting(test_future.GetCallback());
ClickLinkAndWait(web_contents, link_url, LinkTarget::SELF, "");
run_loop.Run();
EXPECT_TRUE(test_future.Wait());
}

// Installs a web app on the same host as InstallTestWebApp(), but with "/" as
Expand Down Expand Up @@ -317,11 +318,11 @@ void IntentChipButtonBrowserUiTest::ShowUi(const std::string& name) {
auto* const web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
auto* const tab_helper = IntentPickerTabHelper::FromWebContents(web_contents);
base::RunLoop run_loop;
tab_helper->SetIconUpdateCallbackForTesting(run_loop.QuitClosure());
base::test::TestFuture<bool> test_future;
tab_helper->SetIconUpdateCallbackForTesting(test_future.GetCallback());
tab_helper->MaybeShowIconForApps(
{{apps::PickerEntryType::kWeb, ui::ImageModel(), "app_id", "Test app"}});
run_loop.Run();
EXPECT_TRUE(test_future.Wait());
}

bool IntentChipButtonBrowserUiTest::VerifyUi() {
Expand Down

0 comments on commit 36f2c72

Please sign in to comment.