diff options
-rw-r--r-- | chrome/browser/android/banners/app_banner_manager.cc | 21 | ||||
-rw-r--r-- | chrome/browser/android/banners/app_banner_manager.h | 6 | ||||
-rw-r--r-- | chrome/browser/android/banners/app_banner_manager_unittest.cc | 103 | ||||
-rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 |
4 files changed, 126 insertions, 5 deletions
diff --git a/chrome/browser/android/banners/app_banner_manager.cc b/chrome/browser/android/banners/app_banner_manager.cc index f428abb..0283d2e 100644 --- a/chrome/browser/android/banners/app_banner_manager.cc +++ b/chrome/browser/android/banners/app_banner_manager.cc @@ -54,7 +54,7 @@ const int kIconMinimumSize = 144; // The requirement for now is an image/png that is at least 144x144. bool DoesManifestContainRequiredIcon(const content::Manifest& manifest) { for (const auto& icon : manifest.icons) { - if (EqualsASCII(icon.type.string(), "image/png")) + if (!EqualsASCII(icon.type.string(), "image/png")) continue; for (const auto& size : icon.sizes) { @@ -166,14 +166,25 @@ void AppBannerManager::DidFinishLoad( weak_factory_.GetWeakPtr())); } +// static +bool AppBannerManager::IsManifestValid(const content::Manifest& manifest) { + if (manifest.IsEmpty()) + return false; + if (!manifest.start_url.is_valid()) + return false; + if (manifest.name.is_null() && manifest.short_name.is_null()) + return false; + if (!DoesManifestContainRequiredIcon(manifest)) + return false; + + return true; +} + void AppBannerManager::OnDidGetManifest(const content::Manifest& manifest) { if (web_contents()->IsBeingDestroyed()) return; - if (manifest.IsEmpty() - || !manifest.start_url.is_valid() - || (manifest.name.is_null() && manifest.short_name.is_null()) - || !DoesManifestContainRequiredIcon(manifest)) { + if (!IsManifestValid(manifest)) { // No usable manifest, see if there is a play store meta tag. if (!IsEnabledForNativeApps()) return; diff --git a/chrome/browser/android/banners/app_banner_manager.h b/chrome/browser/android/banners/app_banner_manager.h index d4f7c26..78aa3d3 100644 --- a/chrome/browser/android/banners/app_banner_manager.h +++ b/chrome/browser/android/banners/app_banner_manager.h @@ -87,6 +87,12 @@ class AppBannerManager : public content::WebContentsObserver { bool OnMessageReceived(const IPC::Message& message) override; private: + friend class AppBannerManagerTest; + + // Returns whether the given Manifest is following the requirements to show + // a web app banner. + static bool IsManifestValid(const content::Manifest& manifest); + // Called when the manifest has been retrieved, or if there is no manifest to // retrieve. void OnDidGetManifest(const content::Manifest& manifest); diff --git a/chrome/browser/android/banners/app_banner_manager_unittest.cc b/chrome/browser/android/banners/app_banner_manager_unittest.cc new file mode 100644 index 0000000..93b94ea --- /dev/null +++ b/chrome/browser/android/banners/app_banner_manager_unittest.cc @@ -0,0 +1,103 @@ +// Copyright 2015 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/android/banners/app_banner_manager.h" + +#include "base/strings/utf_string_conversions.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace banners { + +class AppBannerManagerTest : public testing::Test { + protected: + static base::NullableString16 ToNullableUTF16(const std::string& str) { + return base::NullableString16(base::UTF8ToUTF16(str), false); + } + + static content::Manifest GetValidManifest() { + content::Manifest manifest; + manifest.name = ToNullableUTF16("foo"); + manifest.short_name = ToNullableUTF16("bar"); + manifest.start_url = GURL("http://example.com"); + + content::Manifest::Icon icon; + icon.type = ToNullableUTF16("image/png"); + icon.sizes.push_back(gfx::Size(144, 144)); + manifest.icons.push_back(icon); + + return manifest; + } + static bool IsManifestValid(const content::Manifest& manifest) { + return AppBannerManager::IsManifestValid(manifest); + } +}; + +TEST_F(AppBannerManagerTest, EmptyManifestIsInvalid) { + content::Manifest manifest; + EXPECT_FALSE(IsManifestValid(manifest)); +} + +TEST_F(AppBannerManagerTest, CheckMinimalValidManifest) { + content::Manifest manifest = GetValidManifest(); + EXPECT_TRUE(IsManifestValid(manifest)); +} + +TEST_F(AppBannerManagerTest, ManifestRequiresNameORShortName) { + content::Manifest manifest = GetValidManifest(); + + manifest.name = base::NullableString16(); + EXPECT_TRUE(IsManifestValid(manifest)); + + manifest.name = ToNullableUTF16("foo"); + manifest.short_name = base::NullableString16(); + EXPECT_TRUE(IsManifestValid(manifest)); + + manifest.name = base::NullableString16(); + EXPECT_FALSE(IsManifestValid(manifest)); +} + +TEST_F(AppBannerManagerTest, ManifestRequiresValidStartURL) { + content::Manifest manifest = GetValidManifest(); + + manifest.start_url = GURL(); + EXPECT_FALSE(IsManifestValid(manifest)); + + manifest.start_url = GURL("/"); + EXPECT_FALSE(IsManifestValid(manifest)); +} + +TEST_F(AppBannerManagerTest, ManifestRequiresImagePNG) { + content::Manifest manifest = GetValidManifest(); + + manifest.icons[0].type = ToNullableUTF16("image/gif"); + EXPECT_FALSE(IsManifestValid(manifest)); + manifest.icons[0].type = base::NullableString16(); + EXPECT_FALSE(IsManifestValid(manifest)); +} + +TEST_F(AppBannerManagerTest, ManifestRequiresMinimalSize) { + content::Manifest manifest = GetValidManifest(); + + // The icon MUST be 144x144 size at least. + manifest.icons[0].sizes[0] = gfx::Size(1, 1); + EXPECT_FALSE(IsManifestValid(manifest)); + + // If one of the sizes match the requirement, it should be accepted. + manifest.icons[0].sizes.push_back(gfx::Size(144, 144)); + EXPECT_TRUE(IsManifestValid(manifest)); + + // Higher than the required size is okay. + manifest.icons[0].sizes[1] = gfx::Size(200, 200); + EXPECT_TRUE(IsManifestValid(manifest)); + + // Non-square is okay. + manifest.icons[0].sizes[1] = gfx::Size(144, 200); + EXPECT_TRUE(IsManifestValid(manifest)); + + // The representation of the keyword 'any' should be recognized. + manifest.icons[0].sizes[1] = gfx::Size(0, 0); + EXPECT_TRUE(IsManifestValid(manifest)); +} + +} // namespace banners diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index c368c5c..e49743e 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -14,6 +14,7 @@ '../tools/metrics/histograms/histograms.xml', # All unittests in browser, common, renderer and service. 'browser/about_flags_unittest.cc', + 'browser/android/banners/app_banner_manager_unittest.cc', 'browser/android/bookmarks/partner_bookmarks_shim_unittest.cc', 'browser/android/manifest_icon_selector_unittest.cc', # TODO(newt): move this to test_support_unit? |