Skip to content

GitLab

T
tangled
Commit fe8ac1d7 authored by Giovanni Ortuño Urquidi's avatar Giovanni Ortuño Urquidi Committed by Chromium LUCI CQ
Browse files
Options

system-web-apps: Remove SetManifest() for system web app tests 

There are three related changes:

1. Change ManifestUpdateManagerSystemAppBrowserTest.CheckUpdateSkipped
   to closer match how installs are done for System Web Apps i.e.
   we no longer use a manifest for the initial install.
2. Remove set_manifest() which the test used to change the manifest of
   the SWA
3. Remove a couple of other unnecessary methods now that we removed
   set_manifest().

We don't completely remove the manifest because a test still uses it.

Bug: 1158686
Change-Id: Ib9ed2f81c0b3ba877f2bc74151fc0dd9c4c1c291
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641786
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: default avatarJiewei Qian  <qjw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845970}
parent bb3460ad
Hide whitespace changes
Inline Side-by-side
Showing with 24 additions and 73 deletions
chrome/browser/web_applications/manifest_update_manager_browsertest.cc
View file @ fe8ac1d7
...@@ -1114,26 +1114,9 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest, ...@@ -1114,26 +1114,9 @@ IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerBrowserTest,
class ManifestUpdateManagerSystemAppBrowserTest class ManifestUpdateManagerSystemAppBrowserTest
: public ManifestUpdateManagerBrowserTest { : public ManifestUpdateManagerBrowserTest {
public: public:
static constexpr char kSystemAppManifestText[] =
R"({
"name": "Test System App",
"display": "standalone",
"icons": [
{
"src": "icon-256.png",
"sizes": "256x256",
"type": "image/png"
}
],
"start_url": "/pwa.html",
"theme_color": "$1"
})";
ManifestUpdateManagerSystemAppBrowserTest() ManifestUpdateManagerSystemAppBrowserTest()
: system_app_( : system_app_(
TestSystemWebAppInstallation::SetUpStandaloneSingleWindowApp()) { TestSystemWebAppInstallation::SetUpStandaloneSingleWindowApp()) {
system_app_->SetManifest(base::ReplaceStringPlaceholders(
kSystemAppManifestText, {"#0f0"}, nullptr));
} }
void SetUpOnMainThread() override { system_app_->WaitForAppInstall(); } void SetUpOnMainThread() override { system_app_->WaitForAppInstall(); }
...@@ -1142,14 +1125,8 @@ class ManifestUpdateManagerSystemAppBrowserTest ...@@ -1142,14 +1125,8 @@ class ManifestUpdateManagerSystemAppBrowserTest
std::unique_ptr<TestSystemWebAppInstallation> system_app_; std::unique_ptr<TestSystemWebAppInstallation> system_app_;
}; };
constexpr char
ManifestUpdateManagerSystemAppBrowserTest::kSystemAppManifestText[];
IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerSystemAppBrowserTest, IN_PROC_BROWSER_TEST_F(ManifestUpdateManagerSystemAppBrowserTest,
CheckUpdateSkipped) { CheckUpdateSkipped) {
system_app_->SetManifest(base::ReplaceStringPlaceholders(
kSystemAppManifestText, {"#f00"}, nullptr));
AppId app_id = system_app_->GetAppId(); AppId app_id = system_app_->GetAppId();
EXPECT_EQ(GetResultAfterPageLoad(system_app_->GetAppUrl(), &app_id), EXPECT_EQ(GetResultAfterPageLoad(system_app_->GetAppUrl(), &app_id),
ManifestUpdateResult::kAppIsSystemWebApp); ManifestUpdateResult::kAppIsSystemWebApp);
......
chrome/browser/web_applications/test/test_system_web_app_installation.cc
View file @ fe8ac1d7
...@@ -376,10 +376,6 @@ SystemAppType TestSystemWebAppInstallation::GetType() { ...@@ -376,10 +376,6 @@ SystemAppType TestSystemWebAppInstallation::GetType() {
return type_.value(); return type_.value();
} }
void TestSystemWebAppInstallation::SetManifest(std::string manifest) {
web_ui_controller_factories_[0]->set_manifest(std::move(manifest));
}
void TestSystemWebAppInstallation::RegisterAutoGrantedPermissions( void TestSystemWebAppInstallation::RegisterAutoGrantedPermissions(
ContentSettingsType permission) { ContentSettingsType permission) {
auto_granted_permissions_.insert(permission); auto_granted_permissions_.insert(permission);
......
chrome/browser/web_applications/test/test_system_web_app_installation.h
View file @ fe8ac1d7
...@@ -69,9 +69,6 @@ class TestSystemWebAppInstallation { ...@@ -69,9 +69,6 @@ class TestSystemWebAppInstallation {
const GURL& GetAppUrl(); const GURL& GetAppUrl();
SystemAppType GetType(); SystemAppType GetType();
// Override the contents served by chrome://test-system-app/manifest.json.
void SetManifest(std::string manifest);
void set_update_policy(SystemWebAppManager::UpdatePolicy update_policy) { void set_update_policy(SystemWebAppManager::UpdatePolicy update_policy) {
update_policy_ = update_policy; update_policy_ = update_policy;
} }
......
chrome/browser/web_applications/test/test_system_web_app_url_data_source.cc
View file @ fe8ac1d7
...@@ -16,6 +16,21 @@ namespace web_app { ...@@ -16,6 +16,21 @@ namespace web_app {
namespace { namespace {
constexpr char kManifestText[] =
R"({
"name": "Test System App",
"display": "standalone",
"icons": [
{
"src": "icon-256.png",
"sizes": "256x256",
"type": "image/png"
}
],
"start_url": "/pwa.html",
"theme_color": "#00FF00"
})";
constexpr char kPwaHtml[] = constexpr char kPwaHtml[] =
R"( R"(
<html> <html>
...@@ -39,13 +54,6 @@ constexpr char kSwJs[] = "globalThis.addEventListener('fetch', event => {});"; ...@@ -39,13 +54,6 @@ constexpr char kSwJs[] = "globalThis.addEventListener('fetch', event => {});";
void AddTestURLDataSource(const std::string& source_name, void AddTestURLDataSource(const std::string& source_name,
content::BrowserContext* browser_context) { content::BrowserContext* browser_context) {
static std::string manifest(kSystemAppManifestText);
AddTestURLDataSource(source_name, &manifest, browser_context);
}
void AddTestURLDataSource(const std::string& source_name,
const std::string* manifest_text,
content::BrowserContext* browser_context) {
content::WebUIDataSource* data_source = content::WebUIDataSource* data_source =
content::WebUIDataSource::Create(source_name); content::WebUIDataSource::Create(source_name);
data_source->DisableTrustedTypesCSP(); data_source->DisableTrustedTypesCSP();
...@@ -56,12 +64,12 @@ void AddTestURLDataSource(const std::string& source_name, ...@@ -56,12 +64,12 @@ void AddTestURLDataSource(const std::string& source_name,
path == "page2.html"; path == "page2.html";
}), }),
base::BindLambdaForTesting( base::BindLambdaForTesting(
[manifest_text](const std::string& id, [](const std::string& id,
content::WebUIDataSource::GotDataCallback callback) { content::WebUIDataSource::GotDataCallback callback) {
scoped_refptr<base::RefCountedString> ref_contents( scoped_refptr<base::RefCountedString> ref_contents(
new base::RefCountedString); new base::RefCountedString);
if (id == "manifest.json") if (id == "manifest.json")
ref_contents->data() = *manifest_text; ref_contents->data() = kManifestText;
else if (id == "pwa.html") else if (id == "pwa.html")
ref_contents->data() = kPwaHtml; ref_contents->data() = kPwaHtml;
else if (id == "sw.js") else if (id == "sw.js")
......
chrome/browser/web_applications/test/test_system_web_app_url_data_source.h
View file @ fe8ac1d7
...@@ -13,30 +13,9 @@ class BrowserContext; ...@@ -13,30 +13,9 @@ class BrowserContext;
namespace web_app { namespace web_app {
constexpr char kSystemAppManifestText[] =
R"({
"name": "Test System App",
"display": "standalone",
"icons": [
{
"src": "icon-256.png",
"sizes": "256x256",
"type": "image/png"
}
],
"start_url": "/pwa.html",
"theme_color": "#00FF00"
})";
// Creates and registers a URLDataSource that serves a blank page with
// a |kSystemAppManifestText| manifest.
void AddTestURLDataSource(const std::string& source_name,
content::BrowserContext* browser_context);
// Creates and registers a URLDataSource that serves a blank page with // Creates and registers a URLDataSource that serves a blank page with
// a |manifest_text| manifest. // a manifest.
void AddTestURLDataSource(const std::string& source_name, void AddTestURLDataSource(const std::string& source_name,
const std::string* manifest_text,
content::BrowserContext* browser_context); content::BrowserContext* browser_context);
} // namespace web_app } // namespace web_app
......
chrome/browser/web_applications/test/test_system_web_app_web_ui_controller_factory.cc
View file @ fe8ac1d7
...@@ -17,11 +17,10 @@ namespace { ...@@ -17,11 +17,10 @@ namespace {
class TestSystemWebAppWebUIController : public content::WebUIController { class TestSystemWebAppWebUIController : public content::WebUIController {
public: public:
explicit TestSystemWebAppWebUIController(std::string source_name, explicit TestSystemWebAppWebUIController(std::string source_name,
const std::string* manifest,
content::WebUI* web_ui) content::WebUI* web_ui)
: WebUIController(web_ui) { : WebUIController(web_ui) {
web_app::AddTestURLDataSource( web_app::AddTestURLDataSource(
source_name, manifest, web_ui->GetWebContents()->GetBrowserContext()); source_name, web_ui->GetWebContents()->GetBrowserContext());
} }
TestSystemWebAppWebUIController(const TestSystemWebAppWebUIController&) = TestSystemWebAppWebUIController(const TestSystemWebAppWebUIController&) =
delete; delete;
...@@ -33,8 +32,7 @@ class TestSystemWebAppWebUIController : public content::WebUIController { ...@@ -33,8 +32,7 @@ class TestSystemWebAppWebUIController : public content::WebUIController {
TestSystemWebAppWebUIControllerFactory::TestSystemWebAppWebUIControllerFactory( TestSystemWebAppWebUIControllerFactory::TestSystemWebAppWebUIControllerFactory(
std::string source_name) std::string source_name)
: source_name_(std::move(source_name)), : source_name_(std::move(source_name)) {}
manifest_(web_app::kSystemAppManifestText) {}
std::unique_ptr<content::WebUIController> std::unique_ptr<content::WebUIController>
TestSystemWebAppWebUIControllerFactory::CreateWebUIControllerForURL( TestSystemWebAppWebUIControllerFactory::CreateWebUIControllerForURL(
...@@ -46,7 +44,7 @@ TestSystemWebAppWebUIControllerFactory::CreateWebUIControllerForURL( ...@@ -46,7 +44,7 @@ TestSystemWebAppWebUIControllerFactory::CreateWebUIControllerForURL(
} }
return std::make_unique<TestSystemWebAppWebUIController>(source_name_, return std::make_unique<TestSystemWebAppWebUIController>(source_name_,
&manifest_, web_ui); web_ui);
} }
content::WebUI::TypeID TestSystemWebAppWebUIControllerFactory::GetWebUIType( content::WebUI::TypeID TestSystemWebAppWebUIControllerFactory::GetWebUIType(
......
chrome/browser/web_applications/test/test_system_web_app_web_ui_controller_factory.h
View file @ fe8ac1d7
...@@ -14,9 +14,8 @@ ...@@ -14,9 +14,8 @@
#include "content/public/browser/web_ui_controller_factory.h" #include "content/public/browser/web_ui_controller_factory.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
// WebUIControllerFactory that creates a TestWebUIController, which serves the // WebUIControllerFactory that creates a TestWebUIController, which serves a
// resources needed for a minimal System Web App (a page with a web manifest and // page with a web manifest and an icon.
// an icon).
class TestSystemWebAppWebUIControllerFactory class TestSystemWebAppWebUIControllerFactory
: public content::WebUIControllerFactory { : public content::WebUIControllerFactory {
public: public:
...@@ -26,8 +25,6 @@ class TestSystemWebAppWebUIControllerFactory ...@@ -26,8 +25,6 @@ class TestSystemWebAppWebUIControllerFactory
TestSystemWebAppWebUIControllerFactory& operator=( TestSystemWebAppWebUIControllerFactory& operator=(
const TestSystemWebAppWebUIControllerFactory&) = delete; const TestSystemWebAppWebUIControllerFactory&) = delete;
void set_manifest(std::string manifest) { manifest_ = std::move(manifest); }
// content::WebUIControllerFactory // content::WebUIControllerFactory
std::unique_ptr<content::WebUIController> CreateWebUIControllerForURL( std::unique_ptr<content::WebUIController> CreateWebUIControllerForURL(
content::WebUI* web_ui, content::WebUI* web_ui,
...@@ -41,7 +38,6 @@ class TestSystemWebAppWebUIControllerFactory ...@@ -41,7 +38,6 @@ class TestSystemWebAppWebUIControllerFactory
private: private:
std::string source_name_; std::string source_name_;
std::string manifest_;
}; };
#endif // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_SYSTEM_WEB_APP_WEB_UI_CONTROLLER_FACTORY_H_ #endif // CHROME_BROWSER_WEB_APPLICATIONS_TEST_TEST_SYSTEM_WEB_APP_WEB_UI_CONTROLLER_FACTORY_H_
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