Part 1 of repairing regressions to my old clang check plugins so Nico can

chrome/browser/tab_contents/web_navigation_observer.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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_TAB_CONTENTS_WEB_NAVIGATION_OBSERVER_H_
 #define CHROME_BROWSER_TAB_CONTENTS_WEB_NAVIGATION_OBSERVER_H_
 
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "ipc/ipc_channel.h"
 
 struct ViewHostMsg_FrameNavigate_Params;
 
 // An observer API implemented by classes which are interested in various page
 // load events from TabContents.  They also get a chance to filter IPC messages.
 class WebNavigationObserver : public IPC::Channel::Listener {
  public:

[Nico, 2011/01/27 03:55:17]

no constructor/destructor here?

[Elliot Glaysher, 2011/01/27 18:48:05]

Correct. This is a base class whose base class already has declared virtualness
of the dtor. No member vars on this class, so as long as the subclasses have
their c/dtors ool this should be good.

   virtual void NavigateToPendingEntry() { }
 
   virtual void DidNavigateMainFramePostCommit(
       const NavigationController::LoadCommittedDetails& details,
       const ViewHostMsg_FrameNavigate_Params& params) { }
   virtual void DidNavigateAnyFramePostCommit(
       const NavigationController::LoadCommittedDetails& details,
       const ViewHostMsg_FrameNavigate_Params& params) { }
 
   virtual void DidStartLoading() { }
   virtual void DidStopLoading() { }
 
   // IPC::Channel::Listener implementation.
-  virtual bool OnMessageReceived(const IPC::Message& message) { return false; }

[Nico, 2011/01/27 03:55:17]

this looks harmless too?

[Elliot Glaysher, 2011/01/27 18:48:05]

See this code review for discussion between me and Peter:
http://codereview.chromium.org/5741001/

After doing that bit of measurement, I've adopted a policy of no non-empty
virtual methods (since most compilers have a common case for that) and check for
that in the plugin. There are probably a few cases where you are correct but I'm
now doing this for consistency so I can automate the check.  (though this is
probably not one of them since the derived class hierarchy here is wide instead
of deep.)

+  virtual bool OnMessageReceived(const IPC::Message& message);
 
 #if 0
   // For unifying with delegate...
 
   // Notifies the delegate that this contents is starting or is done loading
   // some resource. The delegate should use this notification to represent
   // loading feedback. See TabContents::is_loading()
   virtual void LoadingStateChanged(TabContents* contents) { }
   // Called to inform the delegate that the tab content's navigation state
   // changed. The |changed_flags| indicates the parts of the navigation state
   // that have been updated, and is any combination of the
   // |TabContents::InvalidateTypes| bits.
   virtual void NavigationStateChanged(const TabContents* source,
                                       unsigned changed_flags) { }
 #endif
 };
 
 #endif  // CHROME_BROWSER_TAB_CONTENTS_WEB_NAVIGATION_OBSERVER_H_