[iOS] Only disable AppBar button for Lock if we are in incognito [chromium/src : main]
Gauthier Ambard (Gerrit)
Gauthier Ambard voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Ginny Huang
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ginny Huang (Gerrit)
Ginny Huang voted and added 2 comments![]( "Open in Gerrit")
Votes added by Ginny Huang
| Code-Review | +1 |
2 comments
- (void)willEnterIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _incognitoWebStateList;
}
- (void)willExitIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _regularWebStateList;
}Do they cover cases like "open a tab in incognito"?
// Tests that the consumer is updated when opening the app on a regular tabsLGTM but I'm curious, which of the new test cases would not have passed if you don't update the code? What did I miss in the internal implementation?
Related details
- Gauthier Ambard
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. |
Ginny Huang (Gerrit)
Ginny Huang added 2 comments![]( "Open in Gerrit")
- (void)willEnterIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _incognitoWebStateList;
}
- (void)willExitIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _regularWebStateList;
}Do they cover cases like "open a tab in incognito"?
Nevermind, found the conversation about this.
// Tests that the consumer is updated when opening the app on a regular tabsLGTM but I'm curious, which of the new test cases would not have passed if you don't update the code? What did I miss in the internal implementation?
Acknowledged
Related details
- Gauthier Ambard
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ginny Huang (Gerrit)
Ginny Huang added 2 comments![]( "Open in Gerrit")
- (void)willEnterIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _incognitoWebStateList;
}
- (void)willExitIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _regularWebStateList;
}Ginny HuangDo they cover cases like "open a tab in incognito"?
Nevermind, found the conversation about this.
Apologies... I actually didn't
// Tests that the consumer is updated when opening the app on a regular tabsGinny HuangLGTM but I'm curious, which of the new test cases would not have passed if you don't update the code? What did I miss in the internal implementation?
Acknowledged
un-ack, last message was a mistake
Related details
- Gauthier Ambard
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. |
Gauthier Ambard (Gerrit)
Gauthier Ambard added 2 comments![]( "Open in Gerrit")
- (void)willEnterIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _incognitoWebStateList;
}
- (void)willExitIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _regularWebStateList;
}Ginny HuangDo they cover cases like "open a tab in incognito"?
Ginny HuangNevermind, found the conversation about this.
Apologies... I actually didn't
I am not sure to understand what you are sugesting here. Is it "I am opening a new incognito tab while I can't" which would be blocked? Like that https://screencast.googleplex.com/cast/NDU1MTI1MTk3ODA5MjU0NHw1YzRhMDM4NS1mNw ?
Or something else?
// Tests that the consumer is updated when opening the app on a regular tabsGinny HuangLGTM but I'm curious, which of the new test cases would not have passed if you don't update the code? What did I miss in the internal implementation?
Ginny HuangAcknowledged
un-ack, last message was a mistake
Sometimes (unclear when), the app is started in the state:
```
tab_grid_state_.tabGridVisible = NO;
tab_grid_state_.currentPage = TabGridPageIncognitoTabs;
incognito_state_.incognitoContentVisible = NO;
```
So basically, I am seeing a regular page but the TabGrid state is incognito.
Related details
- Ginny Huang
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. |
Ginny Huang (Gerrit)
Ginny Huang added 2 comments![]( "Open in Gerrit")
- (void)willEnterIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _incognitoWebStateList;
}
- (void)willExitIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _regularWebStateList;
}Ginny HuangDo they cover cases like "open a tab in incognito"?
Ginny HuangNevermind, found the conversation about this.
Gauthier AmbardApologies... I actually didn't
I am not sure to understand what you are sugesting here. Is it "I am opening a new incognito tab while I can't" which would be blocked? Like that https://screencast.googleplex.com/cast/NDU1MTI1MTk3ODA5MjU0NHw1YzRhMDM4NS1mNw ?
Or something else?
Apologies for being unclear - my real question is this -
Why do you need these calls? Aren't they handled by anything in `TabGridStateObserver`methods, which will call `updateForTabGridPage`?
// Tests that the consumer is updated when opening the app on a regular tabsGinny HuangLGTM but I'm curious, which of the new test cases would not have passed if you don't update the code? What did I miss in the internal implementation?
Ginny HuangAcknowledged
Gauthier Ambardun-ack, last message was a mistake
Sometimes (unclear when), the app is started in the state:
```
tab_grid_state_.tabGridVisible = NO;
tab_grid_state_.currentPage = TabGridPageIncognitoTabs;
incognito_state_.incognitoContentVisible = NO;
```So basically, I am seeing a regular page but the TabGrid state is incognito.
- Gauthier Ambard
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. |
Gauthier Ambard (Gerrit)
Gauthier Ambard voted and added 2 comments![]( "Open in Gerrit")
Votes added by Gauthier Ambard
| Commit-Queue | +2 |
- (void)willEnterIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _incognitoWebStateList;
}
- (void)willExitIncognitoForState:(IncognitoState*)incognitoState {
if (_tabGridState.tabGridVisible) {
return;
}
[self updateButtonsForCurrentTabGridPage];
self.currentWebStateList = _regularWebStateList;
}Ginny HuangDo they cover cases like "open a tab in incognito"?
Ginny HuangNevermind, found the conversation about this.
Gauthier AmbardApologies... I actually didn't
Ginny HuangI am not sure to understand what you are sugesting here. Is it "I am opening a new incognito tab while I can't" which would be blocked? Like that https://screencast.googleplex.com/cast/NDU1MTI1MTk3ODA5MjU0NHw1YzRhMDM4NS1mNw ?
Or something else?
Apologies for being unclear - my real question is this -
Why do you need these calls? Aren't they handled by anything in `TabGridStateObserver`methods, which will call `updateForTabGridPage`?
The TabGrid is not necessarily in sync with what's presented by the browser. That's by design, we want to be able to remove the TabGrid from the view hierarchy when showing the Browser at some point in the future.
And also `willChangePageTo` is gated by the TabGrid visibility. So it will not update the consumer.
And yes, if you open a new incognito tab from a regular tab, it doesn't trigger a page change in the TabGrid.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard voted Commit-Queue+0![]( "Open in Gerrit")
| Commit-Queue | +0 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
[iOS] Only disable AppBar button for Lock if we are in incognito
When the incognito lock is on (so the incognito state can't be access),
the AppBar should be disabled only if the incognito content is visible.
- M ios/chrome/browser/app_bar/coordinator/app_bar_mediator.mm
- M ios/chrome/browser/app_bar/coordinator/app_bar_mediator_unittest.mm
Code-Review: +1 by Ginny Huang
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |