[ios] Improvements in BackgroundRefresh code
Main changes in this CL:
- Add initial AppRefreshProvider test class (to be improved)
- Remove _refreshInterval from app_refresh_provider.mm because it's
already defined in the header file
- Move private methods in background_refresh_app_agent.mm under
pragma mark
Bug: none
Change-Id: I39647bb7bd0a7f5ba00f90439e9690e9e4bda6d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5743299
Reviewed-by: Mark Cogan <marq@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Mark Cogan <marq@chromium.org>
Auto-Submit: Federica Germinario <fedegermi@google.com>
Cr-Commit-Position: refs/heads/main@{#1334307}
diff --git a/ios/chrome/app/background_refresh/BUILD.gn b/ios/chrome/app/background_refresh/BUILD.gn
index 2f09121..1e105ac 100644
--- a/ios/chrome/app/background_refresh/BUILD.gn
+++ b/ios/chrome/app/background_refresh/BUILD.gn
@@ -30,3 +30,13 @@
"UserNotifications.framework",
]
}
+
+source_set("unit_tests") {
+ testonly = true
+ sources = [ "app_refresh_provider_unittest.mm" ]
+ deps = [
+ ":background_refresh",
+ "//base/test:test_support",
+ "//testing/gtest",
+ ]
+}
diff --git a/ios/chrome/app/background_refresh/app_refresh_provider.h b/ios/chrome/app/background_refresh/app_refresh_provider.h
index 5fbdf91..7b70c17 100644
--- a/ios/chrome/app/background_refresh/app_refresh_provider.h
+++ b/ios/chrome/app/background_refresh/app_refresh_provider.h
@@ -21,7 +21,7 @@
// for this.
@property(nonatomic, readonly) NSString* identifier;
-// Refresh interval for this provider. Default is one hour.
+// Refresh interval for this provider. Default is 15 minutes.
@property(nonatomic, readonly) base::TimeDelta refreshInterval;
// Last *completed* run time for the provider's operations. Backed by a user
@@ -36,6 +36,7 @@
// superclass implementation.
- (void)handleRefreshWithCompletion:(ProceduralBlock)completion;
+// TODO(crbug.com/354918188): Implement cancellation.
// Terminate the running task immediately.
- (void)cancelRefresh;
diff --git a/ios/chrome/app/background_refresh/app_refresh_provider.mm b/ios/chrome/app/background_refresh/app_refresh_provider.mm
index f35b6e3..813470f 100644
--- a/ios/chrome/app/background_refresh/app_refresh_provider.mm
+++ b/ios/chrome/app/background_refresh/app_refresh_provider.mm
@@ -12,9 +12,7 @@
@end
-@implementation AppRefreshProvider {
- base::TimeDelta _refreshInterval;
-}
+@implementation AppRefreshProvider
- (instancetype)init {
if (self = [super init]) {
diff --git a/ios/chrome/app/background_refresh/app_refresh_provider_unittest.mm b/ios/chrome/app/background_refresh/app_refresh_provider_unittest.mm
new file mode 100644
index 0000000..8d4389c
--- /dev/null
+++ b/ios/chrome/app/background_refresh/app_refresh_provider_unittest.mm
@@ -0,0 +1,17 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/app/background_refresh/app_refresh_provider.h"
+
+#import "testing/gtest/include/gtest/gtest.h"
+#import "testing/platform_test.h"
+
+using AppRefreshProviderTest = PlatformTest;
+
+// Expect that refreshInterval has a default value when AppRefreshProvider is
+// created.
+TEST_F(AppRefreshProviderTest, VerifyInitializer) {
+ AppRefreshProvider* provider = [[AppRefreshProvider alloc] init];
+ EXPECT_EQ(provider.refreshInterval, base::Minutes(15));
+}
diff --git a/ios/chrome/app/background_refresh/background_refresh_app_agent.mm b/ios/chrome/app/background_refresh/background_refresh_app_agent.mm
index 51a86d85..2bb75f81 100644
--- a/ios/chrome/app/background_refresh/background_refresh_app_agent.mm
+++ b/ios/chrome/app/background_refresh/background_refresh_app_agent.mm
@@ -33,22 +33,10 @@
}
- (void)addAppRefreshProvider:(AppRefreshProvider*)provider {
+ CHECK(provider);
[self.providers addObject:provider];
}
-- (void)registerBackgroundRefreshTask {
- auto handler = ^(BGTask* task) {
- [self systemTriggeredRefreshForTask:task];
- };
-
- // TODO(crbug.com/354919106): Consider moving this task to a queue known to
- // Chromium, so it's easy to safely thread hop.
- [BGTaskScheduler.sharedScheduler
- registerForTaskWithIdentifier:kAppBackgroundRefreshTaskIdentifier
- usingQueue:nil
- launchHandler:handler];
-}
-
- (void)requestAppRefreshWithDelay:(NSTimeInterval)delay {
// Schedule requests only if flag is enabled.
if (!IsAppBackgroundRefreshEnabled()) {
@@ -88,6 +76,21 @@
}
}
+#pragma mark - Private
+
+- (void)registerBackgroundRefreshTask {
+ auto handler = ^(BGTask* task) {
+ [self systemTriggeredRefreshForTask:task];
+ };
+
+ // TODO(crbug.com/354919106): Consider moving this task to a queue known to
+ // Chromium, so it's easy to safely thread hop.
+ [BGTaskScheduler.sharedScheduler
+ registerForTaskWithIdentifier:kAppBackgroundRefreshTaskIdentifier
+ usingQueue:nil
+ launchHandler:handler];
+}
+
// Debugging note: To induce the scheduler to call this task, you should
// (1) Set a breakpoint sometime after `-registerBaskgroundRefreshTask` is
// called.
diff --git a/ios/chrome/test/BUILD.gn b/ios/chrome/test/BUILD.gn
index 5f69d43..5171255 100644
--- a/ios/chrome/test/BUILD.gn
+++ b/ios/chrome/test/BUILD.gn
@@ -198,6 +198,7 @@
# Add unit_tests target here.
"//ios/chrome/app:unit_tests",
"//ios/chrome/app/application_delegate:unit_tests",
+ "//ios/chrome/app/background_refresh:unit_tests",
"//ios/chrome/app/profile:unit_tests",
"//ios/chrome/app/spotlight:unit_tests",
"//ios/chrome/app/startup:unit_tests",