Hira Mahmood (Gerrit)

unread,

Apr 1, 2026, 10:04:18 AM (yesterday) Apr 1

to Scott Yoder, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Attention needed from Scott Yoder

Hira Mahmood added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Hira Mahmood . resolved

@scott...@google.com PTAL, thanks! Decided not to force animate the exit fullscreen here since for sad tab the animation does look a bit weird (https://screencast.googleplex.com/cast/NjI5MTM4MDc5NTA4MDcwNHxiYzU4OWIwNC1mZg and https://screencast.googleplex.com/cast/NTE4NTc4NzUxMDM5MDc4NHxlN2M1MzVlOS04Mw to compare). Also, apparently side swipe for iPad also doesn't animate here. So the choice is given to the owner of each ScopedFullscreenDisabler instance just like the original implementation, but defaulting to animated

Open in Gerrit

Related details

Attention is currently required from:

Scott Yoder

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Scott Yoder (Gerrit)

unread,

Apr 1, 2026, 10:33:27 AM (yesterday) Apr 1

to Hira Mahmood, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Attention needed from Hira Mahmood

Scott Yoder voted and added 3 comments

Votes added by Scott Yoder

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Scott Yoder . resolved

LGTM with a couple of notes.

File ios/chrome/browser/bubble/ui_bundled/bubble_presenter.mm

Line 110, Patchset 9 (Latest): std::unique_ptr<ScopedFullscreenDisabler> _animatedFullscreenDisabler;

Scott Yoder . unresolved

super nit: You could probably simplify this to just `_fullscreenDisabler`. Also in other classes.

File ios/chrome/browser/fullscreen/coordinator/BUILD.gn

Line 15, Patchset 9 (Latest): "//ios/chrome/browser/fullscreen/ui_bundled",

Scott Yoder . unresolved

I was wondering why this was needed, and then I realized that ScopedFullscreenDisabler is still in ui_bundled. It should probably be moved to `ios/chrome/browser/fullscreen/public` or `model`. But that could happen in a separate CL.

Open in Gerrit

Related details

Attention is currently required from:

Hira Mahmood

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hira Mahmood (Gerrit)

unread,

Apr 1, 2026, 1:35:01 PM (yesterday) Apr 1

to Scott Yoder, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Hira Mahmood added 2 comments

File ios/chrome/browser/bubble/ui_bundled/bubble_presenter.mm

Line 110, Patchset 9: std::unique_ptr<ScopedFullscreenDisabler> _animatedFullscreenDisabler;

Scott Yoder . resolved

super nit: You could probably simplify this to just `_fullscreenDisabler`. Also in other classes.

Hira Mahmood

Done

File ios/chrome/browser/fullscreen/coordinator/BUILD.gn

Line 15, Patchset 9: "//ios/chrome/browser/fullscreen/ui_bundled",

Scott Yoder . resolved

I was wondering why this was needed, and then I realized that ScopedFullscreenDisabler is still in ui_bundled. It should probably be moved to `ios/chrome/browser/fullscreen/public` or `model`. But that could happen in a separate CL.

Hira Mahmood

Acknowledged

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hira Mahmood (Gerrit)

unread,

Apr 1, 2026, 1:36:29 PM (yesterday) Apr 1

to Gauthier Ambard, Scott Yoder, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Attention needed from Gauthier Ambard

Hira Mahmood added 1 comment

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Hira Mahmood . resolved

+gambard for owners review outside i/c/b/fullscreen, thanks!

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Scott Yoder (Gerrit)

unread,

Apr 1, 2026, 3:33:33 PM (yesterday) Apr 1

to Hira Mahmood, Aliona Dangla, Gauthier Ambard, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Attention needed from Gauthier Ambard and Hira Mahmood

Scott Yoder voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Gauthier Ambard
Hira Mahmood

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Gauthier Ambard (Gerrit)

unread,

6:11 AM (13 hours ago) 6:11 AM

to Hira Mahmood, Aliona Dangla, Scott Yoder, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Attention needed from Hira Mahmood

Gauthier Ambard voted and added 2 comments

Votes added by Gauthier Ambard

Code-Review

+1

2 comments

File ios/chrome/browser/browser_view/ui_bundled/browser_coordinator.mm

Line 1318, Patchset 10 (Latest): initWithBaseViewController:self.baseViewController

Gauthier Ambard . unresolved

You are updating the baseVC from self.view to self.baseVC. Does it matter?

File ios/chrome/browser/fullscreen/ui_bundled/scoped_fullscreen_disabler.mm

Line 27, Patchset 10 (Latest): if (handler_) {

Gauthier Ambard . unresolved

You don't need this check I think.

Open in Gerrit

Related details

Attention is currently required from:

Hira Mahmood

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Hira Mahmood (Gerrit)

unread,

2:10 PM (5 hours ago) 2:10 PM

to Gauthier Ambard, Aliona Dangla, Scott Yoder, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Hira Mahmood added 2 comments

File ios/chrome/browser/browser_view/ui_bundled/browser_coordinator.mm

Line 1318, Patchset 10: initWithBaseViewController:self.baseViewController

Gauthier Ambard . resolved

You are updating the baseVC from self.view to self.baseVC. Does it matter?

Hira Mahmood

ah, this was unintentional, changed it back. but actually the FullscreenCoordinator doesn't use the view controller rn and I do not think it'll need to. I'll update its constructor to just accept the browser in a follow-up

File ios/chrome/browser/fullscreen/ui_bundled/scoped_fullscreen_disabler.mm

Line 27, Patchset 10: if (handler_) {

Gauthier Ambard . resolved

You don't need this check I think.

Hira Mahmood

Done

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Hira Mahmood (Gerrit)

unread,

2:10 PM (5 hours ago) 2:10 PM

to Gauthier Ambard, Aliona Dangla, Scott Yoder, chromium...@chromium.org, Permissions Reviews, Chromium LUCI CQ, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Hira Mahmood voted Commit-Queue+2

Commit-Queue

+2

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

3:12 PM (4 hours ago) 3:12 PM

to Hira Mahmood, Gauthier Ambard, Aliona Dangla, Scott Yoder, chromium...@chromium.org, Permissions Reviews, AyeAye, feature-me...@chromium.org, ios-r...@chromium.org, ios-revie...@chromium.org, marq+...@chromium.org, christia...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

10 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: ios/chrome/browser/browser_view/ui_bundled/browser_coordinator.mm
Insertions: 4, Deletions: 3.

@@ -1315,7 +1315,7 @@
   // them to use it (e.g., BubblePresenterCoordinator and SideSwipeCoordinator).
   if (IsFullscreenRefactoringEnabled()) {
     _fullscreenCoordinator = [[FullscreenCoordinator alloc]
-        initWithBaseViewController:self.baseViewController
+        initWithBaseViewController:self.viewController
                            browser:browser];
     [_fullscreenCoordinator start];
   }
@@ -1491,6 +1491,9 @@
   [_sideSwipeCoordinator stop];
   _sideSwipeCoordinator = nil;
 
+  [_fullscreenCoordinator stop];
+  _fullscreenCoordinator = nil;
+
   [_toolbarCoordinator stop];
   _toolbarCoordinator = nil;
   _omniboxCommandsHandler = nil;
@@ -1861,8 +1864,6 @@
   [self dismissDockingPromo];
   [self hideWelcomeBackPromo];
   [self hideComposeboxImmediately:YES completion:nil];
-  [_fullscreenCoordinator stop];
-  _fullscreenCoordinator = nil;
 }
 
 // Starts independent mediators owned by this coordinator.
```
```
The name of the file: ios/chrome/browser/fullscreen/ui_bundled/scoped_fullscreen_disabler.mm
Insertions: 1, Deletions: 3.

@@ -24,9 +24,7 @@
   if (controller_) {
     controller_->DecrementDisabledCounter();
   }
-  if (handler_) {
-    [handler_ reenableFullscreen];
-  }
+  [handler_ reenableFullscreen];
 }
 
 void ScopedFullscreenDisabler::FullscreenControllerWillShutDown(
```
```
The name of the file: ios/chrome/browser/location_bar/badge/coordinator/location_bar_badge_coordinator.mm
Insertions: 0, Deletions: 1.

@@ -14,7 +14,6 @@
 #import "ios/chrome/browser/fullscreen/ui_bundled/animated_scoped_fullscreen_disabler.h"
 #import "ios/chrome/browser/fullscreen/ui_bundled/fullscreen_ui_updater.h"
 #import "ios/chrome/browser/fullscreen/ui_bundled/scoped_fullscreen_disabler.h"
-#import "ios/chrome/browser/intelligence/bwg/model/bwg_service_factory.h"
 #import "ios/chrome/browser/intelligence/bwg/model/gemini_service_factory.h"
 #import "ios/chrome/browser/intelligence/bwg/utils/gemini_constants.h"
 #import "ios/chrome/browser/location_bar/badge/coordinator/location_bar_badge_coordinator_delegate.h"
```
```
The name of the file: ios/chrome/browser/side_swipe/ui_bundled/side_swipe_ui_controller.mm
Insertions: 0, Deletions: 4.

@@ -69,10 +69,6 @@
   // The disabler that prevents the toolbar from being scrolled away when the
   // side swipe gesture is being recognized.
   std::unique_ptr<ScopedFullscreenDisabler> _fullscreenDisabler;
-
-  // The animated disabler displays the toolbar when a side swipe navigation
-  // gesture is being recognized.
-  std::unique_ptr<ScopedFullscreenDisabler> _fullscreenDisabler;
   std::unique_ptr<AnimatedScopedFullscreenDisabler>
       _legacyAnimatedFullscreenDisabler;
 
```

Change information

Commit message:

[iOS] Update ScopedFullscreenDisabler and consumers

This CL updates ScopedFullscreenDisabler to use FullscreenCommands and
an animated flag. This will allow for the eventual removal of
AnimatedScopedFullscreenDisabler once the refactor is complete.

More specifically, this CL:
 - Updates all consumers of
   ScopedFullscreenDisabler/AnimatedScopedFullscreenDisabler to use
   the new disabler constructor behind the feature flag.
 - Updates FullscreenCommands to support an animated boolean.
 - Starts FullscreenCoordinator earlier for handler availability.
 - Removes ChromeCoordinator+FullscreenDisabling.

Fixed: 493903024

Change-Id: I125fb632fa1b6c97f681904fa46ca6cace78c645

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7709909

Commit-Queue: Hira Mahmood <hiram...@google.com>

Reviewed-by: Scott Yoder <scott...@google.com>

Reviewed-by: Gauthier Ambard <gam...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1609418}

Files:

M ios/chrome/browser/browser_view/ui_bundled/browser_coordinator.mm
M ios/chrome/browser/bubble/ui_bundled/bubble_presenter.h
M ios/chrome/browser/bubble/ui_bundled/bubble_presenter.mm
M ios/chrome/browser/bubble/ui_bundled/bubble_presenter_coordinator.mm
M ios/chrome/browser/contextual_panel/coordinator/contextual_sheet_coordinator.mm
M ios/chrome/browser/contextual_panel/entrypoint/coordinator/contextual_panel_entrypoint_coordinator.mm
M ios/chrome/browser/fullscreen/coordinator/BUILD.gn
M ios/chrome/browser/fullscreen/coordinator/fullscreen_mediator.mm
M ios/chrome/browser/fullscreen/model/fullscreen_browser_agent.h
M ios/chrome/browser/fullscreen/model/fullscreen_browser_agent.mm
M ios/chrome/browser/fullscreen/model/fullscreen_browser_agent_unittest.mm
M ios/chrome/browser/fullscreen/ui_bundled/BUILD.gn
M ios/chrome/browser/fullscreen/ui_bundled/animated_scoped_fullscreen_disabler.h
D ios/chrome/browser/fullscreen/ui_bundled/chrome_coordinator+fullscreen_disabling.h
D ios/chrome/browser/fullscreen/ui_bundled/chrome_coordinator+fullscreen_disabling.mm
M ios/chrome/browser/fullscreen/ui_bundled/scoped_fullscreen_disabler.h
M ios/chrome/browser/fullscreen/ui_bundled/scoped_fullscreen_disabler.mm
M ios/chrome/browser/infobars/ui_bundled/coordinators/infobar_coordinator.mm
M ios/chrome/browser/location_bar/badge/coordinator/location_bar_badge_coordinator.mm
M ios/chrome/browser/overlays/ui_bundled/BUILD.gn
M ios/chrome/browser/overlays/ui_bundled/overlay_presentation_context_fullscreen_disabler.h
M ios/chrome/browser/overlays/ui_bundled/overlay_presentation_context_fullscreen_disabler.mm
M ios/chrome/browser/page_info/coordinator/BUILD.gn
M ios/chrome/browser/sad_tab/ui_bundled/BUILD.gn
M ios/chrome/browser/sad_tab/ui_bundled/sad_tab_coordinator.mm
M ios/chrome/browser/shared/public/commands/fullscreen_commands.h
M ios/chrome/browser/side_swipe/ui_bundled/side_swipe_coordinator.mm
M ios/chrome/browser/side_swipe/ui_bundled/side_swipe_ui_controller.h
M ios/chrome/browser/side_swipe/ui_bundled/side_swipe_ui_controller.mm

Change size: L

Delta: 29 files changed, 226 insertions(+), 218 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Gauthier Ambard, +1 by Scott Yoder

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[iOS] Update ScopedFullscreenDisabler and consumers [chromium/src : main]

Hira Mahmood (Gerrit)

Hira Mahmood added 1 comment

Related details

Scott Yoder (Gerrit)

Scott Yoder voted and added 3 comments

Votes added by Scott Yoder

3 comments

Related details

Hira Mahmood (Gerrit)

Hira Mahmood added 2 comments

Related details

Hira Mahmood (Gerrit)

Hira Mahmood added 1 comment

Related details

Scott Yoder (Gerrit)

Scott Yoder voted Code-Review+1

Related details

Gauthier Ambard (Gerrit)

Gauthier Ambard voted and added 2 comments

Votes added by Gauthier Ambard

2 comments

Related details

Hira Mahmood (Gerrit)

Hira Mahmood added 2 comments

Related details

Hira Mahmood (Gerrit)

Hira Mahmood voted Commit-Queue+2

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information