Compute the correct maximum contents width when eliding away the description in

chrome/browser/ui/omnibox/omnibox_popup_model.cc

 // 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.
 
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
 
 #include <algorithm>
 
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 33 matching lines @@
 }
 
 // static
 void OmniboxPopupModel::ComputeMatchMaxWidths(int contents_width,
                                               int separator_width,
                                               int description_width,
                                               int available_width,
                                               bool allow_shrinking_contents,
                                               int* contents_max_width,
                                               int* description_max_width) {
-  if (available_width <= 0) {
-    *contents_max_width = 0;
-    *description_max_width = 0;
-    return;
-  }
+  available_width = std::max(available_width, 0);
+  *contents_max_width = std::min(contents_width, available_width);

[Mark P, 2015/04/22 18:29:12]

This doesn't make sense; you immediately override this value on the next line.

Did you mean the left side of the equals to be contents_width, not
*contents_max_width?

If so, this error indicates you should test this changelist better.

[Peter Kasting, 2015/04/22 23:08:06]

Yes, this was a typo when splitting this change out of my larger change.  I'll
fix this.
The tests here already failed (likely because of this) :P

 
   *contents_max_width = contents_width;
   *description_max_width = description_width;
 
   // If the description is empty, the contents can get the full width.
   if (!description_width)
     return;
 
   // If we want to display the description, we need to reserve enough space for
   // the separator.
   available_width -= separator_width;
+  if (available_width < 0) {
+    *description_max_width = 0;
+    return;
+  }
 
   if (contents_width + description_width > available_width) {
     if (allow_shrinking_contents) {
       // Try to split the available space fairly between contents and
       // description (if one wants less than half, give it all it wants and
       // give the other the remaining space; otherwise, give each half).
       // However, if this makes the contents too narrow to show a significant
       // amount of information, give the contents more space.
       *contents_max_width = std::max(
           (available_width + 1) / 2, available_width - description_width);
 
       const int kMinimumContentsWidth = 300;
-      *contents_max_width = std::min(
-          std::max(*contents_max_width, kMinimumContentsWidth), contents_width);
+      *contents_max_width = std::min(std::min(
+          std::max(*contents_max_width, kMinimumContentsWidth), contents_width),
+          available_width);
     }
 
     // Give the description the remaining space, unless this makes it too small
     // to display anything meaningful, in which case just hide the description
     // and let the contents take up the whole width.
-    *description_max_width = available_width - *contents_max_width;
+    *description_max_width =
+        std::min(description_width, available_width - *contents_max_width);
     const int kMinimumDescriptionWidth = 75;
     if (*description_max_width <
         std::min(description_width, kMinimumDescriptionWidth)) {
       *description_max_width = 0;
-      *contents_max_width = contents_width;
+      // Since we're not going to display the description, the contents can have
+      // the space we reserved for the separator.
+      available_width += separator_width;
+      *contents_max_width = std::min(contents_width, available_width);
     }
   }
 }
 
 bool OmniboxPopupModel::IsOpen() const {
   return view_->IsOpen();
 }
 
 void OmniboxPopupModel::SetHoveredLine(size_t line) {
   const bool is_disabling = (line == kNoMatch);
@@ skipping 185 matching lines @@
   }
 }
 
 void OmniboxPopupModel::AddObserver(OmniboxPopupModelObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void OmniboxPopupModel::RemoveObserver(OmniboxPopupModelObserver* observer) {
   observers_.RemoveObserver(observer);
 }