[ios] Creates and uses HistoryConsumer protocol.

- Moves out the HistoryService and Driver creation from HistoryCollectionVC to HistoryPanelVC.

Because of this HistoryCollectionVC will have less model responsibilities. On the new implementation
this responsibilities will be owned by a Coordinator instead of a parent VC like HistoryPanel.

Bug: 805190
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I749be91308ed35eb8d08a2a9f6c4bf984eb8bb79
Reviewed-on: https://chromium-review.googlesource.com/991253
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547908}

ios/chrome/browser/ui/history/BUILD.gn

@@ -91,6 +91,7 @@ source_set("history") {
 source_set("history_ui") {
   configs += [ "//build/config/compiler:enable_arc" ]
   sources = [
+    "history_consumer.h",
     "history_entries_status_item.h",
     "history_entries_status_item.mm",
     "history_entries_status_item_delegate.h",
@@ -140,6 +141,7 @@ source_set("unit_tests") {
     ":resources_unit_tests",
     "//base",
     "//base/test:test_support",
+    "//components/browser_sync",
     "//components/favicon/core",
     "//components/favicon/core/test:test_support",
     "//components/favicon_base",

ios/chrome/browser/ui/history/history_collection_view_controller_unittest.mm

@@ -10,8 +10,11 @@
 #include "base/strings/string16.h"
 #import "base/test/ios/wait_util.h"
 #include "base/time/time.h"
+#include "components/browser_sync/profile_sync_service.h"
 #include "components/history/core/browser/browsing_history_service.h"
+#include "components/keyed_service/core/service_access_type.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#include "ios/chrome/browser/history/history_service_factory.h"
 #include "ios/chrome/browser/signin/authentication_service_factory.h"
 #include "ios/chrome/browser/signin/authentication_service_fake.h"
 #include "ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h"
@@ -20,6 +23,7 @@
 #include "ios/chrome/browser/sync/sync_setup_service_factory.h"
 #include "ios/chrome/browser/sync/sync_setup_service_mock.h"
 #import "ios/chrome/browser/ui/history/ios_browsing_history_driver.h"
+#include "ios/chrome/browser/ui/history/ios_browsing_history_driver.h"
 #import "ios/chrome/browser/ui/url_loader.h"
 #include "ios/chrome/test/block_cleanup_test.h"
 #include "ios/web/public/test/test_web_thread.h"
@@ -65,8 +69,7 @@ std::unique_ptr<KeyedService> BuildMockSyncSetupService(
 
 }  // namespace
 
-@interface LegacyHistoryCollectionViewController (
-    Testing)<BrowsingHistoryDriverDelegate>
+@interface LegacyHistoryCollectionViewController (Testing)<HistoryConsumer>
 - (void)didPressClearBrowsingBar;
 @end
 
@@ -96,6 +99,19 @@ class LegacyHistoryCollectionViewControllerTest : public BlockCleanupTest {
             initWithLoader:mock_url_loader_
               browserState:mock_browser_state_.get()
                   delegate:mock_delegate_];
+
+    _browsingHistoryDriver = std::make_unique<IOSBrowsingHistoryDriver>(
+        mock_browser_state_.get(), history_collection_view_controller_);
+
+    _browsingHistoryService = std::make_unique<history::BrowsingHistoryService>(
+        _browsingHistoryDriver.get(),
+        ios::HistoryServiceFactory::GetForBrowserState(
+            mock_browser_state_.get(), ServiceAccessType::EXPLICIT_ACCESS),
+        IOSChromeProfileSyncServiceFactory::GetForBrowserState(
+            mock_browser_state_.get()));
+
+    history_collection_view_controller_.historyService =
+        _browsingHistoryService.get();
   }
 
   void TearDown() override {
@@ -109,9 +125,9 @@ class LegacyHistoryCollectionViewControllerTest : public BlockCleanupTest {
     BrowsingHistoryService::QueryResultsInfo query_results_info;
     query_results_info.reached_beginning = true;
     [history_collection_view_controller_
-        onQueryCompleteWithResults:results
-                  queryResultsInfo:query_results_info
-               continuationClosure:base::OnceClosure()];
+        historyQueryWasCompletedWithResults:results
+                           queryResultsInfo:query_results_info
+                        continuationClosure:base::OnceClosure()];
   }
 
  protected:
@@ -123,6 +139,8 @@ class LegacyHistoryCollectionViewControllerTest : public BlockCleanupTest {
   bool privacy_settings_opened_;
   SyncSetupServiceMock* sync_setup_service_mock_;
   DISALLOW_COPY_AND_ASSIGN(LegacyHistoryCollectionViewControllerTest);
+  std::unique_ptr<IOSBrowsingHistoryDriver> _browsingHistoryDriver;
+  std::unique_ptr<history::BrowsingHistoryService> _browsingHistoryService;
 };
 
 // Tests that isEmpty property returns NO after entries have been received.

ios/chrome/browser/ui/history/history_consumer.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.
+
+#ifndef IOS_CHROME_BROWSER_UI_HISTORY_HISTORY_CONSUMER_H_
+#define IOS_CHROME_BROWSER_UI_HISTORY_HISTORY_CONSUMER_H_
+
+#include <vector>
+
+#include "components/history/core/browser/browsing_history_service.h"
+
+// Defines methods to manage history query results and deletion actions.
+@protocol HistoryConsumer
+
+// Tells the consumer that the result of a history query has been retrieved.
+// Entries in |result| are already sorted.
+- (void)
+historyQueryWasCompletedWithResults:
+    (const std::vector<history::BrowsingHistoryService::HistoryEntry>&)results
+                   queryResultsInfo:(const history::BrowsingHistoryService::
+                                         QueryResultsInfo&)queryResultsInfo
+                continuationClosure:(base::OnceClosure)continuationClosure;
+
+// Tells the consumer that history entries have been deleted by a different
+// client.
+- (void)historyWasDeleted;
+
+// Tells the consumer whether to show notice about other forms of
+// browsing history or not.
+- (void)showNoticeAboutOtherFormsOfBrowsingHistory:(BOOL)shouldShowNotice;
+@end
+
+#endif  // IOS_CHROME_BROWSER_UI_HISTORY_HISTORY_CONSUMER_H_

ios/chrome/browser/ui/history/history_panel_view_controller.mm

@@ -4,14 +4,22 @@
 
 #import "ios/chrome/browser/ui/history/history_panel_view_controller.h"
 
+#include <memory>
+
 #include "base/ios/block_types.h"
 #include "base/ios/ios_util.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
+#include "components/browser_sync/profile_sync_service.h"
+#include "components/history/core/browser/browsing_history_service.h"
+#include "components/keyed_service/core/service_access_type.h"
 #include "components/strings/grit/components_strings.h"
+#include "ios/chrome/browser/history/history_service_factory.h"
+#include "ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h"
 #import "ios/chrome/browser/ui/commands/application_commands.h"
 #import "ios/chrome/browser/ui/history/clear_browsing_bar.h"
 #import "ios/chrome/browser/ui/history/history_search_view_controller.h"
+#include "ios/chrome/browser/ui/history/ios_browsing_history_driver.h"
 #import "ios/chrome/browser/ui/history/legacy_history_collection_view_controller.h"
 #import "ios/chrome/browser/ui/icons/chrome_icon.h"
 #import "ios/chrome/browser/ui/keyboard/UIKeyCommand+Chrome.h"
@@ -57,6 +65,10 @@ CGFloat kShadowOpacity = 0.2f;
   UIBarButtonItem* _rightBarButtonItem;
   // YES if NSLayoutConstraits were added.
   BOOL _addedConstraints;
+  // Provides dependencies and funnels callbacks from BrowsingHistoryService.
+  std::unique_ptr<IOSBrowsingHistoryDriver> _browsingHistoryDriver;
+  // Abstraction to communicate with HistoryService and WebHistoryService.
+  std::unique_ptr<history::BrowsingHistoryService> _browsingHistoryService;
 }
 // Closes history.
 - (void)closeHistory;
@@ -99,6 +111,18 @@ CGFloat kShadowOpacity = 0.2f;
             initWithLoader:loader
               browserState:browserState
                   delegate:self];
+
+    _browsingHistoryDriver = std::make_unique<IOSBrowsingHistoryDriver>(
+        browserState, _historyCollectionController);
+
+    _browsingHistoryService = std::make_unique<history::BrowsingHistoryService>(
+        _browsingHistoryDriver.get(),
+        ios::HistoryServiceFactory::GetForBrowserState(
+            browserState, ServiceAccessType::EXPLICIT_ACCESS),
+        IOSChromeProfileSyncServiceFactory::GetForBrowserState(browserState));
+
+    _historyCollectionController.historyService = _browsingHistoryService.get();
+
     _dispatcher = dispatcher;
 
     // Configure modal presentation.

ios/chrome/browser/ui/history/ios_browsing_history_driver.h

@@ -21,35 +21,14 @@ namespace ios {
 class ChromeBrowserState;
 }
 
-// Defines methods to manage history query results and deletion actions.
-@protocol BrowsingHistoryDriverDelegate<NSObject>
-
-// Notifies the delegate that the result of a history query has been retrieved.
-// Entries in |result| are already sorted.
-- (void)onQueryCompleteWithResults:
-            (const std::vector<history::BrowsingHistoryService::HistoryEntry>&)
-                results
-                  queryResultsInfo:
-                      (const history::BrowsingHistoryService::QueryResultsInfo&)
-                          queryResultsInfo
-               continuationClosure:(base::OnceClosure)continuationClosure;
-
-// Notifies the delegate that history entries have been deleted by a different
-// client and that the UI should be updated.
-- (void)didObserverHistoryDeletion;
-
-// Indicates to the delegate whether to show notice about other forms of
-// browsing history.
-- (void)shouldShowNoticeAboutOtherFormsOfBrowsingHistory:(BOOL)shouldShowNotice;
-
-@end
+@protocol HistoryConsumer;
 
 // A simple implementation of BrowsingHistoryServiceHandler that delegates to
-// objective-c object BrowsingHistoryDriverDelegate for most actions.
+// objective-c object HistoryConsumer for most actions.
 class IOSBrowsingHistoryDriver : public history::BrowsingHistoryDriver {
  public:
   IOSBrowsingHistoryDriver(ios::ChromeBrowserState* browser_state,
-                           id<BrowsingHistoryDriverDelegate> delegate);
+                           id<HistoryConsumer> consumer);
   ~IOSBrowsingHistoryDriver() override;
 
  private:
@@ -77,8 +56,8 @@ class IOSBrowsingHistoryDriver : public history::BrowsingHistoryDriver {
   // The current browser state.
   ios::ChromeBrowserState* browser_state_;  // weak
 
-  // Delegate for IOSBrowsingHistoryDriver. Serves as client for HistoryService.
-  __weak id<BrowsingHistoryDriverDelegate> delegate_;
+  // Consumer for IOSBrowsingHistoryDriver. Serves as client for HistoryService.
+  __weak id<HistoryConsumer> consumer_;
 
   DISALLOW_COPY_AND_ASSIGN(IOSBrowsingHistoryDriver);
 };

ios/chrome/browser/ui/history/ios_browsing_history_driver.mm

@@ -13,6 +13,7 @@
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/history/history_utils.h"
 #include "ios/chrome/browser/history/web_history_service_factory.h"
+#include "ios/chrome/browser/ui/history/history_consumer.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -24,13 +25,13 @@ using history::BrowsingHistoryService;
 
 IOSBrowsingHistoryDriver::IOSBrowsingHistoryDriver(
     ios::ChromeBrowserState* browser_state,
-    id<BrowsingHistoryDriverDelegate> delegate)
-    : browser_state_(browser_state), delegate_(delegate) {
+    id<HistoryConsumer> consumer)
+    : browser_state_(browser_state), consumer_(consumer) {
   DCHECK(browser_state_);
 }
 
 IOSBrowsingHistoryDriver::~IOSBrowsingHistoryDriver() {
-  delegate_ = nil;
+  consumer_ = nil;
 }
 
 #pragma mark - Private methods
@@ -39,9 +40,10 @@ void IOSBrowsingHistoryDriver::OnQueryComplete(
     const std::vector<BrowsingHistoryService::HistoryEntry>& results,
     const BrowsingHistoryService::QueryResultsInfo& query_results_info,
     base::OnceClosure continuation_closure) {
-  [delegate_ onQueryCompleteWithResults:results
-                       queryResultsInfo:query_results_info
-                    continuationClosure:std::move(continuation_closure)];
+  [consumer_
+      historyQueryWasCompletedWithResults:results
+                         queryResultsInfo:query_results_info
+                      continuationClosure:std::move(continuation_closure)];
 }
 
 void IOSBrowsingHistoryDriver::OnRemoveVisitsComplete() {
@@ -58,13 +60,13 @@ void IOSBrowsingHistoryDriver::OnRemoveVisits(
 }
 
 void IOSBrowsingHistoryDriver::HistoryDeleted() {
-  [delegate_ didObserverHistoryDeletion];
+  [consumer_ historyWasDeleted];
 }
 
 void IOSBrowsingHistoryDriver::HasOtherFormsOfBrowsingHistory(
     bool has_other_forms,
     bool has_synced_results) {
-  [delegate_ shouldShowNoticeAboutOtherFormsOfBrowsingHistory:has_other_forms];
+  [consumer_ showNoticeAboutOtherFormsOfBrowsingHistory:has_other_forms];
 }
 
 bool IOSBrowsingHistoryDriver::AllowHistoryDeletions() {

ios/chrome/browser/ui/history/legacy_history_collection_view_controller.h

@@ -8,6 +8,7 @@
 #import "ios/chrome/browser/ui/collection_view/collection_view_controller.h"
 
 #include "base/ios/block_types.h"
+#include "ios/chrome/browser/ui/history/history_consumer.h"
 
 namespace ios {
 class ChromeBrowserState;
@@ -35,7 +36,8 @@ class ChromeBrowserState;
 @end
 
 // View controller for displaying a collection of history entries.
-@interface LegacyHistoryCollectionViewController : CollectionViewController
+@interface LegacyHistoryCollectionViewController
+    : CollectionViewController<HistoryConsumer>
 // YES if the collection view is in editing mode. Setting |editing| turns
 // editing mode on or off accordingly.
 @property(nonatomic, assign, getter=isEditing) BOOL editing;
@@ -45,6 +47,9 @@ class ChromeBrowserState;
 @property(nonatomic, assign, readonly, getter=isEmpty) BOOL empty;
 // YES if the collection view has selected entries while in editing mode.
 @property(nonatomic, assign, readonly) BOOL hasSelectedEntries;
+// Abstraction to communicate with HistoryService and WebHistoryService.
+// Not owned by LegacyHistoryCollectionViewController.
+@property(nonatomic, assign) history::BrowsingHistoryService* historyService;
 
 - (instancetype)
 initWithLoader:(id<UrlLoader>)loader

ios/chrome/browser/ui/history/legacy_history_collection_view_controller.mm

@@ -14,20 +14,14 @@
 #include "base/metrics/user_metrics_action.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "components/browser_sync/profile_sync_service.h"
 #include "components/browsing_data/core/history_notice_utils.h"
-#include "components/history/core/browser/browsing_history_driver.h"
-#include "components/history/core/browser/browsing_history_service.h"
-#include "components/keyed_service/core/service_access_type.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/url_formatter.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
-#include "ios/chrome/browser/history/history_service_factory.h"
 #import "ios/chrome/browser/metrics/new_tab_page_uma.h"
 #import "ios/chrome/browser/signin/authentication_service.h"
 #include "ios/chrome/browser/signin/authentication_service_factory.h"
-#include "ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory.h"
 #include "ios/chrome/browser/sync/sync_setup_service.h"
 #include "ios/chrome/browser/sync/sync_setup_service_factory.h"
 #import "ios/chrome/browser/ui/collection_view/cells/MDCCollectionViewCell+Chrome.h"
@@ -41,7 +35,6 @@
 #include "ios/chrome/browser/ui/history/history_entry_inserter.h"
 #import "ios/chrome/browser/ui/history/history_entry_item_delegate.h"
 #include "ios/chrome/browser/ui/history/history_util.h"
-#include "ios/chrome/browser/ui/history/ios_browsing_history_driver.h"
 #include "ios/chrome/browser/ui/history/legacy_history_entries_status_item.h"
 #import "ios/chrome/browser/ui/history/legacy_history_entry_item.h"
 #import "ios/chrome/browser/ui/url_loader.h"
@@ -80,12 +73,7 @@ const CGFloat kSeparatorInset = 10;
 @interface LegacyHistoryCollectionViewController ()<
     HistoryEntriesStatusItemDelegate,
     HistoryEntryInserterDelegate,
-    HistoryEntryItemDelegate,
-    BrowsingHistoryDriverDelegate> {
-  // Abstraction to communicate with HistoryService and WebHistoryService.
-  std::unique_ptr<BrowsingHistoryService> _browsingHistoryService;
-  // Provides dependencies and funnels callbacks from BrowsingHistoryService.
-  std::unique_ptr<IOSBrowsingHistoryDriver> _browsingHistoryDriver;
+    HistoryEntryItemDelegate> {
   // The main browser state. Not owned by HistoryCollectionViewController.
   ios::ChromeBrowserState* _browserState;
   // Backing ivar for delegate property.
@@ -166,6 +154,7 @@ const CGFloat kSeparatorInset = 10;
 @synthesize loading = _loading;
 @synthesize finishedLoading = _finishedLoading;
 @synthesize filterQueryResult = _filterQueryResult;
+@synthesize historyService = _historyService;
 
 - (instancetype)
 initWithLoader:(id<UrlLoader>)loader
@@ -175,13 +164,6 @@ initWithLoader:(id<UrlLoader>)loader
   self =
       [super initWithLayout:layout style:CollectionViewControllerStyleDefault];
   if (self) {
-    _browsingHistoryDriver =
-        std::make_unique<IOSBrowsingHistoryDriver>(browserState, self);
-    _browsingHistoryService = std::make_unique<BrowsingHistoryService>(
-        _browsingHistoryDriver.get(),
-        ios::HistoryServiceFactory::GetForBrowserState(
-            browserState, ServiceAccessType::EXPLICIT_ACCESS),
-        IOSChromeProfileSyncServiceFactory::GetForBrowserState(browserState));
     _browserState = browserState;
     _delegate = delegate;
     _URLLoader = loader;
@@ -196,13 +178,13 @@ initWithLoader:(id<UrlLoader>)loader
         [[HistoryEntryInserter alloc] initWithModel:self.collectionViewModel];
     _entryInserter.delegate = self;
     _empty = YES;
-    [self showHistoryMatchingQuery:nil];
   }
   return self;
 }
 
 - (void)viewDidLoad {
   [super viewDidLoad];
+  [self showHistoryMatchingQuery:nil];
   self.styler.cellLayoutType = MDCCollectionViewCellLayoutTypeList;
   self.styler.separatorInset =
       UIEdgeInsetsMake(0, kSeparatorInset, 0, kSeparatorInset);
@@ -272,7 +254,7 @@ initWithLoader:(id<UrlLoader>)loader
     entry.all_timestamps.insert(object.timestamp.ToInternalValue());
     entries.push_back(entry);
   }
-  _browsingHistoryService->RemoveVisits(entries);
+  self.historyService->RemoveVisits(entries);
   [self removeSelectedItemsFromCollection];
 }
 
@@ -365,14 +347,15 @@ initWithLoader:(id<UrlLoader>)loader
   }
 }
 
-#pragma mark - BrowsingHistoryDriverDelegate
+#pragma mark - HistoryConsumer
 
-- (void)onQueryCompleteWithResults:
+- (void)historyQueryWasCompletedWithResults:
             (const std::vector<BrowsingHistoryService::HistoryEntry>&)results
-                  queryResultsInfo:
-                      (const BrowsingHistoryService::QueryResultsInfo&)
-                          queryResultsInfo
-               continuationClosure:(base::OnceClosure)continuationClosure {
+                           queryResultsInfo:
+                               (const BrowsingHistoryService::QueryResultsInfo&)
+                                   queryResultsInfo
+                        continuationClosure:
+                            (base::OnceClosure)continuationClosure {
   self.loading = NO;
   _query_history_continuation = std::move(continuationClosure);
 
@@ -472,8 +455,7 @@ initWithLoader:(id<UrlLoader>)loader
   }
 }
 
-- (void)shouldShowNoticeAboutOtherFormsOfBrowsingHistory:
-    (BOOL)shouldShowNotice {
+- (void)showNoticeAboutOtherFormsOfBrowsingHistory:(BOOL)shouldShowNotice {
   self.shouldShowNoticeAboutOtherFormsOfBrowsingHistory = shouldShowNotice;
   // Update the history entries status message if there is no query in progress.
   if (!self.isLoading) {
@@ -481,7 +463,7 @@ initWithLoader:(id<UrlLoader>)loader
   }
 }
 
-- (void)didObserverHistoryDeletion {
+- (void)historyWasDeleted {
   // If history has been deleted, reload history filtering for the current
   // results. This only observes local changes to history, i.e. removing
   // history via the clear browsing data page.
@@ -648,7 +630,7 @@ initWithLoader:(id<UrlLoader>)loader
     options.max_count = kMaxFetchCount;
     options.matching_algorithm =
         query_parser::MatchingAlgorithm::ALWAYS_PREFIX_SEARCH;
-    _browsingHistoryService->QueryHistory(queryString, options);
+    self.historyService->QueryHistory(queryString, options);
   }
 }