Elides the beginning of tab titles that have common prefixes. ...

chrome/browser/ui/title_prefix_matcher.h

+// 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_UI_TITLE_PREFIX_MATCHER_H_
+#define CHROME_BROWSER_UI_TITLE_PREFIX_MATCHER_H_
+#pragma once
+
+#include <vector>
+
+#include "base/string16.h"
+
+namespace browser {

[sky, 2011/03/11 00:00:45]

We generally don't use namespaces in cases like this.

[MAD, 2011/03/11 03:01:37]

Done.

+
+// This exposes a static method that receives a vector of TitleInfo objects
+// so that it can find the length of the common prefixes among all the titles.

[sky, 2011/03/11 00:00:45]

Imagine if you didn't write this class and went to use it on the mac and/or
linux. Is there enough info here to tell you how to use it? For example,
TitleInfo::caller_value isn't described, nor do you document what prefix_length
is. Could you clean this up? You should also document what this is intended to
be used for.

[MAD, 2011/03/11 03:01:37]

Done.

+class TitlePrefixMatcher {
+ public:
+  struct TitleInfo {
+    TitleInfo(const string16* title, int caller_value)
+        : title(title), prefix_length(0), caller_value(caller_value) {}

[sky, 2011/03/11 00:00:45]

Since you wrapper, each param should be on its own line.
How a DCHECK(title) too.

[MAD, 2011/03/11 03:01:37]

Done.

+    TitleInfo(const TitleInfo& other)

[sky, 2011/03/11 00:00:45]

How come you wrote copy constructor/assignment operator. For this class aren't
the compiler generated ones what you want?

[MAD, 2011/03/11 03:01:37]

Done.
This was needed in a previous iteration when the title was a const& and I was
allocating the TitleInfo... When I changed it to not allocating the TitleInfo,
it needed these, but I could find a way to make it work with const string16&, so
I changed it to a pointer and forgot to remove these... Thanks for mentioning
it... :-)

+        : title(other.title),
+          prefix_length(other.prefix_length),
+          caller_value(other.caller_value) {
+    }
+    TitleInfo& operator=(const TitleInfo& other) {
+      title = other.title;
+      prefix_length = other.prefix_length;
+      caller_value = other.caller_value;
+      return *this;
+    }
+    // We assume the title string will be valid throughout the execution of
+    // the prefix lengths calculation, and use a pointer to avoid an uncessary
+    // string copy.
+    const string16* title;
+    size_t prefix_length;
+    int caller_value;  // Unused by CalculatePrefixLengths.
+  };
+  static void CalculatePrefixLengths(std::vector<TitleInfo>* title_infos);
+
+  DISALLOW_COPY_AND_ASSIGN(TitlePrefixMatcher);

[sky, 2011/03/11 00:00:45]

after private. But you probably want DISALLOW_IMPLICIT_CONSTRUCTORS.

[MAD, 2011/03/11 03:01:37]

Done.

+};
+
+}  // namespace browser
+
+#endif  // CHROME_BROWSER_UI_TITLE_PREFIX_MATCHER_H_