Commit 5f73d42d authored by Robbie Gibson's avatar Robbie Gibson Committed by Commit Bot

[iOS] Prevent navigations in one tab from closing FIP in another tab

The underlying issue is that the find_bar_mediator was observing
navigation events and closing FIP when a navigation occurs. As the
state for "show FIP" is a tab-level state, the observation should occur
in the tab helper.

Now, the tab helper observes the navigation in its own webstate.
Alerting the UI that Find In Page has stopped is done through the
FindInPageResponseDelegate. (This used to be the coordinator, but
should really be the mediator, so that has changed too.) This is hooked
up to the active mediator only when the UI is active.

From the bug, the same issue occurs with Text Zoom, but Text Zoom is
closed automatically on switching tabs, so that behavior was kept.

Fixed: 1067603
Change-Id: Ic5b1fb61142ab3deadc8ef792642e880cf23ebea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2157226
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760945}
parent a3984348
......@@ -83,6 +83,8 @@ class FindTabHelper : public web::WebStateObserver,
// web::WebStateObserver.
void WebStateDestroyed(web::WebState* web_state) override;
void DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override;
// The ObjC find in page controller.
FindInPageController* controller_;
......
......@@ -95,4 +95,12 @@ void FindTabHelper::WebStateDestroyed(web::WebState* web_state) {
web_state->RemoveObserver(this);
}
void FindTabHelper::DidFinishNavigation(
web::WebState* web_state,
web::NavigationContext* navigation_context) {
if (IsFindUIActive()) {
StopFinding(nil);
}
}
WEB_STATE_USER_DATA_KEY_IMPL(FindTabHelper)
......@@ -5,6 +5,7 @@
source_set("find_bar") {
configs += [ "//build/config/compiler:enable_arc" ]
sources = [
"find_bar_consumer.h",
"find_bar_controller_ios.h",
"find_bar_controller_ios.mm",
"find_bar_coordinator.h",
......
// Copyright 2020 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_FIND_BAR_FIND_BAR_CONSUMER_H_
#define IOS_CHROME_BROWSER_UI_FIND_BAR_FIND_BAR_CONSUMER_H_
@class FindInPageModel;
@protocol FindBarConsumer <NSObject>
- (void)updateResultsCount:(FindInPageModel*)model;
@end
#endif // IOS_CHROME_BROWSER_UI_FIND_BAR_FIND_BAR_CONSUMER_H_
......@@ -7,12 +7,14 @@
#import <UIKit/UIKit.h>
#import "ios/chrome/browser/ui/find_bar/find_bar_consumer.h"
@protocol BrowserCommands;
@class FindBarViewController;
@protocol FindInPageCommands;
@class FindInPageModel;
@interface FindBarControllerIOS : NSObject
@interface FindBarControllerIOS : NSObject <FindBarConsumer>
// The command handler for all necessary commands
@property(nonatomic, weak) id<FindInPageCommands> commandHandler;
......
......@@ -5,7 +5,6 @@
#import "ios/chrome/browser/ui/find_bar/find_bar_coordinator.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#import "ios/chrome/browser/find_in_page/find_in_page_response_delegate.h"
#import "ios/chrome/browser/find_in_page/find_tab_helper.h"
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/ui/commands/browser_commands.h"
......@@ -23,8 +22,7 @@
#error "This file requires ARC support."
#endif
@interface FindBarCoordinator () <FindInPageResponseDelegate,
ContainedPresenterDelegate>
@interface FindBarCoordinator () <ContainedPresenterDelegate>
@property(nonatomic, strong) FindBarMediator* mediator;
......@@ -45,17 +43,16 @@
self.presenter.delegate = self;
self.mediator = [[FindBarMediator alloc]
initWithWebStateList:self.browser->GetWebStateList()
commandHandler:self.findInPageCommandHandler];
initWithCommandHandler:self.findInPageCommandHandler];
self.mediator.consumer = self.findBarController;
DCHECK(self.currentWebState);
FindTabHelper* helper = FindTabHelper::FromWebState(self.currentWebState);
helper->SetResponseDelegate(self.mediator);
// If the FindUI is already active, just reshow it.
if (helper->IsFindUIActive()) {
[self showAnimated:NO shouldFocus:[self.findBarController isFocused]];
} else {
DCHECK(!helper->IsFindUIActive());
helper->SetResponseDelegate(self);
helper->SetFindUIActive(true);
[self showAnimated:YES shouldFocus:YES];
}
......@@ -113,16 +110,6 @@
}
}
#pragma mark - FindInPageResponseDelegate
- (void)findDidFinishWithUpdatedModel:(FindInPageModel*)model {
[self.findBarController updateResultsCount:model];
}
- (void)findDidStop {
[self.findInPageCommandHandler closeFindInPage];
}
#pragma mark - ContainedPresenterDelegate
- (void)containedPresenterDidPresent:(id<ContainedPresenter>)presenter {
......
......@@ -7,16 +7,20 @@
#import <Foundation/Foundation.h>
#import "ios/chrome/browser/find_in_page/find_in_page_response_delegate.h"
@protocol FindBarConsumer;
@protocol FindInPageCommands;
class WebStateList;
// Mediator for the Find Bar and the Find In page feature. As this feature is
// currently being split off from BVC, this mediator will have more features
// added and is not an ideal example of the mediator pattern.
@interface FindBarMediator : NSObject
@interface FindBarMediator : NSObject <FindInPageResponseDelegate>
- (instancetype)initWithCommandHandler:(id<FindInPageCommands>)commandHandler;
- (instancetype)initWithWebStateList:(WebStateList*)webStateList
commandHandler:(id<FindInPageCommands>)commandHandler;
@property(nonatomic, weak) id<FindBarConsumer> consumer;
@end
......
......@@ -7,6 +7,7 @@
#import "ios/chrome/browser/main/browser.h"
#import "ios/chrome/browser/ui/commands/command_dispatcher.h"
#import "ios/chrome/browser/ui/commands/find_in_page_commands.h"
#import "ios/chrome/browser/ui/find_bar/find_bar_consumer.h"
#import "ios/chrome/browser/web_state_list/active_web_state_observation_forwarder.h"
#import "ios/web/public/web_state.h"
#import "ios/web/public/web_state_observer_bridge.h"
......@@ -15,10 +16,7 @@
#error "This file requires ARC support."
#endif
@interface FindBarMediator () <CRWWebStateObserver> {
std::unique_ptr<web::WebStateObserverBridge> _observer;
std::unique_ptr<ActiveWebStateObservationForwarder> _forwarder;
}
@interface FindBarMediator ()
// Handler for any FindInPageCommands.
@property(nonatomic, weak) id<FindInPageCommands> commandHandler;
......@@ -27,30 +25,21 @@
@implementation FindBarMediator
- (instancetype)initWithWebStateList:(WebStateList*)webStateList
commandHandler:(id<FindInPageCommands>)commandHandler {
- (instancetype)initWithCommandHandler:(id<FindInPageCommands>)commandHandler {
self = [super init];
if (self) {
DCHECK(webStateList);
_commandHandler = commandHandler;
// Set up the WebState and its observer.
_observer = std::make_unique<web::WebStateObserverBridge>(self);
_forwarder = std::make_unique<ActiveWebStateObservationForwarder>(
webStateList, _observer.get());
}
return self;
}
- (void)dealloc {
_forwarder = nullptr;
}
#pragma mark - FindInPageResponseDelegate
#pragma mark - CRWWebStateObserver
- (void)findDidFinishWithUpdatedModel:(FindInPageModel*)model {
[self.consumer updateResultsCount:model];
}
- (void)webState:(web::WebState*)webState
didFinishNavigation:(web::NavigationContext*)navigation {
- (void)findDidStop {
[self.commandHandler closeFindInPage];
}
......
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