Commit 10f4fffc authored by Mark Cogan's avatar Mark Cogan Committed by Commit Bot

[iOS] Modernise QuickLook coordinator

This CL modernizes the ARQuickLookCoordinator to use the superclass
-initWithBaseViewController:browser: initializer, and to remove the
spurious injection of BrowserState and WebStateList.

It also updates the coordinator's unit test, using the new TestBrowser()
constructor.

Bug: 1029346
Change-Id: I29f6e14409dc66b34da36126d0ea7e516323ddaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1943240
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721514}
parent 12c2303f
...@@ -287,8 +287,7 @@ ...@@ -287,8 +287,7 @@
self.ARQuickLookCoordinator = [[ARQuickLookCoordinator alloc] self.ARQuickLookCoordinator = [[ARQuickLookCoordinator alloc]
initWithBaseViewController:self.viewController initWithBaseViewController:self.viewController
browserState:self.browserState browser:self.browser];
webStateList:self.browser->GetWebStateList()];
[self.ARQuickLookCoordinator start]; [self.ARQuickLookCoordinator start];
self.injectionHandler = [[ManualFillInjectionHandler alloc] self.injectionHandler = [[ManualFillInjectionHandler alloc]
......
...@@ -37,6 +37,7 @@ source_set("download") { ...@@ -37,6 +37,7 @@ source_set("download") {
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/download", "//ios/chrome/browser/download",
"//ios/chrome/browser/infobars", "//ios/chrome/browser/infobars",
"//ios/chrome/browser/main",
"//ios/chrome/browser/store_kit", "//ios/chrome/browser/store_kit",
"//ios/chrome/browser/ui/alert_coordinator", "//ios/chrome/browser/ui/alert_coordinator",
"//ios/chrome/browser/ui/colors", "//ios/chrome/browser/ui/colors",
...@@ -81,6 +82,7 @@ source_set("unit_tests") { ...@@ -81,6 +82,7 @@ source_set("unit_tests") {
"//ios/chrome/browser/download", "//ios/chrome/browser/download",
"//ios/chrome/browser/download:test_support", "//ios/chrome/browser/download:test_support",
"//ios/chrome/browser/infobars", "//ios/chrome/browser/infobars",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/ui/util", "//ios/chrome/browser/ui/util",
"//ios/chrome/browser/web:test_support", "//ios/chrome/browser/web:test_support",
"//ios/chrome/browser/web_state_list:test_support", "//ios/chrome/browser/web_state_list:test_support",
......
...@@ -27,19 +27,10 @@ enum class PresentQLPreviewController { ...@@ -27,19 +27,10 @@ enum class PresentQLPreviewController {
kMaxValue = kAnotherViewControllerIsPresented, kMaxValue = kAnotherViewControllerIsPresented,
}; };
class WebStateList;
// Presents QLPreviewController in order to display USDZ format 3D models. // Presents QLPreviewController in order to display USDZ format 3D models.
@interface ARQuickLookCoordinator : ChromeCoordinator @interface ARQuickLookCoordinator : ChromeCoordinator
// Creates a coordinator that uses a |viewController| a |browserState| and // Unavailable, use -initWithBaseViewController:browser:
// a |webStateList|.
- (instancetype)initWithBaseViewController:(UIViewController*)viewController
browserState:
(ios::ChromeBrowserState*)browserState
webStateList:(WebStateList*)webStateList;
// Unavailable, use -initWithBaseViewController:browserState:webStateList:.
- (instancetype)initWithBaseViewController:(UIViewController*)viewController - (instancetype)initWithBaseViewController:(UIViewController*)viewController
browserState: browserState:
(ios::ChromeBrowserState*)browserState (ios::ChromeBrowserState*)browserState
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#import "ios/chrome/browser/download/ar_quick_look_tab_helper.h" #import "ios/chrome/browser/download/ar_quick_look_tab_helper.h"
#import "ios/chrome/browser/download/ar_quick_look_tab_helper_delegate.h" #import "ios/chrome/browser/download/ar_quick_look_tab_helper_delegate.h"
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h" #import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
...@@ -58,7 +59,7 @@ PresentQLPreviewController GetHistogramEnum( ...@@ -58,7 +59,7 @@ PresentQLPreviewController GetHistogramEnum(
} }
// The WebStateList being observed. // The WebStateList being observed.
@property(nonatomic, assign) WebStateList* webStateList; @property(nonatomic, readonly) WebStateList* webStateList;
// Whether the coordinator is started. // Whether the coordinator is started.
@property(nonatomic, assign) BOOL started; @property(nonatomic, assign) BOOL started;
...@@ -74,17 +75,8 @@ PresentQLPreviewController GetHistogramEnum( ...@@ -74,17 +75,8 @@ PresentQLPreviewController GetHistogramEnum(
@implementation ARQuickLookCoordinator @implementation ARQuickLookCoordinator
- (instancetype)initWithBaseViewController:(UIViewController*)viewController - (WebStateList*)webStateList {
browserState: return self.browser->GetWebStateList();
(ios::ChromeBrowserState*)browserState
webStateList:(WebStateList*)webStateList {
DCHECK(webStateList);
self = [super initWithBaseViewController:viewController
browserState:browserState];
if (self) {
_webStateList = webStateList;
}
return self;
} }
- (void)start { - (void)start {
......
...@@ -14,9 +14,11 @@ ...@@ -14,9 +14,11 @@
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#import "base/test/ios/wait_util.h" #import "base/test/ios/wait_util.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/task_environment.h"
#import "ios/chrome/browser/download/ar_quick_look_tab_helper.h" #import "ios/chrome/browser/download/ar_quick_look_tab_helper.h"
#import "ios/chrome/browser/download/ar_quick_look_tab_helper_delegate.h" #import "ios/chrome/browser/download/ar_quick_look_tab_helper_delegate.h"
#include "ios/chrome/browser/download/download_test_util.h" #include "ios/chrome/browser/download/download_test_util.h"
#import "ios/chrome/browser/main/test_browser.h"
#include "ios/chrome/browser/ui/util/ui_util.h" #include "ios/chrome/browser/ui/util/ui_util.h"
#import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h" #import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_list.h"
...@@ -49,12 +51,10 @@ class ARQuickLookCoordinatorTest : public PlatformTest { ...@@ -49,12 +51,10 @@ class ARQuickLookCoordinatorTest : public PlatformTest {
protected: protected:
ARQuickLookCoordinatorTest() ARQuickLookCoordinatorTest()
: base_view_controller_([[UIViewController alloc] init]), : base_view_controller_([[UIViewController alloc] init]),
web_state_list_( browser_(std::make_unique<TestBrowser>()),
std::make_unique<WebStateList>(&web_state_list_delegate_)),
coordinator_([[ARQuickLookCoordinator alloc] coordinator_([[ARQuickLookCoordinator alloc]
initWithBaseViewController:base_view_controller_ initWithBaseViewController:base_view_controller_
browserState:nullptr browser:browser_.get()]) {
webStateList:web_state_list_.get()]) {
[scoped_key_window_.Get() setRootViewController:base_view_controller_]; [scoped_key_window_.Get() setRootViewController:base_view_controller_];
// The Coordinator should install itself as delegate for the existing // The Coordinator should install itself as delegate for the existing
...@@ -62,7 +62,7 @@ class ARQuickLookCoordinatorTest : public PlatformTest { ...@@ -62,7 +62,7 @@ class ARQuickLookCoordinatorTest : public PlatformTest {
auto web_state = std::make_unique<web::TestWebState>(); auto web_state = std::make_unique<web::TestWebState>();
auto* web_state_ptr = web_state.get(); auto* web_state_ptr = web_state.get();
ARQuickLookTabHelper::CreateForWebState(web_state_ptr); ARQuickLookTabHelper::CreateForWebState(web_state_ptr);
web_state_list_->InsertWebState(0, std::move(web_state), browser_->GetWebStateList()->InsertWebState(0, std::move(web_state),
WebStateList::INSERT_NO_FLAGS, WebStateList::INSERT_NO_FLAGS,
WebStateOpener()); WebStateOpener());
[coordinator_ start]; [coordinator_ start];
...@@ -72,12 +72,14 @@ class ARQuickLookCoordinatorTest : public PlatformTest { ...@@ -72,12 +72,14 @@ class ARQuickLookCoordinatorTest : public PlatformTest {
ARQuickLookTabHelper* tab_helper() { ARQuickLookTabHelper* tab_helper() {
return ARQuickLookTabHelper::FromWebState( return ARQuickLookTabHelper::FromWebState(
web_state_list_->GetWebStateAt(0)); browser_->GetWebStateList()->GetWebStateAt(0));
} }
// Needed for test browser state created by TestBrowser().
base::test::TaskEnvironment task_environment_;
UIViewController* base_view_controller_; UIViewController* base_view_controller_;
FakeWebStateListDelegate web_state_list_delegate_; std::unique_ptr<Browser> browser_;
std::unique_ptr<WebStateList> web_state_list_;
ARQuickLookCoordinator* coordinator_; ARQuickLookCoordinator* coordinator_;
ScopedKeyWindow scoped_key_window_; ScopedKeyWindow scoped_key_window_;
base::HistogramTester histogram_tester_; base::HistogramTester histogram_tester_;
...@@ -86,19 +88,12 @@ class ARQuickLookCoordinatorTest : public PlatformTest { ...@@ -86,19 +88,12 @@ class ARQuickLookCoordinatorTest : public PlatformTest {
// Tests that the coordinator installs itself as a ARQuickLookTabHelper // Tests that the coordinator installs itself as a ARQuickLookTabHelper
// delegate when ARQuickLookTabHelper instances become available. // delegate when ARQuickLookTabHelper instances become available.
TEST_F(ARQuickLookCoordinatorTest, InstallDelegates) { TEST_F(ARQuickLookCoordinatorTest, InstallDelegates) {
WebStateList web_state_list(&web_state_list_delegate_);
ARQuickLookCoordinator* coordinator = [[ARQuickLookCoordinator alloc]
initWithBaseViewController:base_view_controller_
browserState:nullptr
webStateList:&web_state_list];
[coordinator start];
// Coordinator should install itself as delegate for a new web state. // Coordinator should install itself as delegate for a new web state.
auto web_state2 = std::make_unique<web::TestWebState>(); auto web_state2 = std::make_unique<web::TestWebState>();
auto* web_state_ptr2 = web_state2.get(); auto* web_state_ptr2 = web_state2.get();
ARQuickLookTabHelper::CreateForWebState(web_state_ptr2); ARQuickLookTabHelper::CreateForWebState(web_state_ptr2);
EXPECT_FALSE(ARQuickLookTabHelper::FromWebState(web_state_ptr2)->delegate()); EXPECT_FALSE(ARQuickLookTabHelper::FromWebState(web_state_ptr2)->delegate());
web_state_list.InsertWebState(0, std::move(web_state2), browser_->GetWebStateList()->InsertWebState(0, std::move(web_state2),
WebStateList::INSERT_NO_FLAGS, WebStateList::INSERT_NO_FLAGS,
WebStateOpener()); WebStateOpener());
EXPECT_TRUE(ARQuickLookTabHelper::FromWebState(web_state_ptr2)->delegate()); EXPECT_TRUE(ARQuickLookTabHelper::FromWebState(web_state_ptr2)->delegate());
...@@ -109,10 +104,8 @@ TEST_F(ARQuickLookCoordinatorTest, InstallDelegates) { ...@@ -109,10 +104,8 @@ TEST_F(ARQuickLookCoordinatorTest, InstallDelegates) {
auto* web_state_ptr3 = web_state3.get(); auto* web_state_ptr3 = web_state3.get();
ARQuickLookTabHelper::CreateForWebState(web_state_ptr3); ARQuickLookTabHelper::CreateForWebState(web_state_ptr3);
EXPECT_FALSE(ARQuickLookTabHelper::FromWebState(web_state_ptr3)->delegate()); EXPECT_FALSE(ARQuickLookTabHelper::FromWebState(web_state_ptr3)->delegate());
web_state_list.ReplaceWebStateAt(0, std::move(web_state3)); browser_->GetWebStateList()->ReplaceWebStateAt(0, std::move(web_state3));
EXPECT_TRUE(ARQuickLookTabHelper::FromWebState(web_state_ptr3)->delegate()); EXPECT_TRUE(ARQuickLookTabHelper::FromWebState(web_state_ptr3)->delegate());
[coordinator stop];
} }
// Tests presenting a valid USDZ file. // Tests presenting a valid USDZ file.
......
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