[iOS][PRDBD] Implement QuickDeleteOtherDataConsumer [chromium/src : main]
Leo Zhao (Gerrit)
Leo Zhao added 4 comments![]( "Open in Gerrit")
// Sets the default search engine to nonGoogle, a not prepopulate searchIs this `nonGoogle' a special word to refer to nongoogle? Also, you can define a const at the beginning to describe it better. If switching to any other search engine that is not google, this can just say switching to other search engine.
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));
If you want to verify when Google is the default search engine, the code behaves in certain way, then you can do line 136 does, set it to Google and then verify. This ASSERT_EQ is asserting that the default search engine is Google. It is using a test to test the default value of the `template_url_service_`, which should not be the focus of this unit test for the mediator.
// Verifies that Google is the default search provider.
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));Same here. If it is needed for Google to be the default search engine, you either need to mock it or use `setUp` to set it up that way. If it is been set up that way, there is no need to verify it. If something is wrong, then the code below will fail the test. There are 4 more similar cases below.
// Sets the ViewController with the title for the "Quick Delete Other data"
// page.
- (void)setOtherDataPageTitle:(NSString*)title;
// Sets the ViewController with the subtitle for the search history cell.
- (void)setSearchHistorySubtitle:(NSString*)subtitle;
// Sets the boolean on whether the ViewController should show the "My Activity"
// cell.
- (void)setShouldShowMyActivityCell:(BOOL)shouldShowMyActivityCell;
// Sets the boolean on whether the ViewController should show the Search History
// cell.
- (void)setShouldShowSearchHistoryCell:(BOOL)shouldShowSearchHistoryCell;Something to think about is to refine this interface to describe intents. Right now, there are 4 things that are configurable. This does not feel like a consumer, the mediator is deciding all the UI details. Hypothetically, if the UI can show two modes: one with just history, the other with all three of them. You can consider a function like: setTitle:historySubtitle:full:. If you need the flexility to dynamically configuration everything, then it is fine to keep this set of functions as is.
Related details
- Angela Novakovic
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. |
Leo Zhao (Gerrit)
Leo Zhao added 1 comment![]( "Open in Gerrit")
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));
If you want to verify when Google is the default search engine, the code behaves in certain way, then you can do line 136 does, set it to Google and then verify. This ASSERT_EQ is asserting that the default search engine is Google. It is using a test to test the default value of the `template_url_service_`, which should not be the focus of this unit test for the mediator.
I came up with a good example. Let's say, a person is writing a test that uses dateService.
The test looks like this:
test {
assert dateService.year == 2026
dateService.setYear(2029)
assert dateService.year == 2029
}This test works, until when it is next year, then it stops to work. Here, assuming dateService's year is 2026 isn't a reasonable assumption, since it can change.
However, you can write test like this:
test {
assert dateService.year != nil
dateService.setYear(2029)
assert dateService.year == 2029
}Here, you can assume dateService should provide a valid year for your code to function. This is a reasonable assumption. Since the program is designed to have dateService to provide current year. The year changes, however, it should always provide a year. If it does, that is an issue that we need to take a look.
Angela Novakovic (Gerrit)
Angela Novakovic added 4 comments![]( "Open in Gerrit")
// Sets the default search engine to nonGoogle, a not prepopulate searchIs this `nonGoogle' a special word to refer to nongoogle? Also, you can define a const at the beginning to describe it better. If switching to any other search engine that is not google, this can just say switching to other search engine.
This seems to be the norm in the clear browsing data flow. The nonGoogle is, as you've said, just another search engine, in this case `nongoogle`. The same has been done in `QuickDeleteMediatorTest`.
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));
Leo ZhaoIf you want to verify when Google is the default search engine, the code behaves in certain way, then you can do line 136 does, set it to Google and then verify. This ASSERT_EQ is asserting that the default search engine is Google. It is using a test to test the default value of the `template_url_service_`, which should not be the focus of this unit test for the mediator.
I came up with a good example. Let's say, a person is writing a test that uses dateService.
The test looks like this:
test {
assert dateService.year == 2026
dateService.setYear(2029)
assert dateService.year == 2029
}This test works, until when it is next year, then it stops to work. Here, assuming dateService's year is 2026 isn't a reasonable assumption, since it can change.
However, you can write test like this:
test {
assert dateService.year != nil
dateService.setYear(2029)
assert dateService.year == 2029
}Here, you can assume dateService should provide a valid year for your code to function. This is a reasonable assumption. Since the program is designed to have dateService to provide current year. The year changes, however, it should always provide a year. If it does, that is an issue that we need to take a look.
Ah, I see. You are saying that we shouldn't make assumptions that a current default value will stay the default value, thus it would be better (to avoid future issues) that we set the default search engine to Google.
// Verifies that Google is the default search provider.
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));Same here. If it is needed for Google to be the default search engine, you either need to mock it or use `setUp` to set it up that way. If it is been set up that way, there is no need to verify it. If something is wrong, then the code below will fail the test. There are 4 more similar cases below.
Done
// Sets the ViewController with the title for the "Quick Delete Other data"
// page.
- (void)setOtherDataPageTitle:(NSString*)title;
// Sets the ViewController with the subtitle for the search history cell.
- (void)setSearchHistorySubtitle:(NSString*)subtitle;
// Sets the boolean on whether the ViewController should show the "My Activity"
// cell.
- (void)setShouldShowMyActivityCell:(BOOL)shouldShowMyActivityCell;
// Sets the boolean on whether the ViewController should show the Search History
// cell.
- (void)setShouldShowSearchHistoryCell:(BOOL)shouldShowSearchHistoryCell;Something to think about is to refine this interface to describe intents. Right now, there are 4 things that are configurable. This does not feel like a consumer, the mediator is deciding all the UI details. Hypothetically, if the UI can show two modes: one with just history, the other with all three of them. You can consider a function like: setTitle:historySubtitle:full:. If you need the flexility to dynamically configuration everything, then it is fine to keep this set of functions as is.
I need to dynamically update the strings that are shown in the UI based on the user's sign-in status and the default search engine.
These are the 4 items that change, irrespective of each other. Other strings will be static, thus I will be adding them directly in the ViewController.
Related details
- Leo Zhao
- Noémie St-Onge
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. |
Leo Zhao (Gerrit)
Leo Zhao added 3 comments![]( "Open in Gerrit")
// Sets the default search engine to a not prepopulated search engine which is
// not be Google."... which is not be Google.", is the "be" before Google a typo?
// Sets the default search engine to a prepopulated search engine which is not
// be Google."which is not be Google" -> "which is not Google".
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));
Leo ZhaoIf you want to verify when Google is the default search engine, the code behaves in certain way, then you can do line 136 does, set it to Google and then verify. This ASSERT_EQ is asserting that the default search engine is Google. It is using a test to test the default value of the `template_url_service_`, which should not be the focus of this unit test for the mediator.
Angela NovakovicI came up with a good example. Let's say, a person is writing a test that uses dateService.
The test looks like this:
test {
assert dateService.year == 2026
dateService.setYear(2029)
assert dateService.year == 2029
}This test works, until when it is next year, then it stops to work. Here, assuming dateService's year is 2026 isn't a reasonable assumption, since it can change.
However, you can write test like this:
test {
assert dateService.year != nil
dateService.setYear(2029)
assert dateService.year == 2029
}Here, you can assume dateService should provide a valid year for your code to function. This is a reasonable assumption. Since the program is designed to have dateService to provide current year. The year changes, however, it should always provide a year. If it does, that is an issue that we need to take a look.
Ah, I see. You are saying that we shouldn't make assumptions that a current default value will stay the default value, thus it would be better (to avoid future issues) that we set the default search engine to Google.
Google isn't necessarily the default search engine. It just happens to be for this test. Is there any document or comment specifies that Google is the default service engine for this feature? If there are external measures to guard this condition, then you can assume it.
Related details
- Angela Novakovic
- Noémie St-Onge
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. |
Angela Novakovic (Gerrit)
Angela Novakovic voted and added 3 comments![]( "Open in Gerrit")
Votes added by Angela Novakovic
| Commit-Queue | +1 |
3 comments
// Sets the default search engine to a not prepopulated search engine which is
// not be Google."... which is not be Google.", is the "be" before Google a typo?
It was a typo. Thank you!
// Sets the default search engine to a prepopulated search engine which is not
// be Google."which is not be Google" -> "which is not Google".
Done
ASSERT_EQ(SEARCH_ENGINE_GOOGLE,
template_url_service_->GetDefaultSearchProvider()->GetEngineType(
template_url_service_->search_terms_data()));
Leo ZhaoIf you want to verify when Google is the default search engine, the code behaves in certain way, then you can do line 136 does, set it to Google and then verify. This ASSERT_EQ is asserting that the default search engine is Google. It is using a test to test the default value of the `template_url_service_`, which should not be the focus of this unit test for the mediator.
Angela NovakovicI came up with a good example. Let's say, a person is writing a test that uses dateService.
The test looks like this:
test {
assert dateService.year == 2026
dateService.setYear(2029)
assert dateService.year == 2029
}This test works, until when it is next year, then it stops to work. Here, assuming dateService's year is 2026 isn't a reasonable assumption, since it can change.
However, you can write test like this:
test {
assert dateService.year != nil
dateService.setYear(2029)
assert dateService.year == 2029
}Here, you can assume dateService should provide a valid year for your code to function. This is a reasonable assumption. Since the program is designed to have dateService to provide current year. The year changes, however, it should always provide a year. If it does, that is an issue that we need to take a look.
Leo ZhaoAh, I see. You are saying that we shouldn't make assumptions that a current default value will stay the default value, thus it would be better (to avoid future issues) that we set the default search engine to Google.
Google isn't necessarily the default search engine. It just happens to be for this test. Is there any document or comment specifies that Google is the default service engine for this feature? If there are external measures to guard this condition, then you can assume it.
That's a good question. I am not aware of documentation which specifies Google as the default search engine.
I asked Duckie, and it seems to depend on the location of the user:
"In some countries, Google Search is Chrome's default search engine. In others, you may be asked to choose your default search engine."
Related details
- Leo Zhao
- Noémie St-Onge
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leo Zhao (Gerrit)
Leo Zhao voted Code-Review+1![]( "Open in Gerrit")
- Angela Novakovic
- Noémie St-Onge
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Noémie St-Onge (Gerrit)
Noémie St-Onge added 19 comments![]( "Open in Gerrit")
":coordinator",
"//components/search_engines",
"//components/search_engines:test_support",
"//components/strings",
"//ios/chrome/app/strings",
"//ios/chrome/browser/search_engines/model",
"//ios/chrome/browser/search_engines/model:template_url_service_factory",
"//ios/chrome/browser/settings/ui_bundled/clear_browsing_data/coordinator:util",
"//ios/chrome/browser/settings/ui_bundled/clear_browsing_data/public:features",
"//ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/ui",
"//ios/chrome/browser/shared/model/application_context",
"//ios/chrome/browser/shared/model/profile/test",
"//ios/chrome/browser/signin/model",
"//ios/chrome/browser/signin/model:authentication_service",
"//ios/chrome/browser/signin/model:authentication_service_factory",
"//ios/chrome/browser/signin/model:fake_system_identity",
"//ios/chrome/browser/signin/model:fake_system_identity_manager",
"//ios/chrome/browser/signin/model:test_support",
"//ios/chrome/test:test_support",
"//ios/web/public/test",
"//third_party/ocmock",
"//ui/base",I have a feeling that some of these deps are not necessary - can you please double check and remove any unused ones?
@property(nonatomic, weak) id<QuickDeleteOtherDataConsumer> consumer;Missing comment
// Used to get the user's current sign-in status.This comment makes it sound like this is _retrieving_ the status IMO
```suggestion
// Whether the user is signed in.
```
- (NSString*)searchHistorySubtitle {```suggestion
- (NSString*)searchHistoryCellSubtitle {
- ```
const TemplateURL* dse = _templateURLService->GetDefaultSearchProvider();Using non-standard acronyms for variable names is not recommended go/objc-style#naming
return (dse && dse->prepopulate_id() > 0)Can you document why we're looking at the prepopulate ID here? I feel like the concept of prepopulated DSEs is not really clear without more context
return nil;Why is it okay to return nil here? Is it because the cell won't be shown in that case? If so, we should document it with a comment
~QuickDeleteOtherDataMediatorTest() override {
EXPECT_OCMOCK_VERIFY(consumer_);
mediator_.consumer = nil;
[mediator_ disconnect];
mediator_ = nil;
auth_service_ = nullptr;
template_url_service_ = nullptr;
profile_ = nullptr;
}I would override `TearDown` instead and just disconnect the mediator as done in your test project
TemplateURL* dse = nullptr;Same, using non-standard acronyms is not recommended
Why don't we need to call CreateMediator() here as done in the other tests?
Some of these test cases are pretty repetitive, e.g., some are testing the same thing but for a different DSE state. Can you look into parameterizing these tests to reduce duplication?
// Sets the boolean on whether the ViewController should show the Search History
// cell.```suggestion
// Sets whether the "Search history" cell should be shown.
```
// Sets the boolean on whether the ViewController should show the "My Activity"
// cell.```suggestion
// Sets whether the "My Activity" cell should be shown.
```
- (void)setSearchHistorySubtitle:(NSString*)subtitle;```suggestion
- (void)setSearchHistoryCellSubtitle:(NSString*)subtitle;
- ```
// Sets the ViewController with the subtitle for the search history cell.`"Search history"` to be consistent
// Sets the ViewController with the title for the "Quick Delete Other data"nit: we can remove `the ViewController with` from the comments as it adds length to already clear comments
// Consumer for the QuickDeleteOtherDataMediator to set the
// QuickDeleteOtherDataViewController.avoid implementation details in comments
```suggestion
// Consumer for the "Quick Delete Other Data" page.
```
// The subtitle for the search history cell.`"Search history"` for consistency (also applies to other occurrences)
NSString* _searchHistorySubtitle;```suggestion
NSString* _searchHistoryCellSubtitle;
```
Related details
- Angela Novakovic
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. |
Angela Novakovic (Gerrit)
Angela Novakovic voted and added 19 comments![]( "Open in Gerrit")
Votes added by Angela Novakovic
| Commit-Queue | +1 |
19 comments
":coordinator",
"//components/search_engines",
"//components/search_engines:test_support",
"//components/strings",
"//ios/chrome/app/strings",
"//ios/chrome/browser/search_engines/model",
"//ios/chrome/browser/search_engines/model:template_url_service_factory",
"//ios/chrome/browser/settings/ui_bundled/clear_browsing_data/coordinator:util",
"//ios/chrome/browser/settings/ui_bundled/clear_browsing_data/public:features",
"//ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/ui",
"//ios/chrome/browser/shared/model/application_context",
"//ios/chrome/browser/shared/model/profile/test",
"//ios/chrome/browser/signin/model",
"//ios/chrome/browser/signin/model:authentication_service",
"//ios/chrome/browser/signin/model:authentication_service_factory",
"//ios/chrome/browser/signin/model:fake_system_identity",
"//ios/chrome/browser/signin/model:fake_system_identity_manager",
"//ios/chrome/browser/signin/model:test_support",
"//ios/chrome/test:test_support",
"//ios/web/public/test",
"//third_party/ocmock",
"//ui/base",I have a feeling that some of these deps are not necessary - can you please double check and remove any unused ones?
Done
@property(nonatomic, weak) id<QuickDeleteOtherDataConsumer> consumer;Angela NovakovicMissing comment
Done
This comment makes it sound like this is _retrieving_ the status IMO
```suggestion
// Whether the user is signed in.
```
Done
- (NSString*)searchHistorySubtitle {Angela Novakovic```suggestion
- (NSString*)searchHistoryCellSubtitle {
- ```
Done
const TemplateURL* dse = _templateURLService->GetDefaultSearchProvider();Using non-standard acronyms for variable names is not recommended go/objc-style#naming
Done
Can you document why we're looking at the prepopulate ID here? I feel like the concept of prepopulated DSEs is not really clear without more context
Done
Why is it okay to return nil here? Is it because the cell won't be shown in that case? If so, we should document it with a comment
Done
~QuickDeleteOtherDataMediatorTest() override {
EXPECT_OCMOCK_VERIFY(consumer_);
mediator_.consumer = nil;
[mediator_ disconnect];
mediator_ = nil;
auth_service_ = nullptr;
template_url_service_ = nullptr;
profile_ = nullptr;
}I would override `TearDown` instead and just disconnect the mediator as done in your test project
Done
Same, using non-standard acronyms is not recommended
Done
Why don't we need to call CreateMediator() here as done in the other tests?
Technically, I do call CreateMediator() in the setup, but if we go with the assumption that Google might not be the DSE, then yes, it is better to remove it from the setup and add it in the tests. 😊 I've updated that!
Some of these test cases are pretty repetitive, e.g., some are testing the same thing but for a different DSE state. Can you look into parameterizing these tests to reduce duplication?
Done
// Sets the boolean on whether the ViewController should show the Search History
// cell.```suggestion
// Sets whether the "Search history" cell should be shown.
```
Done
// Sets the boolean on whether the ViewController should show the "My Activity"
// cell.```suggestion
// Sets whether the "My Activity" cell should be shown.
```
Done
- (void)setSearchHistorySubtitle:(NSString*)subtitle;Angela Novakovic```suggestion
- (void)setSearchHistoryCellSubtitle:(NSString*)subtitle;
- ```
Done
// Sets the ViewController with the subtitle for the search history cell.`"Search history"` to be consistent
Done
// Sets the ViewController with the title for the "Quick Delete Other data"nit: we can remove `the ViewController with` from the comments as it adds length to already clear comments
Done
// Consumer for the QuickDeleteOtherDataMediator to set the
// QuickDeleteOtherDataViewController.avoid implementation details in comments
```suggestion
// Consumer for the "Quick Delete Other Data" page.
```
Done
`"Search history"` for consistency (also applies to other occurrences)
Done
NSString* _searchHistorySubtitle;Angela Novakovic```suggestion
NSString* _searchHistoryCellSubtitle;
```
Done
Related details
- Noémie St-Onge
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexis Hétu (Gerrit)
Alexis Hétu added 2 comments![]( "Open in Gerrit")
// "Search history" cell is not shown when DSE is null.Is it possible to validate that it's set to nil on error, like so?
`OCMExpect([consumer_ setSearchHistoryCellSubtitle:nil]);`
NSString* _title;Accidental shadowing: A UIViewController already has a `title` properly:
https://developer.apple.com/documentation/uikit/uiviewcontroller/title?language=objc
Please rename this.
Related details
- Angela Novakovic
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. |
Angela Novakovic (Gerrit)
Angela Novakovic added 2 comments![]( "Open in Gerrit")
Is it possible to validate that it's set to nil on error, like so?
`OCMExpect([consumer_ setSearchHistoryCellSubtitle:nil]);`
Yes! Thank you for the suggestion. 😊
Accidental shadowing: A UIViewController already has a `title` properly:
https://developer.apple.com/documentation/uikit/uiviewcontroller/title?language=objc
Please rename this.
- Alexis Hétu
- Filipa Senra
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexis Hétu (Gerrit)
Alexis Hétu voted Code-Review+1![]( "Open in Gerrit")
- Angela Novakovic
- Filipa Senra
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Filipa Senra (Gerrit)
Filipa Senra voted and added 1 comment![]( "Open in Gerrit")
Votes added by Filipa Senra
| Code-Review | +1 |
1 comment
case signin::PrimaryAccountChangeEvent::Type::kNone:nit: should this one be treated as signed out?
Related details
- Angela Novakovic
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. |
Angela Novakovic (Gerrit)
Angela Novakovic added 1 comment![]( "Open in Gerrit")
case signin::PrimaryAccountChangeEvent::Type::kNone:nit: should this one be treated as signed out?
I wouldn't treat it as such. This is for the case when the sign-in status doesn't change. Please see https://source.chromium.org/chromium/chromium/src/+/main:components/signin/public/identity_manager/primary_account_change_event.h;drc=75bd37cd98d8654e740d5b267c1be6fbf81c6ede;l=23
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rainhard Findling (Gerrit)
Rainhard Findling voted and added 1 comment![]( "Open in Gerrit")
Votes added by Rainhard Findling
| Code-Review | +1 |
1 comment
Related details
- Angela Novakovic
- Sylvain Defresne
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sylvain Defresne (Gerrit)
Sylvain Defresne voted and added 1 comment![]( "Open in Gerrit")
Votes added by Sylvain Defresne
| Code-Review | +1 |
1 comment
lgtm
Related details
- Angela Novakovic
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Leo Zhao (Gerrit)
Leo Zhao voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Angela Novakovic (Gerrit)
Angela Novakovic voted Commit-Queue+2![]( "Open in Gerrit")
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")
Change information
[iOS][PRDBD] Implement QuickDeleteOtherDataConsumer
This CL adds the QuickDeleteOtherDataConsumer in order to forward the
correct strings and cell visibility based on the user's default search
engine and sign-in status from the QuickDeleteOtherDataMediator.
A follow-up CL will modify the table view based on the value obtained
from the consumer.
Design Doc: go/bling-delete-browsing-data
- M chrome/app/settings_strings.grdp
- M components/components_strings.grd
- R components/components_strings_grd/IDS_SETTINGS_CLEAR_NON_GOOGLE_SEARCH_HISTORY_NON_PREPOPULATED_DSE.png.sha1
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/coordinator/BUILD.gn
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/coordinator/quick_delete_other_data_coordinator.mm
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/coordinator/quick_delete_other_data_mediator.h
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/coordinator/quick_delete_other_data_mediator.mm
- A ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/coordinator/quick_delete_other_data_mediator_unittest.mm
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/ui/BUILD.gn
- A ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/ui/quick_delete_other_data_consumer.h
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/ui/quick_delete_other_data_view_controller.h
- M ios/chrome/browser/settings/ui_bundled/clear_browsing_data/quick_delete_other_data/ui/quick_delete_other_data_view_controller.mm
- M ios/chrome/test/BUILD.gn
Code-Review: +1 by Leo Zhao, +1 by Sylvain Defresne, +1 by Alexis Hétu, +1 by Rainhard Findling, +1 by Filipa Senra
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |