Migrate bookmark_promo_controller.mm to use IdentityManagerObserverBridge |
||||
Issue descriptionAs suggested by @blundell in [1], it would be nice to migrate bookmark_promo_controller.mm to using IdentityManagerObserverBridge instead of having its own implementation of an IdentityManager::Observer to pass the relevant events to the Obj-C class BookmarkPromoController. Since this might involve a few more changes than I was initially expecting, I'm splitting it of to its own ticket instead of reusing crbug.com/890824 . [1] https://chromium-review.googlesource.com/c/chromium/src/+/1315213#message-8ecdddc7db75509b9d94ed349a0a590a2255a01e
,
Nov 8
,
Nov 8
,
Nov 8
> ...ideally one that would also provide unit for that class, if possible. What do you think? > > Also, if agreed... any suggestion on how to implement such unit tests would be great too, thanks! @blundell After spending some time today and realizing this would not be straightforward for me to do, I split the implementation of that unit test to a different ticket so that it can be prioritized independently from this other ticket. See https://bugs.chromium.org/p/chromium/issues/detail?id=903396
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/add4a3fc779e84faf4d4e8757a009db8c3338026 commit add4a3fc779e84faf4d4e8757a009db8c3338026 Author: Mario Sanchez Prada <mario@igalia.com> Date: Tue Nov 13 19:29:08 2018 [ios] Migrate bookmark_promo_controller.mm to use IdentityManagerObserverBridge Remove the custom C++ class IdentityManagerObserver and rely on the new API from //services/identity/public/objc instead. Bug: 903262 Change-Id: I32c3d46bbd6c1cd45c092c859ef303d9cd6b240f Reviewed-on: https://chromium-review.googlesource.com/c/1326151 Commit-Queue: Mario Sanchez Prada <mario@igalia.com> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Sergio Collazos <sczs@chromium.org> Cr-Commit-Position: refs/heads/master@{#607701} [modify] https://crrev.com/add4a3fc779e84faf4d4e8757a009db8c3338026/ios/chrome/browser/ui/bookmarks/BUILD.gn [modify] https://crrev.com/add4a3fc779e84faf4d4e8757a009db8c3338026/ios/chrome/browser/ui/bookmarks/bookmark_promo_controller.mm
,
Nov 13
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/273d06b686217c636a8647761433643a7f5d881e commit 273d06b686217c636a8647761433643a7f5d881e Author: Mario Sanchez Prada <mario@igalia.com> Date: Tue Nov 27 18:48:15 2018 Remove unnecessary reset for identity::IdentityManagerObserverBridge As discussed in the review for CL1326151, there is no need to do this explicitly on dealloc() since the std::unique_ptr for this object will be destroyed as part of the deallocation. Thus, we're removing the call to .reset() to prevent further confusion. Bug: 903262 Change-Id: Iec51c701501de948d32ce557ca37a7850db7b107 Reviewed-on: https://chromium-review.googlesource.com/c/1337611 Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Sergio Collazos <sczs@chromium.org> Commit-Queue: Mario Sanchez Prada <mario@igalia.com> Cr-Commit-Position: refs/heads/master@{#611223} [modify] https://crrev.com/273d06b686217c636a8647761433643a7f5d881e/ios/chrome/browser/ui/bookmarks/bookmark_promo_controller.mm |
||||
►
Sign in to add a comment |
||||
Comment 1 by ma...@igalia.com
, Nov 8