New issue
Advanced search Search tips

Issue 701126 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Task

Blocking:
issue 701095



Sign in to add a comment

Eliminate multiple inheritance style guide violation for WebFaviconDriver

Project Member Reported by kkhorimoto@chromium.org, Mar 13 2017

Issue description

Multiple inheritance is prohibited by the style guide unless only one superclass has an implementation.  All other superclasses must be pure interfaces with an "Interface" suffix: https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance
 
Cc: rohitrao@chromium.org
[Bulk Edit]

I'm not sure those bugs are worth fixing.

First our observer classes are interface-ish classes (they are not named XXXInterface because it make much more sense to name them XXXObserver) and if they have implementation for their method they are empty (i.e. you never have to call the super class implementation in the derived class). So those inheritance are not cases of implementation inheritance.

Second, those classes (e.g. WebFaviconDriver) are designed to mimic their chrome/ counterpart (e.g. ContentFaviconDriver) and removing the multiple inheritance from the ios/chrome/ implementation would need to be done in sync with removing the inheritance from the chrome/ class too.

Finally some of the bugs are incorrect as the classes inheritance correspond to what is allowed by the style guide (e.g. IOSChromeSyncedTabDelegate inherits from sync_sessions::SyncedTabDelegate and web::WebStateUserData<IOSChromeSyncedTabDelegate> where sync_sessions::SyncedTabDelegate is a pure interface). 
Components: Mobile>WebView>Glue
Labels: Hotlist-Refactoring
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
kkhorimoto@ - feel free to close/re-assign once it is decided if we are okay with the current code or not.
Components: -Mobile>WebView>Glue Internals
Labels: -Type-Bug Type-Task
Status: WontFix (was: Assigned)

Sign in to add a comment