I'm not convinced this is something we want to fix, because this is a pattern that's used all over the codebase.  Let's discuss the specific motivation behind this bug (and loop in some //content folks) before we make any changes.

Avi, the tab helper docs specifically say to use WebContentsUserData + WebContentsObserver, but the style guide bans multiple inheritance.  Is this a case where we should ask for an exception?  Or should we make a common base class to satisfy the style guide?

https://chromium.googlesource.com/chromium/src/+/master/docs/tab_helpers.md#Adding-a-feature-to-a-browser-tab

It looks like the Chromium C++ style guide allows (but discourages) multiple inheritance: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md

If the classes being multiply-inherited from are small and intended as mix-ins, is it a real issue?

+ Brett

As some background, this is not a high priority issue; I just noticed that we had several style guide violations in some of our tab helper implementations on iOS, which is a pattern we've only started using, so I created this bug tree to track these violations.  As far as I can tell, multiple inheritance hasn't caused any issues here.  It just seems that multiple inheritance is generally a C++ anti-pattern that we should avoid.

Brett & Avi: Do you have any context as to why multiple inheritance is prohibited by the Google style guide, but only discouraged by the chromium guide?  I'm not sure if this is an issue worth putting in the effort to fix, as I understand that the pattern is widely used in chromium code for other platforms, but I'd like to get some discussion to help better understand our approach here.

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). 

BTW, I read the google style guide as prohibiting "implementation multiple inheritance" (i.e. mix-in) and not prohibiting having a class implementing multiple interfaces (and WebStateObserver is an interface class even though it lack the Interface and has default empty implementation for the events method to allow only implementing the one we are interested in).

I think that those classes are in the spirit of the style guide (i.e. no "multiple inheritance to get the implementation of all super-class" but "multiple inheritance to declare conformance to multiple interfaces") even though they don't conform to the letter (i.e. interfaces are not named "FooInterface").

kkhorimoto@ - feel free to close/re-assign once it is decided if we are okay with the current code or not.