Migrate OpenInController to SimpleURLLoader
URLFetcher will stop working with advent of Network Service, and
SimpleURLLoader is the replacement API for most clients.
This CL migrates iOS' OpenInController.
Additionally, the CL also fixes the unittest TestDisplayOpenInMenu
that was flagged as DISABLED.
BUG=773295
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I130b80cc7854d95266fba45b94df0caa40f89eda
Reviewed-on: https://chromium-review.googlesource.com/1239373
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#593938}diff --git a/ios/chrome/browser/tabs/tab.mm b/ios/chrome/browser/tabs/tab.mm
index 129c6658..934e7a0 100644
--- a/ios/chrome/browser/tabs/tab.mm
+++ b/ios/chrome/browser/tabs/tab.mm
@@ -100,6 +100,7 @@
#include "net/cert/x509_certificate.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_fetcher.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/page_transition_types.h"
#include "url/origin.h"
@@ -462,8 +463,8 @@
- (OpenInController*)openInController {
if (!_openInController) {
_openInController = [[OpenInController alloc]
- initWithRequestContext:_browserState->GetRequestContext()
- webController:self.webController];
+ initWithURLLoaderFactory:_browserState->GetSharedURLLoaderFactory()
+ webController:self.webController];
_openInController.baseView = self.view;
}
return _openInController;
diff --git a/ios/chrome/browser/ui/BUILD.gn b/ios/chrome/browser/ui/BUILD.gn
index 12d09e83..8bc66db2 100644
--- a/ios/chrome/browser/ui/BUILD.gn
+++ b/ios/chrome/browser/ui/BUILD.gn
@@ -156,6 +156,7 @@
"//ios/web/public/test/fakes",
"//net",
"//net:test_support",
+ "//services/network:test_support",
"//testing/gmock",
"//testing/gtest",
"//third_party/ocmock",
diff --git a/ios/chrome/browser/ui/open_in_controller.h b/ios/chrome/browser/ui/open_in_controller.h
index 50c8517..fd89aac 100644
--- a/ios/chrome/browser/ui/open_in_controller.h
+++ b/ios/chrome/browser/ui/open_in_controller.h
@@ -13,8 +13,8 @@
#import "ios/chrome/browser/ui/open_in_toolbar.h"
#include "url/gurl.h"
-namespace net {
-class URLRequestContextGetter;
+namespace network {
+class SharedURLLoaderFactory;
}
@class CRWWebController;
@@ -23,8 +23,9 @@
@interface OpenInController : NSObject<UIGestureRecognizerDelegate,
UIDocumentInteractionControllerDelegate>
// Designated initializer.
-- (id)initWithRequestContext:(net::URLRequestContextGetter*)requestContext
- webController:(CRWWebController*)webController;
+- (id)initWithURLLoaderFactory:
+ (scoped_refptr<network::SharedURLLoaderFactory>)urlLoaderFactory
+ webController:(CRWWebController*)webController;
// Base view on which the Open In toolbar will be presented.
@property(nonatomic, weak) UIView* baseView;
diff --git a/ios/chrome/browser/ui/open_in_controller.mm b/ios/chrome/browser/ui/open_in_controller.mm
index db37595..6ecc569 100644
--- a/ios/chrome/browser/ui/open_in_controller.mm
+++ b/ios/chrome/browser/ui/open_in_controller.mm
@@ -25,9 +25,8 @@
#include "ios/web/public/web_thread.h"
#import "ios/web/web_state/ui/crw_web_controller.h"
#include "net/base/load_flags.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
#include "ui/base/l10n/l10n_util_mac.h"
#import "ui/gfx/ios/NSString+CrStringDrawing.h"
#include "url/gurl.h"
@@ -41,8 +40,6 @@
// other applications.
static NSString* const kDocumentsTempPath = @"OpenIn";
-static const int kHTTPResponseCodeSucceeded = 200;
-
// Duration of the show/hide animation for the |openInToolbar_|.
const NSTimeInterval kOpenInToolbarAnimationDuration = 0.2;
@@ -77,8 +74,8 @@
// updated. Used to know in which direction the scroll view is scrolling.
@property(nonatomic, assign) CGFloat previousScrollViewOffset;
-// URLFetcher delegate method called when |fetcher_| completes a request.
-- (void)urlFetchDidComplete:(const net::URLFetcher*)source;
+// SimpleURLLoader completion callback, when |urlLoader_| completes a request.
+- (void)urlLoadDidComplete:(const base::FilePath&)file_path;
// Ensures the destination directory is created and any contained obsolete files
// are deleted. Returns YES if the directory is created successfully.
+ (BOOL)createDestinationDirectoryAndRemoveObsoleteFiles;
@@ -120,16 +117,10 @@
// Bridge to deliver method calls from C++ to the |OpenInController| class.
class OpenInControllerBridge
- : public net::URLFetcherDelegate,
- public base::RefCountedThreadSafe<OpenInControllerBridge> {
+ : public base::RefCountedThreadSafe<OpenInControllerBridge> {
public:
explicit OpenInControllerBridge(OpenInController* owner) : owner_(owner) {}
- void OnURLFetchComplete(const net::URLFetcher* source) override {
- DCHECK(owner_);
- [owner_ urlFetchDidComplete:source];
- }
-
BOOL CreateDestinationDirectoryAndRemoveObsoleteFiles(void) {
return [OpenInController createDestinationDirectoryAndRemoveObsoleteFiles];
}
@@ -157,7 +148,7 @@
protected:
friend base::RefCountedThreadSafe<OpenInControllerBridge>;
- ~OpenInControllerBridge() override {}
+ virtual ~OpenInControllerBridge() {}
private:
__weak OpenInController* owner_;
@@ -185,15 +176,15 @@
// Suggested filename for the document.
NSString* suggestedFilename_;
- // Fetcher used to redownload the document and save it in the sandbox.
- std::unique_ptr<net::URLFetcher> fetcher_;
+ // Loader used to redownload the document and save it in the sandbox.
+ std::unique_ptr<network::SimpleURLLoader> urlLoader_;
// CRWWebController used to check if the tap is not on a link and the
// |openInToolbar_| should be displayed.
CRWWebController* webController_;
- // URLRequestContextGetter needed for the URLFetcher.
- scoped_refptr<net::URLRequestContextGetter> requestContext_;
+ // URLLoaderFactory instance needed for URLLoader.
+ scoped_refptr<network::SharedURLLoaderFactory> urlLoaderFactory_;
// Spinner view displayed while the file is downloading.
UIView* overlayedView_;
@@ -217,11 +208,12 @@
@synthesize baseView = _baseView;
@synthesize previousScrollViewOffset = _previousScrollViewOffset;
-- (id)initWithRequestContext:(net::URLRequestContextGetter*)requestContext
- webController:(CRWWebController*)webController {
+- (id)initWithURLLoaderFactory:
+ (scoped_refptr<network::SharedURLLoaderFactory>)urlLoaderFactory
+ webController:(CRWWebController*)webController {
self = [super init];
if (self) {
- requestContext_ = requestContext;
+ urlLoaderFactory_ = std::move(urlLoaderFactory);
webController_ = webController;
tapRecognizer_ = [[UITapGestureRecognizer alloc]
initWithTarget:self
@@ -261,7 +253,7 @@
[documentController_ setDelegate:nil];
documentURL_ = GURL();
suggestedFilename_ = nil;
- fetcher_.reset();
+ urlLoader_.reset();
}
- (void)detachFromWebController {
@@ -370,13 +362,17 @@
bridge_ = new OpenInControllerBridge(self);
// Download the document and save it at |filePath|.
- fetcher_ = net::URLFetcher::Create(0, documentURL_, net::URLFetcher::GET,
- bridge_.get());
- fetcher_->SetRequestContext(requestContext_.get());
- fetcher_->SetLoadFlags(net::LOAD_SKIP_CACHE_VALIDATION);
- fetcher_->SaveResponseToFileAtPath(
- base::FilePath(base::SysNSStringToUTF8(filePath)), sequencedTaskRunner_);
- fetcher_->Start();
+ auto resourceRequest = std::make_unique<network::ResourceRequest>();
+ resourceRequest->url = documentURL_;
+ resourceRequest->load_flags = net::LOAD_SKIP_CACHE_VALIDATION;
+
+ urlLoader_ = network::SimpleURLLoader::Create(std::move(resourceRequest),
+ NO_TRAFFIC_ANNOTATION_YET);
+ urlLoader_->DownloadToFile(urlLoaderFactory_.get(),
+ base::BindOnce(^(base::FilePath filePath) {
+ [self urlLoadDidComplete:filePath];
+ }),
+ base::FilePath(base::SysNSStringToUTF8(filePath)));
}
- (void)handleTapOnOverlayedView:(UIGestureRecognizer*)gestureRecognizer {
@@ -424,9 +420,9 @@
if (!webController_)
return;
- if (requestContext_.get()) {
- // |requestContext_| is nil only if this is called from a unit test, in
- // which case the |documentController_| was set already.
+ if (!documentController_) {
+ // If this is called from a unit test, |documentController_| was set
+ // already.
documentController_ =
[UIDocumentInteractionController interactionControllerWithURL:fileURL];
}
@@ -439,17 +435,15 @@
[documentController_ presentOpenInMenuFromRect:anchorLocation_
inView:[webController_ view]
animated:YES];
- if (requestContext_.get()) {
- [self removeOverlayedView];
- if (!success) {
- if (IsIPadIdiom())
- [self hideOpenInToolbar];
- NSString* errorMessage =
- l10n_util::GetNSStringWithFixup(IDS_IOS_OPEN_IN_NO_APPS_REGISTERED);
- [self showErrorWithMessage:errorMessage];
- } else {
- isOpenInMenuDisplayed_ = YES;
- }
+ [self removeOverlayedView];
+ if (!success) {
+ if (IsIPadIdiom())
+ [self hideOpenInToolbar];
+ NSString* errorMessage =
+ l10n_util::GetNSStringWithFixup(IDS_IOS_OPEN_IN_NO_APPS_REGISTERED);
+ [self showErrorWithMessage:errorMessage];
+ } else {
+ isOpenInMenuDisplayed_ = YES;
}
}
@@ -589,16 +583,8 @@
return YES;
}
-#pragma mark -
-#pragma mark URLFetcher delegate method
-
-- (void)urlFetchDidComplete:(const net::URLFetcher*)fetcher {
- DCHECK(fetcher);
- if (requestContext_.get())
- DCHECK_CURRENTLY_ON(web::WebThread::UI);
- base::FilePath filePath;
- if (fetcher->GetResponseCode() == kHTTPResponseCodeSucceeded &&
- fetcher->GetResponseAsFilePath(true, &filePath)) {
+- (void)urlLoadDidComplete:(const base::FilePath&)filePath {
+ if (!filePath.empty()) {
NSURL* fileURL =
[NSURL fileURLWithPath:base::SysUTF8ToNSString(filePath.value())];
if (downloadCanceled_) {
diff --git a/ios/chrome/browser/ui/open_in_controller_unittest.mm b/ios/chrome/browser/ui/open_in_controller_unittest.mm
index 77abe37..493acb7 100644
--- a/ios/chrome/browser/ui/open_in_controller_unittest.mm
+++ b/ios/chrome/browser/ui/open_in_controller_unittest.mm
@@ -6,8 +6,9 @@
#include <memory>
-#include "base/message_loop/message_loop.h"
+#include "base/files/file_util.h"
#include "base/strings/sys_string_conversions.h"
+#include "base/test/scoped_task_environment.h"
#import "ios/chrome/browser/tabs/tab.h"
#import "ios/chrome/browser/ui/open_in_controller.h"
#import "ios/chrome/browser/ui/open_in_controller_testing.h"
@@ -15,6 +16,8 @@
#import "ios/web/web_state/ui/crw_web_controller.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_fetcher_delegate.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
#include "testing/platform_test.h"
#import "third_party/ocmock/OCMock/OCMock.h"
#include "third_party/ocmock/gtest_support.h"
@@ -27,37 +30,48 @@
class OpenInControllerTest : public PlatformTest {
public:
- OpenInControllerTest() {
- io_thread_.reset(
- new web::TestWebThread(web::WebThread::IO, &message_loop_));
- }
+ OpenInControllerTest()
+ : test_shared_url_loader_factory_(
+ base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+ &test_url_loader_factory_)) {}
void TearDown() override { PlatformTest::TearDown(); }
void SetUp() override {
PlatformTest::SetUp();
+
+ // Set up the directory it downloads the file to.
+ // Note that the value of kDocumentsTempPath must match the one in
+ // open_in_controller.mm
+ {
+ NSString* const kDocumentsTempPath = @"OpenIn";
+ NSString* tempDirPath = [NSTemporaryDirectory()
+ stringByAppendingPathComponent:kDocumentsTempPath];
+ base::FilePath directory(base::SysNSStringToUTF8(tempDirPath));
+ EXPECT_TRUE(base::CreateDirectory(directory));
+ }
+
GURL documentURL = GURL("http://www.test.com/doc.pdf");
parent_view_ = [[UIView alloc] init];
id webController = [OCMockObject niceMockForClass:[CRWWebController class]];
- open_in_controller_ =
- [[OpenInController alloc] initWithRequestContext:nil
- webController:webController];
+ open_in_controller_ = [[OpenInController alloc]
+ initWithURLLoaderFactory:test_shared_url_loader_factory_
+ webController:webController];
[open_in_controller_ enableWithDocumentURL:documentURL
suggestedFilename:@"doc.pdf"];
}
- // |TestURLFetcher| requires a |MessageLoop|.
- base::MessageLoopForUI message_loop_;
- // Add |io_thread_| to release |URLRequestContextGetter| in
- // |URLFetcher::Core|.
- std::unique_ptr<web::TestWebThread> io_thread_;
- // Creates a |TestURLFetcherFactory|, which automatically sets itself as
- // |URLFetcher|'s factory.
+
+ base::test::ScopedTaskEnvironment scoped_task_environment_;
+ network::TestURLLoaderFactory test_url_loader_factory_;
+ scoped_refptr<network::SharedURLLoaderFactory>
+ test_shared_url_loader_factory_;
+
net::TestURLFetcherFactory factory_;
OpenInController* open_in_controller_;
UIView* parent_view_;
};
-TEST_F(OpenInControllerTest, DISABLED_TestDisplayOpenInMenu) {
+TEST_F(OpenInControllerTest, TestDisplayOpenInMenu) {
id documentController =
[OCMockObject niceMockForClass:[UIDocumentInteractionController class]];
[open_in_controller_ setDocumentInteractionController:documentController];
@@ -67,19 +81,20 @@
inView:OCMOCK_ANY
animated:YES];
- net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
- DCHECK(fetcher);
- DCHECK(fetcher->delegate());
- fetcher->set_response_code(200);
+ auto* pending_request = test_url_loader_factory_.GetPendingRequest(0);
+ DCHECK(pending_request);
// Set the response for the set URLFetcher to be a blank PDF.
NSMutableData* pdfData = [NSMutableData data];
UIGraphicsBeginPDFContextToData(pdfData, CGRectMake(0, 0, 100, 100), @{});
UIGraphicsBeginPDFPage();
UIGraphicsEndPDFContext();
unsigned char* array = (unsigned char*)[pdfData bytes];
- fetcher->SetResponseString(std::string((char*)array, sizeof(pdfData)));
- fetcher->SetResponseFilePath(base::FilePath("path/to/file.pdf"));
- fetcher->delegate()->OnURLFetchComplete(fetcher);
+
+ test_url_loader_factory_.SimulateResponseForPendingRequest(
+ pending_request->request.url.spec(),
+ std::string((char*)array, sizeof(pdfData)));
+ scoped_task_environment_.RunUntilIdle();
+
EXPECT_OCMOCK_VERIFY(documentController);
}
