New issue
Advanced search Search tips

Issue 831594 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 0
Type: Bug
Q2

Blocking:
issue 818732



Sign in to add a comment

TabGridMediator is not updated when the incognito TabModel is destroyed and recreated

Project Member Reported by rohitrao@chromium.org, Apr 11 2018

Issue description

The easiest way to repro this is to run CookiesTestCase.testClearIncognitoFromIncognito with the UIRefreshPhase1 flag enabled.

The test DCHECKs with a nullptr _browserState.  What's happening is that the TabGridMediator initially gets a pointer to an incognitoTabModel.  When the last incognito tab is closed, the TabModel is disconnected, which clears out its _browserState ivar, but the mediator continues to hold a strong reference to this now-defunct TabModel.  When we try to use that TabModel later, we DCHECK.

https://cs.chromium.org/chromium/src/ios/chrome/app/main_controller.mm?sq=package:chromium&l=860

Here in MainController we update the TABLET_SWITCHER with the new TabModel, but not the GRID switcher.  Simply changing this line (and one below) makes the app crash in different ways, however.  For example, opening a new incognito tab and then closing it from the tablet tabstrip.
 

Comment 1 by marq@chromium.org, Apr 11 2018

Components: UI>Browser>Mobile>TabSwitcher
Labels: -Pri-1 MS-Tab-Grid Q2 S-See-Open-Tabs Pri-0

Comment 2 by marq@chromium.org, Apr 11 2018

NextAction: 2018-04-25

Comment 3 by marq@chromium.org, Apr 11 2018

Status: Assigned (was: Untriaged)

Comment 4 by marq@chromium.org, Apr 12 2018

Status: Started (was: Assigned)

Comment 5 by marq@chromium.org, Apr 12 2018

Labels: small
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48c7dbdfe4928acd61a186a36428583a9b3e99d4

commit 48c7dbdfe4928acd61a186a36428583a9b3e99d4
Author: Mark Cogan <marq@google.com>
Date: Fri Apr 13 08:12:59 2018

[iOS] Fix how the tab grid is notified of OTR tab model changes.

This Cl fixes two problems that can occur when the OTR tab model is
replaced.

First, when the last incognito tab is closed, the tab grid should
have its OTR tab model set to nil so it can stop observing the OTR
web state list.

Second, MainController should set _tabSwitcher to be the TabGridAdaptor
as soon as its created, not when the tab switcher is displayed.

This CL also has the TabGridCoordinator stop keeping pointers to tab
models after it has created mediators.

Bug:  831594 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I61f674bde5d2a64645b8ccb7ed1129b6801eddcc
Reviewed-on: https://chromium-review.googlesource.com/1010288
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550543}
[modify] https://crrev.com/48c7dbdfe4928acd61a186a36428583a9b3e99d4/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/48c7dbdfe4928acd61a186a36428583a9b3e99d4/ios/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm

Comment 7 by marq@chromium.org, Apr 13 2018

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48c7dbdfe4928acd61a186a36428583a9b3e99d4

commit 48c7dbdfe4928acd61a186a36428583a9b3e99d4
Author: Mark Cogan <marq@google.com>
Date: Fri Apr 13 08:12:59 2018

[iOS] Fix how the tab grid is notified of OTR tab model changes.

This Cl fixes two problems that can occur when the OTR tab model is
replaced.

First, when the last incognito tab is closed, the tab grid should
have its OTR tab model set to nil so it can stop observing the OTR
web state list.

Second, MainController should set _tabSwitcher to be the TabGridAdaptor
as soon as its created, not when the tab switcher is displayed.

This CL also has the TabGridCoordinator stop keeping pointers to tab
models after it has created mediators.

Bug:  831594 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I61f674bde5d2a64645b8ccb7ed1129b6801eddcc
Reviewed-on: https://chromium-review.googlesource.com/1010288
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550543}
[modify] https://crrev.com/48c7dbdfe4928acd61a186a36428583a9b3e99d4/ios/chrome/app/main_controller.mm
[modify] https://crrev.com/48c7dbdfe4928acd61a186a36428583a9b3e99d4/ios/chrome/browser/ui/tab_grid/tab_grid_coordinator.mm

The NextAction date has arrived: 2018-04-25
NextAction: ----
Labels: Proj-UIRefresh

Sign in to add a comment