Commit e9054085 authored by Eric Willigers's avatar Eric Willigers Committed by Commit Bot

desktop-pwas: Only show minimal-ui buttons if requested in manifest

If the web app manifest specifies 'standalone' or 'fullscreen', we do not
show minimal-ui Back and Reload buttons.

We only show the buttons when a web app is being opened in a standalone
window, and the manifest requested 'browser' or 'minimal-ui', or failed to
specify a display mode.

https://w3c.github.io/manifest/#display-modes


Not yet implemented: support for the extensions-based implementation
of bookmark apps - crbug.com/1014346

Bug: 1007151
Change-Id: I68be5280d2d5cfb1ad1565c9408635a694aad2c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1883271
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711141}
parent e0619766
......@@ -221,7 +221,23 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
layout.set_cross_axis_alignment(
views::BoxLayout::CrossAxisAlignment::kCenter);
auto* app_controller = browser_view_->browser()->app_controller();
const auto* app_controller = browser_view_->browser()->app_controller();
if (base::FeatureList::IsEnabled(features::kDesktopMinimalUI) &&
app_controller->HasMinimalUiButtons()) {
// TODO(crbug.com/1007151): Place buttons at far left of title bar.
// TODO(crbug.com/1007151): Make the icons have correct sizes.
back_ = AddChildView(CreateBackButton(this, browser_view->browser()));
reload_ = AddChildView(CreateReloadButton(browser_view->browser()));
views::SetHitTestComponent(back_, static_cast<int>(HTCLIENT));
views::SetHitTestComponent(reload_, static_cast<int>(HTCLIENT));
chrome::AddCommandObserver(browser_view->browser(), IDC_BACK, this);
chrome::AddCommandObserver(browser_view->browser(), IDC_RELOAD, this);
md_observer_.Add(ui::MaterialDesignController::GetInstance());
}
if (app_controller->HasTitlebarAppOriginText()) {
web_app_origin_text_ = AddChildView(
std::make_unique<WebAppOriginText>(browser_view->browser()));
......@@ -282,21 +298,6 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
AddChildView(std::make_unique<WebAppMenuButton>(browser_view));
#endif
if (base::FeatureList::IsEnabled(features::kDesktopMinimalUI)) {
// TODO(crbug.com/1007151): Only create buttons for Minimal UI PWAs.
// TODO(crbug.com/1007151): Place buttons at far left of title bar.
// TODO(crbug.com/1007151): Make the icons have correct sizes.
back_ = AddChildView(CreateBackButton(this, browser_view->browser()));
reload_ = AddChildView(CreateReloadButton(browser_view->browser()));
views::SetHitTestComponent(back_, static_cast<int>(HTCLIENT));
views::SetHitTestComponent(reload_, static_cast<int>(HTCLIENT));
chrome::AddCommandObserver(browser_view->browser(), IDC_BACK, this);
chrome::AddCommandObserver(browser_view->browser(), IDC_RELOAD, this);
md_observer_.Add(ui::MaterialDesignController::GetInstance());
}
UpdateChildrenColor();
UpdateStatusIconsVisibility();
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/macros.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/browser/ui/web_applications/web_app_controller_browsertest.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/web_application_info.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
#include "url/gurl.h"
namespace web_app {
class WebAppMinimalUITest : public WebAppControllerBrowserTest {
public:
WebAppMinimalUITest() {
scoped_feature_list_.InitWithFeatures({features::kDesktopMinimalUI}, {});
}
BrowserView* CreateBrowserView(blink::mojom::DisplayMode display_mode) {
auto web_app_info = std::make_unique<WebApplicationInfo>();
web_app_info->app_url = GURL("https://example.org");
web_app_info->display_mode = display_mode;
web_app_info->open_as_window = true;
AppId app_id = InstallWebApp(std::move(web_app_info));
Browser* browser = LaunchWebAppBrowser(app_id);
return BrowserView::GetBrowserViewForBrowser(browser);
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(WebAppMinimalUITest);
};
IN_PROC_BROWSER_TEST_P(WebAppMinimalUITest, Standalone) {
BrowserView* browser_view =
CreateBrowserView(blink::mojom::DisplayMode::kStandalone);
ToolbarButtonProvider* provider = browser_view->toolbar_button_provider();
EXPECT_FALSE(!!provider->GetBackButton());
EXPECT_FALSE(!!provider->GetReloadButton());
}
IN_PROC_BROWSER_TEST_P(WebAppMinimalUITest, MinimalUi) {
BrowserView* browser_view =
CreateBrowserView(blink::mojom::DisplayMode::kMinimalUi);
ToolbarButtonProvider* provider = browser_view->toolbar_button_provider();
EXPECT_TRUE(!!provider->GetBackButton());
EXPECT_TRUE(!!provider->GetReloadButton());
}
// TODO(crbug.com/1014346): Support hosted apps.
INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
WebAppMinimalUITest,
::testing::Values(ControllerType::kUnifiedControllerWithWebApp),
ControllerTypeParamToString);
} // namespace web_app
......@@ -208,6 +208,10 @@ bool AppBrowserController::UseTitlebarTerminalSystemAppMenu() const {
}
#endif
bool AppBrowserController::HasMinimalUiButtons() const {
return false;
}
bool AppBrowserController::IsInstalled() const {
return false;
}
......
......@@ -74,6 +74,9 @@ class AppBrowserController : public TabStripModelObserver,
virtual bool UseTitlebarTerminalSystemAppMenu() const;
#endif
// Whether to show the Back and Refresh buttons in the web app toolbar.
virtual bool HasMinimalUiButtons() const;
// Returns the app icon for the window to use in the task list.
virtual gfx::ImageSkia GetWindowAppIcon() const = 0;
......
......@@ -12,6 +12,7 @@
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "content/public/browser/web_contents.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
#include "ui/gfx/favicon_size.h"
#include "ui/gfx/image/image.h"
#include "url/gurl.h"
......@@ -33,6 +34,11 @@ bool WebAppBrowserController::CreatedForInstalledPwa() const {
return !registrar().IsShortcutApp(app_id_);
}
bool WebAppBrowserController::HasMinimalUiButtons() const {
return registrar().GetAppEffectiveDisplayMode(app_id_) ==
blink::mojom::DisplayMode::kMinimalUi;
}
bool WebAppBrowserController::IsHostedApp() const {
return true;
}
......
......@@ -39,6 +39,7 @@ class WebAppBrowserController : public AppBrowserController {
// AppBrowserController:
base::Optional<AppId> GetAppId() const override;
bool CreatedForInstalledPwa() const override;
bool HasMinimalUiButtons() const override;
gfx::ImageSkia GetWindowAppIcon() const override;
gfx::ImageSkia GetWindowIcon() const override;
base::Optional<SkColor> GetThemeColor() const override;
......
......@@ -2,11 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/strings/string_number_conversions.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/web_applications/app_browser_controller.h"
#include "chrome/browser/ui/web_applications/web_app_controller_browsertest.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/common/web_application_info.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
#include "third_party/skia/include/core/SkColor.h"
namespace {
......@@ -19,6 +21,8 @@ namespace web_app {
class WebAppBrowserTest : public WebAppControllerBrowserTest {};
using WebAppOnlyBrowserTest = WebAppBrowserTest;
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, CreatedForInstalledPwaForPwa) {
auto web_app_info = std::make_unique<WebApplicationInfo>();
web_app_info->app_url = GURL(kExampleURL);
......@@ -56,6 +60,35 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ThemeColor) {
}
}
// TODO(crbug.com/1014346): Support hosted apps, use WebAppBrowserTest.
IN_PROC_BROWSER_TEST_P(WebAppOnlyBrowserTest, HasMinimalUiButtons) {
int index = 0;
auto has_buttons = [this, &index](blink::mojom::DisplayMode display_mode,
bool open_as_window) -> bool {
const std::string base_url = "https://example.com/path";
auto web_app_info = std::make_unique<WebApplicationInfo>();
web_app_info->app_url = GURL(base_url + base::NumberToString(index++));
web_app_info->scope = web_app_info->app_url;
web_app_info->display_mode = display_mode;
web_app_info->open_as_window = open_as_window;
AppId app_id = InstallWebApp(std::move(web_app_info));
Browser* app_browser = LaunchWebAppBrowser(app_id);
return app_browser->app_controller()->HasMinimalUiButtons();
};
EXPECT_TRUE(has_buttons(blink::mojom::DisplayMode::kBrowser,
/*open_as_window=*/true));
EXPECT_TRUE(has_buttons(blink::mojom::DisplayMode::kMinimalUi,
/*open_as_window=*/true));
EXPECT_FALSE(has_buttons(blink::mojom::DisplayMode::kStandalone,
/*open_as_window=*/true));
EXPECT_FALSE(has_buttons(blink::mojom::DisplayMode::kFullscreen,
/*open_as_window=*/true));
EXPECT_FALSE(has_buttons(blink::mojom::DisplayMode::kMinimalUi,
/*open_as_window=*/false));
}
INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
WebAppBrowserTest,
......@@ -63,4 +96,9 @@ INSTANTIATE_TEST_SUITE_P(
ControllerType::kUnifiedControllerWithBookmarkApp,
ControllerType::kUnifiedControllerWithWebApp));
INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
WebAppOnlyBrowserTest,
::testing::Values(ControllerType::kUnifiedControllerWithWebApp));
} // namespace web_app
......@@ -116,6 +116,7 @@ source_set("unit_tests") {
"app_shortcut_manager_unittest.cc",
"file_handler_manager_unittest.cc",
"pending_app_manager_unittest.cc",
"web_app_constants_unittest.cc",
"web_app_data_retriever_unittest.cc",
"web_app_helpers_unittest.cc",
"web_app_icon_downloader_unittest.cc",
......
......@@ -79,7 +79,9 @@ class AppRegistrar {
virtual const GURL& GetAppLaunchURL(const AppId& app_id) const = 0;
virtual base::Optional<GURL> GetAppScope(const AppId& app_id) const = 0;
virtual blink::mojom::DisplayMode GetAppUserDisplayMode(
const web_app::AppId& app_id) const = 0;
const AppId& app_id) const = 0;
virtual blink::mojom::DisplayMode GetAppEffectiveDisplayMode(
const AppId& app_id) const = 0;
virtual std::vector<AppId> GetAppIds() const = 0;
......
......@@ -4,6 +4,8 @@
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
namespace web_app {
static_assert(Source::kMinValue == 0, "Source enum should be zero based");
......@@ -13,4 +15,32 @@ bool IsSuccess(InstallResultCode code) {
code == InstallResultCode::kSuccessAlreadyInstalled;
}
blink::mojom::DisplayMode ResolveEffectiveDisplayMode(
blink::mojom::DisplayMode app_display_mode,
blink::mojom::DisplayMode user_display_mode) {
switch (user_display_mode) {
case blink::mojom::DisplayMode::kBrowser:
return blink::mojom::DisplayMode::kBrowser;
case blink::mojom::DisplayMode::kUndefined:
case blink::mojom::DisplayMode::kMinimalUi:
case blink::mojom::DisplayMode::kFullscreen:
NOTREACHED();
FALLTHROUGH;
case blink::mojom::DisplayMode::kStandalone:
break;
}
switch (app_display_mode) {
case blink::mojom::DisplayMode::kBrowser:
case blink::mojom::DisplayMode::kMinimalUi:
return blink::mojom::DisplayMode::kMinimalUi;
case blink::mojom::DisplayMode::kUndefined:
NOTREACHED();
FALLTHROUGH;
case blink::mojom::DisplayMode::kStandalone:
case blink::mojom::DisplayMode::kFullscreen:
return blink::mojom::DisplayMode::kStandalone;
}
}
} // namespace web_app
......@@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_CONSTANTS_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_CONSTANTS_H_
#include "third_party/blink/public/mojom/manifest/display_mode.mojom-forward.h"
namespace web_app {
// Install sources are listed in the order of priority (from top to bottom).
......@@ -135,6 +137,14 @@ enum class ExternalInstallSource {
kArc = 4,
};
// When user_display_mode indicates a user preference for opening in
// a browser tab, we open in a browser tab. Otherwise, we open in a standalone
// window (for app_display_mode 'standalone' or 'fullscreen'), or a minimal-ui
// window (for app_display_mode 'browser' or 'minimal-ui').
blink::mojom::DisplayMode ResolveEffectiveDisplayMode(
blink::mojom::DisplayMode app_display_mode,
blink::mojom::DisplayMode user_display_mode);
} // namespace web_app
#endif // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_CONSTANTS_H_
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
using blink::mojom::DisplayMode;
namespace web_app {
TEST(WebAppConstants, IsSuccess) {
EXPECT_TRUE(IsSuccess(InstallResultCode::kSuccessNewInstall));
EXPECT_TRUE(IsSuccess(InstallResultCode::kSuccessAlreadyInstalled));
EXPECT_FALSE(IsSuccess(InstallResultCode::kFailedUnknownReason));
EXPECT_FALSE(IsSuccess(InstallResultCode::kExpectedAppIdCheckFailed));
}
TEST(WebAppConstants, ResolveEffectiveDisplayMode) {
// When user_display_mode indicates a user preference for opening in
// a browser tab, we open in a browser tab.
EXPECT_EQ(DisplayMode::kBrowser,
ResolveEffectiveDisplayMode(DisplayMode::kBrowser,
DisplayMode::kBrowser));
EXPECT_EQ(DisplayMode::kBrowser,
ResolveEffectiveDisplayMode(DisplayMode::kMinimalUi,
DisplayMode::kBrowser));
EXPECT_EQ(DisplayMode::kBrowser,
ResolveEffectiveDisplayMode(DisplayMode::kStandalone,
DisplayMode::kBrowser));
EXPECT_EQ(DisplayMode::kBrowser,
ResolveEffectiveDisplayMode(DisplayMode::kFullscreen,
DisplayMode::kBrowser));
// When user_display_mode indicates a user preference for opening in
// a standalone window, we open in a minimal-ui window (for app_display_mode
// 'browser' or 'minimal-ui') or a standalone window (for app_display_mode
// 'standalone' or 'fullscreen').
EXPECT_EQ(DisplayMode::kMinimalUi,
ResolveEffectiveDisplayMode(DisplayMode::kBrowser,
DisplayMode::kStandalone));
EXPECT_EQ(DisplayMode::kMinimalUi,
ResolveEffectiveDisplayMode(DisplayMode::kMinimalUi,
DisplayMode::kStandalone));
EXPECT_EQ(DisplayMode::kStandalone,
ResolveEffectiveDisplayMode(DisplayMode::kStandalone,
DisplayMode::kStandalone));
EXPECT_EQ(DisplayMode::kStandalone,
ResolveEffectiveDisplayMode(DisplayMode::kFullscreen,
DisplayMode::kStandalone));
}
} // namespace web_app
......@@ -169,6 +169,13 @@ blink::mojom::DisplayMode BookmarkAppRegistrar::GetAppUserDisplayMode(
}
}
blink::mojom::DisplayMode BookmarkAppRegistrar::GetAppEffectiveDisplayMode(
const web_app::AppId& app_id) const {
// TODO(crbug.com/1014346): Consider app's requested display mode. Support
// minimal-ui.
return GetAppUserDisplayMode(app_id);
}
std::vector<web_app::AppId> BookmarkAppRegistrar::GetAppIds() const {
std::vector<web_app::AppId> app_ids;
for (scoped_refptr<const Extension> app :
......
......@@ -40,6 +40,8 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar,
base::Optional<GURL> GetAppScope(const web_app::AppId& app_id) const override;
blink::mojom::DisplayMode GetAppUserDisplayMode(
const web_app::AppId& app_id) const override;
blink::mojom::DisplayMode GetAppEffectiveDisplayMode(
const web_app::AppId& app_id) const override;
std::vector<web_app::AppId> GetAppIds() const override;
// ExtensionRegistryObserver:
......
......@@ -136,6 +136,12 @@ blink::mojom::DisplayMode TestAppRegistrar::GetAppUserDisplayMode(
return blink::mojom::DisplayMode::kBrowser;
}
blink::mojom::DisplayMode TestAppRegistrar::GetAppEffectiveDisplayMode(
const AppId& app_id) const {
NOTIMPLEMENTED();
return blink::mojom::DisplayMode::kBrowser;
}
std::vector<AppId> TestAppRegistrar::GetAppIds() const {
std::vector<AppId> result;
for (const std::pair<AppId, AppInfo>& it : installed_apps_) {
......
......@@ -58,7 +58,9 @@ class TestAppRegistrar : public AppRegistrar {
const GURL& GetAppLaunchURL(const AppId& app_id) const override;
base::Optional<GURL> GetAppScope(const AppId& app_id) const override;
blink::mojom::DisplayMode GetAppUserDisplayMode(
const web_app::AppId& app_id) const override;
const AppId& app_id) const override;
blink::mojom::DisplayMode GetAppEffectiveDisplayMode(
const AppId& app_id) const override;
std::vector<AppId> GetAppIds() const override;
private:
......
......@@ -11,6 +11,7 @@
#include "base/bind.h"
#include "base/logging.h"
#include "base/strings/string_util.h"
#include "chrome/browser/web_applications/components/web_app_constants.h"
#include "chrome/browser/web_applications/web_app.h"
#include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
......@@ -118,12 +119,21 @@ base::Optional<GURL> WebAppRegistrar::GetAppScope(const AppId& app_id) const {
}
blink::mojom::DisplayMode WebAppRegistrar::GetAppUserDisplayMode(
const web_app::AppId& app_id) const {
const AppId& app_id) const {
auto* web_app = GetAppById(app_id);
return web_app ? web_app->user_display_mode()
: blink::mojom::DisplayMode::kUndefined;
}
blink::mojom::DisplayMode WebAppRegistrar::GetAppEffectiveDisplayMode(
const AppId& app_id) const {
auto* web_app = GetAppById(app_id);
return web_app ? ResolveEffectiveDisplayMode(
/*app_display_mode=*/web_app->display_mode(),
/*user_display_mode=*/web_app->user_display_mode())
: blink::mojom::DisplayMode::kUndefined;
}
std::vector<AppId> WebAppRegistrar::GetAppIds() const {
std::vector<AppId> app_ids;
app_ids.reserve(registry_.size());
......
......@@ -44,7 +44,9 @@ class WebAppRegistrar : public AppRegistrar {
const GURL& GetAppLaunchURL(const AppId& app_id) const override;
base::Optional<GURL> GetAppScope(const AppId& app_id) const override;
blink::mojom::DisplayMode GetAppUserDisplayMode(
const web_app::AppId& app_id) const override;
const AppId& app_id) const override;
blink::mojom::DisplayMode GetAppEffectiveDisplayMode(
const AppId& app_id) const override;
std::vector<AppId> GetAppIds() const override;
// Only range-based |for| loop supported. Don't use AppSet directly.
......
......@@ -1283,6 +1283,7 @@ if (!is_android) {
"../browser/ui/views/hats/hats_browsertest.cc",
"../browser/ui/views/intent_picker_bubble_view_browsertest.cc",
"../browser/ui/views/try_chrome_dialog_win/try_chrome_dialog_browsertest.cc",
"../browser/ui/views/web_apps/web_app_minimal_ui_test.cc",
"../browser/ui/views/webauthn/authenticator_dialog_view_browsertest.cc",
"../browser/ui/views/webauthn/authenticator_qr_code_test.cc",
"../browser/ui/views/webview_accessibility_browsertest.cc",
......
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