Standardize usage of virtual/override/final in chrome/browser/ui/

chrome/browser/ui/android/tab_model/tab_model.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_UI_ANDROID_TAB_MODEL_TAB_MODEL_H_
 #define CHROME_BROWSER_UI_ANDROID_TAB_MODEL_TAB_MODEL_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/sync/glue/synced_window_delegate.h"
 #include "chrome/browser/ui/toolbar/toolbar_model.h"
@@ skipping 38 matching lines @@
 
   // Used by Developer Tools to create a new tab with a given URL.
   // Replaces CreateTabForTesting.
   virtual content::WebContents* CreateNewTabForDevTools(const GURL& url) = 0;
 
   // Return true if we are currently restoring sessions asynchronously.
   virtual bool IsSessionRestoreInProgress() const = 0;
 
  protected:
   explicit TabModel(Profile* profile);
-  virtual ~TabModel();
+  ~TabModel() override;

[msw, 2014/10/21 19:36:11]

Why does this seem to operate contrary to the Google C++ style guide?
"Make your destructor virtual if necessary. If your class has virtual methods,
its destructor should be virtual."
Is the actual "virtual" keyword no longer necessary to mark the destructor
virtual if the "override" keyword is used?

[dcheng, 2014/10/21 20:14:53]

From http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance
Explicitly annotate overrides of virtual functions or virtual destructors with
an override or (less frequently) final specifier.

You're not the only one that's expressed surprise at this, and I raised this
issue on chromium-dev. However, the wording seems unambiguous and the general
consensus seems to be that we should follow the Google style.

 
   // Instructs the TabModel to broadcast a notification that all tabs are now
   // loaded from storage.
   void BroadcastSessionRestoreComplete();
 
   ToolbarModel* GetToolbarModel();
 
  private:
   // Determines how TabModel will interact with the profile.
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) override;
+  void Observe(int type,

[msw, 2014/10/21 19:36:11]

I guess this is implementing the following policy? "For clarity, use exactly one
of override, final, or virtual when declaring an override."

[dcheng, 2014/10/21 20:14:53]

Yep.

+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) override;
 
   // The profile associated with this TabModel.
   Profile* profile_;
 
   // Describes if this TabModel contains an off-the-record profile.
   bool is_off_the_record_;
 
   // The SyncedWindowDelegate associated with this TabModel.
   scoped_ptr<browser_sync::SyncedWindowDelegateAndroid> synced_window_delegate_;
 
   // Unique identifier of this TabModel for session restore. This id is only
   // unique within the current session, and is not guaranteed to be unique
   // across sessions.
   SessionID session_id_;
 
   // The Registrar used to register TabModel for notifications.
   content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(TabModel);
 };
 
 #endif  // CHROME_BROWSER_UI_ANDROID_TAB_MODEL_TAB_MODEL_H_