Commit a24c33f2 authored by Mohammad Refaat's avatar Mohammad Refaat Committed by Commit Bot

Cleanup App launcher refresh flag

The feature was launched in stable for some time.

Bug: 850760, 774736
Change-Id: I763ddbd5a83d3686cb1cff7336041fd5c729847d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913183Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715150}
parent 151ccf79
......@@ -98,7 +98,6 @@ source_set("browser") {
"//components/webdata_services",
"//google_apis",
"//ios/chrome/app/strings",
"//ios/chrome/browser/app_launcher:feature_flags",
"//ios/chrome/browser/browser_state",
"//ios/chrome/browser/complex_tasks",
"//ios/chrome/browser/crash_report:flags",
......
......@@ -16,7 +16,6 @@ source_set("app_launcher") {
"app_launching_state.mm",
]
deps = [
":feature_flags",
"//base",
"//components/reading_list/core:core",
"//ios/chrome/browser",
......@@ -32,17 +31,6 @@ source_set("app_launcher") {
]
}
source_set("feature_flags") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"app_launcher_flags.h",
"app_launcher_flags.mm",
]
deps = [
"//base",
]
}
source_set("unit_tests") {
configs += [ "//build/config/compiler:enable_arc" ]
testonly = true
......@@ -53,7 +41,6 @@ source_set("unit_tests") {
]
deps = [
":app_launcher",
":feature_flags",
"//base",
"//base/test:test_support",
"//components/reading_list/core:core",
......
// Copyright 2018 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.
#ifndef IOS_CHROME_BROWSER_APP_LAUNCHER_APP_LAUNCHER_FLAGS_H_
#define IOS_CHROME_BROWSER_APP_LAUNCHER_APP_LAUNCHER_FLAGS_H_
#include "base/feature_list.h"
// Feature flag to enable the new app launcher logic.
extern const base::Feature kAppLauncherRefresh;
#endif // IOS_CHROME_BROWSER_APP_LAUNCHER_APP_LAUNCHER_FLAGS_H_
// Copyright 2018 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 "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
const base::Feature kAppLauncherRefresh{"AppLauncherRefresh",
base::FEATURE_ENABLED_BY_DEFAULT};
......@@ -11,7 +11,6 @@
#import "base/strings/sys_string_conversions.h"
#include "components/reading_list/core/reading_list_model.h"
#import "ios/chrome/browser/app_launcher/app_launcher_abuse_detector.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#import "ios/chrome/browser/app_launcher/app_launcher_tab_helper_delegate.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/chrome_url_util.h"
......@@ -222,7 +221,6 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
ios::ChromeBrowserState* browser_state =
ios::ChromeBrowserState::FromBrowserState(web_state_->GetBrowserState());
if (base::FeatureList::IsEnabled(kAppLauncherRefresh)) {
if (!is_link_transition && original_pending_url.is_valid()) {
// At this stage the navigation will be canceled in all cases. If this
// was a redirection, the |source_url| may not have been reported to
......@@ -236,24 +234,6 @@ bool AppLauncherTabHelper::ShouldAllowRequest(
RequestToLaunchApp(request_url, last_committed_url, is_link_transition);
}
return false;
}
if (RequestToLaunchApp(request_url, last_committed_url, is_link_transition)) {
// Clears pending navigation history after successfully launching the
// external app.
web_state_->GetNavigationManager()->DiscardNonCommittedItems();
// When opening applications, the navigation is cancelled. Report the
// opening of the application to the ReadingListWebStateObserver to mark the
// entry as read if needed.
if (original_pending_url.is_valid()) {
ReadingListModel* model =
ReadingListModelFactory::GetForBrowserState(browser_state);
if (model && model->loaded())
model->SetReadStatus(original_pending_url, true);
}
}
return false;
}
WEB_STATE_USER_DATA_KEY_IMPL(AppLauncherTabHelper)
......@@ -8,13 +8,11 @@
#include "base/bind.h"
#include "base/compiler_specific.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "base/time/default_clock.h"
#include "components/reading_list/core/reading_list_entry.h"
#include "components/reading_list/core/reading_list_model_impl.h"
#import "ios/chrome/browser/app_launcher/app_launcher_abuse_detector.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#import "ios/chrome/browser/app_launcher/app_launcher_tab_helper_delegate.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/chrome_url_util.h"
......@@ -363,38 +361,10 @@ TEST_F(AppLauncherTabHelperTest, ChromeBundleUrlScheme) {
EXPECT_EQ(1U, delegate_.countOfAppsLaunched);
}
// Tests that ShouldAllowRequest updates the reading list correctly, when there
// is a valid app URL to be launches successfully.
// TODO(crbug.com/850760): Remove this test, once the new AppLauncherRefresh
// logic is always enabled.
TEST_F(AppLauncherTabHelperTest, UpdatingTheReadingList) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(kAppLauncherRefresh);
// Reading list isn't expected to be updated if there was no app launch.
EXPECT_TRUE(TestReadingListUpdate(/*is_app_blocked=*/true,
/*is_link_transition*/ false,
/*expected_read_status*/ false));
EXPECT_EQ(0U, delegate_.countOfAppsLaunched);
// Reading list to be updated when app launch is successful.
EXPECT_TRUE(TestReadingListUpdate(/*is_app_blocked=*/false,
/*is_link_transition*/ false,
/*expected_read_status*/ true));
EXPECT_EQ(1U, delegate_.countOfAppsLaunched);
// Transition type doesn't affect the reading list status
EXPECT_TRUE(TestReadingListUpdate(/*is_app_blocked=*/false,
/*is_link_transition*/ true,
/*expected_read_status*/ true));
EXPECT_EQ(2U, delegate_.countOfAppsLaunched);
}
// Tests that ShouldAllowRequest updates the reading list correctly for non-link
// transitions regardless of the app launching success when AppLauncherRefresh
// flag is enabled.
TEST_F(AppLauncherTabHelperTest, UpdatingTheReadingListWithAppLauncherRefresh) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kAppLauncherRefresh);
TEST_F(AppLauncherTabHelperTest, UpdatingTheReadingList) {
// Update reading list if the transition is not a link transition.
EXPECT_TRUE(TestReadingListUpdate(/*is_app_blocked=*/true,
/*is_link_transition*/ false,
......
......@@ -39,7 +39,6 @@ source_set("flags") {
"//components/variations",
"//ios/chrome/app/strings:ios_strings",
"//ios/chrome/browser",
"//ios/chrome/browser/app_launcher:feature_flags",
"//ios/chrome/browser/browsing_data:feature_flags",
"//ios/chrome/browser/crash_report:flags",
"//ios/chrome/browser/crash_report/breadcrumbs:feature_flags",
......
......@@ -48,7 +48,6 @@
#include "components/sync/driver/sync_driver_switches.h"
#include "components/translate/core/browser/translate_prefs.h"
#include "components/ukm/ios/features.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#include "ios/chrome/browser/browsing_data/browsing_data_features.h"
#include "ios/chrome/browser/chrome_switches.h"
#include "ios/chrome/browser/crash_report/breadcrumbs/features.h"
......@@ -363,9 +362,6 @@ const flags_ui::FeatureEntry kFeatureEntries[] = {
flag_descriptions::kWebPageTextAccessibilityName,
flag_descriptions::kWebPageTextAccessibilityDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(web::kWebPageTextAccessibility)},
{"app-launcher-refresh", flag_descriptions::kAppLauncherRefreshName,
flag_descriptions::kAppLauncherRefreshDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(kAppLauncherRefresh)},
{"toolbar-container", flag_descriptions::kToolbarContainerName,
flag_descriptions::kToolbarContainerDescription, flags_ui::kOsIos,
FEATURE_VALUE_TYPE(toolbar_container::kToolbarContainerEnabled)},
......
......@@ -11,12 +11,6 @@
namespace flag_descriptions {
const char kAppLauncherRefreshName[] = "Enable the new AppLauncher logic";
const char kAppLauncherRefreshDescription[] =
"AppLauncher will always prompt if there is no direct link navigation, "
"also Apps will launch asynchronously and there will be no logic that"
"depends on the success or the failure of launching an app.";
const char kAutofillCacheQueryResponsesName[] =
"Cache Autofill Query Responses";
const char kAutofillCacheQueryResponsesDescription[] =
......
......@@ -9,10 +9,6 @@
namespace flag_descriptions {
// Title and description for the flag to control the new app launcher.
extern const char kAppLauncherRefreshName[];
extern const char kAppLauncherRefreshDescription[];
// Title and description for the flag to control the autofill query cache.
extern const char kAutofillCacheQueryResponsesName[];
extern const char kAutofillCacheQueryResponsesDescription[];
......
......@@ -18,7 +18,6 @@ source_set("app_launcher") {
"//ios/chrome/app/strings:ios_strings_grit",
"//ios/chrome/browser",
"//ios/chrome/browser/app_launcher",
"//ios/chrome/browser/app_launcher:feature_flags",
"//ios/chrome/browser/overlays",
"//ios/chrome/browser/overlays/public/web_content_area",
"//ios/chrome/browser/ui/dialogs:feature_flags",
......@@ -42,7 +41,6 @@ source_set("unit_tests") {
"//base/test:test_support",
"//ios/chrome/app/strings:ios_strings_grit",
"//ios/chrome/browser/app_launcher",
"//ios/chrome/browser/app_launcher:feature_flags",
"//ios/chrome/test:test_support",
"//ios/web/public/test/fakes",
"//testing/gtest",
......
......@@ -10,7 +10,6 @@
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#import "ios/chrome/browser/app_launcher/app_launcher_tab_helper.h"
#import "ios/chrome/browser/overlays/public/overlay_request.h"
#import "ios/chrome/browser/overlays/public/overlay_request_queue.h"
......@@ -166,7 +165,6 @@ void AppLauncherOverlayCallback(ProceduralBlockWithBool app_launch_completion,
return YES;
}
if (base::FeatureList::IsEnabled(kAppLauncherRefresh)) {
// For all other apps other than AppStore, show a prompt if there was no
// link transition.
if (linkTransition) {
......@@ -177,17 +175,6 @@ void AppLauncherOverlayCallback(ProceduralBlockWithBool app_launch_completion,
[self showAlertAndLaunchAppURL:URL webState:webState];
}
return YES;
}
// If the following call returns YES, an application is about to be
// launched and Chrome will go into the background now.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
// TODO(crbug.com/774736): This method needs to be converted to an
// asynchronous call so that the call below can be replaced with
// |openURL:options:completionHandler:|.
return [[UIApplication sharedApplication] openURL:net::NSURLWithGURL(URL)];
#pragma clang diagnostic pop
}
- (void)appLauncherTabHelper:(AppLauncherTabHelper*)tabHelper
......
......@@ -7,8 +7,6 @@
#import <UIKit/UIKit.h>
#include "base/mac/foundation_util.h"
#include "base/test/scoped_feature_list.h"
#include "ios/chrome/browser/app_launcher/app_launcher_flags.h"
#import "ios/chrome/browser/app_launcher/app_launcher_tab_helper.h"
#include "ios/chrome/grit/ios_strings.h"
#import "ios/chrome/test/scoped_key_window.h"
......@@ -65,27 +63,9 @@ TEST_F(AppLauncherCoordinatorTest, ItmsUrlShowsAlert) {
alert_controller.message);
}
// Tests that an app URL attempts to launch the application.
TEST_F(AppLauncherCoordinatorTest, AppUrlLaunchesApp) {
// Make sure that the new AppLauncherRefresh logic is disabled.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(kAppLauncherRefresh);
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
OCMExpect([application_ openURL:[NSURL URLWithString:@"some-app://1234"]]);
#pragma clang diagnostic pop
[coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("some-app://1234")
linkTransition:NO];
[application_ verify];
}
// Tests that in the new AppLauncher, an app URL attempts to launch the
// application.
TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlLaunchesApp) {
// Make sure that the new AppLauncherRefresh logic is enabled.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kAppLauncherRefresh);
TEST_F(AppLauncherCoordinatorTest, AppUrlLaunchesApp) {
OCMExpect([application_ openURL:[NSURL URLWithString:@"some-app://1234"]
options:@{}
completionHandler:nil]);
......@@ -97,10 +77,7 @@ TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlLaunchesApp) {
// Tests that in the new AppLauncher, an app URL shows a prompt if there was no
// link transition.
TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlShowsPrompt) {
// Make sure that the new AppLauncherRefresh logic is enabled.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kAppLauncherRefresh);
TEST_F(AppLauncherCoordinatorTest, AppUrlShowsPrompt) {
[coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("some-app://1234")
linkTransition:NO];
......@@ -112,23 +89,3 @@ TEST_F(AppLauncherCoordinatorTest, AppLauncherRefreshAppUrlShowsPrompt) {
EXPECT_NSEQ(l10n_util::GetNSString(IDS_IOS_OPEN_IN_ANOTHER_APP),
alert_controller.message);
}
// Tests that |-appLauncherTabHelper:launchAppWithURL:linkTransition:| returns
// NO if there is no application that corresponds to a given URL.
TEST_F(AppLauncherCoordinatorTest, NoApplicationForUrl) {
// Make sure that the new AppLauncherRefresh logic is disabled.
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(kAppLauncherRefresh);
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
OCMStub(
[application_ openURL:[NSURL URLWithString:@"no-app-installed://1234"]])
.andReturn(NO);
#pragma clang diagnostic pop
BOOL app_exists =
[coordinator_ appLauncherTabHelper:tab_helper()
launchAppWithURL:GURL("no-app-installed://1234")
linkTransition:NO];
EXPECT_FALSE(app_exists);
}
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