Commit f46c8b01 authored by sczs's avatar sczs Committed by Commit Bot

[ios] Fixes UserSettingsDisc presentation whenever a Message is present.

If a Message Banner is being presented and the user taps on its picture/
initial letter Disc, the settings screen wouldn't be presented. This
happens because BVC is actually not the presenter VC and it can't dismiss
the Banner.

This CL uses the existent functionality for
showGoogleServicesSettingsFromViewController where BVC is used instead of
the parameter VC. This also takes us closer to not sending a VC through the
dispatcher.

It also removes a DCHECK since a presentedVC should leave that decision to
the presenterVC. This is something we currently lack and I hope to work on
this after Messages.

I didn't add an integration test since we don't have a good way to trigger
an Infobar on NTP in an integration test.

Bug: 961343
Change-Id: I85ea8047f5b55abd6e5606b4046d7dd7ad158625
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834206
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702011}
parent 73063bff
...@@ -1684,7 +1684,7 @@ enum class EnterTabSwitcherSnapshotResult { ...@@ -1684,7 +1684,7 @@ enum class EnterTabSwitcherSnapshotResult {
DCHECK_EQ(self.currentBVC, self.mainCoordinator.activeViewController); DCHECK_EQ(self.currentBVC, self.mainCoordinator.activeViewController);
baseViewController = self.currentBVC; baseViewController = self.currentBVC;
} }
DCHECK(![baseViewController presentedViewController]);
if ([self currentBrowserState]->IsOffTheRecord()) { if ([self currentBrowserState]->IsOffTheRecord()) {
NOTREACHED(); NOTREACHED();
return; return;
...@@ -1710,7 +1710,7 @@ enum class EnterTabSwitcherSnapshotResult { ...@@ -1710,7 +1710,7 @@ enum class EnterTabSwitcherSnapshotResult {
DCHECK_EQ(self.currentBVC, self.mainCoordinator.activeViewController); DCHECK_EQ(self.currentBVC, self.mainCoordinator.activeViewController);
baseViewController = self.currentBVC; baseViewController = self.currentBVC;
} }
DCHECK(![baseViewController presentedViewController]);
if (_settingsNavigationController) { if (_settingsNavigationController) {
// Navigate to the Google services settings if the settings dialog is // Navigate to the Google services settings if the settings dialog is
// already opened. // already opened.
......
...@@ -18,11 +18,15 @@ ...@@ -18,11 +18,15 @@
// may also be forwarded directly to a settings navigation controller. // may also be forwarded directly to a settings navigation controller.
@protocol ApplicationSettingsCommands @protocol ApplicationSettingsCommands
// Shows the accounts settings UI, presenting from |baseViewController|. // TODO(crbug.com/779791) : Do not pass baseViewController through dispatcher.
// Shows the accounts settings UI, presenting from |baseViewController|. If
// |baseViewController| is nil BVC will be used as presenterViewController.
- (void)showAccountsSettingsFromViewController: - (void)showAccountsSettingsFromViewController:
(UIViewController*)baseViewController; (UIViewController*)baseViewController;
// TODO(crbug.com/779791) : Do not pass baseViewController through dispatcher.
// Shows the Google services settings UI, presenting from |baseViewController|. // Shows the Google services settings UI, presenting from |baseViewController|.
// If |baseViewController| is nil BVC will be used as presenterViewController.
- (void)showGoogleServicesSettingsFromViewController: - (void)showGoogleServicesSettingsFromViewController:
(UIViewController*)baseViewController; (UIViewController*)baseViewController;
......
...@@ -366,7 +366,7 @@ using base::UserMetricsAction; ...@@ -366,7 +366,7 @@ using base::UserMetricsAction;
- (void)identityDiscTapped { - (void)identityDiscTapped {
base::RecordAction(base::UserMetricsAction("MobileNTPIdentityDiscTapped")); base::RecordAction(base::UserMetricsAction("MobileNTPIdentityDiscTapped"));
[self.dispatcher showGoogleServicesSettingsFromViewController:self]; [self.dispatcher showGoogleServicesSettingsFromViewController:nil];
} }
// TODO(crbug.com/807330) The fakebox is currently a collection of views spread // TODO(crbug.com/807330) The fakebox is currently a collection of views spread
......
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