New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 903262



Sign in to add a comment
link

Issue 903391: Fix declaration of override method in IdentityManagerObserverBridge

Reported by ma...@igalia.com, Nov 8 Project Member

Issue description

IdentityManager::OnRefreshTokenRemovedForAccount() expects as a first parameter a "const std::string&" instead of a "const AccountInfo&", but the override IdentityManagerObserverBridge::OnRefreshTokenRemovedForAccount() declares a const AccountInfo& instead, resulting in the following build failure when trying to use this new class (e.g. in CL 1326151):

    In file included from ../../services/identity/public/objc/identity_manager_observer_bridge.mm:5:
    ../../services/identity/public/objc/identity_manager_observer_bridge.h:52:40: error: non-virtual member function marked 'override' hides virtual member function
          const AccountInfo& account_info) override;                          
                                           ^
    ../../services/identity/public/cpp/identity_manager.h:101:18: note: hidden overloaded virtual function 'identity::IdentityManager::Observer::OnRefreshTokenRemovedForAccount' declared here: type mismatch at 1st p
    arameter ('const std::string &' (aka 'const basic_string<char, char_traits<char>, allocator<char> > &') vs 'const AccountInfo &')
        virtual void OnRefreshTokenRemovedForAccount(                                                
                     ^
    1 error generated.
    [3/12] OBJCXX obj/ios/chrome/browser/ui/bookmarks/bookmarks/bookmark_promo_controller.o
    FAILED: obj/ios/chrome/browser/ui/bookmarks/bookmarks/bookmark_promo_controller.o


This new IdentityManagerObserverBridge API has been recently introduced as part of CL 1203934, but nothing uses it yet so I suppose that's why this run undetected until I tried to use it now to migrate bookmark_promo_controller.mm as part of CL 1326151.

Filing this issue so that I can attach a CL for it that I need to migrate bookmark_promo_controller.mm.

//cc @blundell & @jlebel
 

Comment 2 by bugdroid1@chromium.org, Nov 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6586665e93e9211f934364c00eee32433027df9

commit b6586665e93e9211f934364c00eee32433027df9
Author: Mario Sanchez Prada <mario@igalia.com>
Date: Fri Nov 09 10:36:59 2018

Fix declaration of override method in IdentityManagerObserverBridge

IdentityManager::OnRefreshTokenRemovedForAccount() expects as a first
parameter a "const std::string&" instead of a "const AccountInfo&",
so this patch fixes it to prevent build failures when using this API
because of OnRefreshTokenRemovedForAccount(const AccountInfo&) not
being a valid override for the virtual method declared.

Bug:  903391 
Change-Id: I46cd6778240a40af6863ca7fd3b8d9a88e735fde
Reviewed-on: https://chromium-review.googlesource.com/c/1327161
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#606787}
[modify] https://crrev.com/b6586665e93e9211f934364c00eee32433027df9/services/identity/public/objc/identity_manager_observer_bridge.h
[modify] https://crrev.com/b6586665e93e9211f934364c00eee32433027df9/services/identity/public/objc/identity_manager_observer_bridge.mm

Comment 3 by ma...@igalia.com, Nov 9

Status: Fixed (was: Started)

Sign in to add a comment