Clarify the documentation for loading_only on TabRenderer::UpdateData.

chrome/browser/tabs/tab_strip_model.h

 // Copyright (c) 2006-2008 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_TABS_TAB_STRIP_MODEL_H_
 #define CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H_
 
 #include <vector>
 
 #include "base/observer_list.h"
@@ skipping 30 matching lines @@
 //
 ////////////////////////////////////////////////////////////////////////////////
 class TabStripModelObserver {
  public:
   // A new TabContents was inserted into the TabStripModel at the specified
   // index. |foreground| is whether or not it was opened in the foreground
   // (selected).
   virtual void TabInsertedAt(TabContents* contents,
                              int index,
                              bool foreground) { }
+
   // The specified TabContents at |index| is being closed (and eventually
   // destroyed).
   virtual void TabClosingAt(TabContents* contents, int index) { }
+
   // The specified TabContents at |index| is being detached, perhaps to be
   // inserted in another TabStripModel. The implementer should take whatever
   // action is necessary to deal with the TabContents no longer being present.
   virtual void TabDetachedAt(TabContents* contents, int index) { }
+
   // The selected TabContents changed from |old_contents| to |new_contents| at
   // |index|. |user_gesture| specifies whether or not this was done by a user
   // input event (e.g. clicking on a tab, keystroke) or as a side-effect of
   // some other function.
   virtual void TabSelectedAt(TabContents* old_contents,
                              TabContents* new_contents,
                              int index,
                              bool user_gesture) { }
+
   // The specified TabContents at |from_index| was moved to |to_index|.
   virtual void TabMoved(TabContents* contents,
                         int from_index,
                         int to_index) { }
+
   // The specified TabContents at |index| changed in some way. |contents| may
   // be an entirely different object and the old value is no longer available
   // by the time this message is delivered.
+  //
+  // If only the loading state was updated, the loading_only flag should be
+  // specified. If other things change, set this flag to false to update

[pink (ping after 24hrs), 2009/04/07 13:29:27]

This seems a little backwards. The client implementing this API won't be setting
|loading_only|. Maybe something like "If other things change, this flag will be
set to false indicating everything should be updated". You can twiddle with it
if you want.

[brettw, 2009/04/07 16:59:31]

I think that comments should generally be written from the perspective of the
caller. You're looking at this as the implementor, but in the future, there will
be people looking at the comment from both the caller's and the implementor's
perspective.

I will add a little more clarification about what should happen.

+  // everything. This allows us to start/stop throbbing without updating the
+  // title (which may be an ugly URL if the real title hasn't come in yet).
   virtual void TabChangedAt(TabContents* contents, int index,
                             bool loading_only) { }
+
   // The TabStripModel now no longer has any "significant" (user created or
   // user manipulated) tabs. The implementer may use this as a trigger to try
   // and close the window containing the TabStripModel, for example...
   virtual void TabStripEmpty() { }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 //
 // TabStripModelDelegate
 //
@@ skipping 457 matching lines @@
   TabStripModelOrderController* order_controller_;
 
   // Our observers.
   typedef ObserverList<TabStripModelObserver> TabStripModelObservers;
   TabStripModelObservers observers_;
 
   DISALLOW_COPY_AND_ASSIGN(TabStripModel);
 };
 
 #endif  // CHROME_BROWSER_TABS_TAB_STRIP_MODEL_H_