Add Vector2d classes that represent offsets, instead of using Point.

chrome/browser/ui/gtk/tabs/dragged_tab_controller_gtk.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/gtk/tabs/dragged_tab_controller_gtk.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 489 matching lines @@
   for (size_t i = 0; i < drag_data_->size(); ++i)
     drag_data_->get(i)->contents_->web_contents()->SetDelegate(this);
 
   attached_tabstrip_ = NULL;
 }
 
 gfx::Point DraggedTabControllerGtk::ConvertScreenPointToTabStripPoint(
     TabStripGtk* tabstrip, const gfx::Point& screen_point) {
   gfx::Point tabstrip_screen_point =
       ui::GetWidgetScreenPosition(tabstrip->tabstrip_.get());
-  return screen_point.Subtract(tabstrip_screen_point);
+  gfx::Vector2d tabstrip_offset =

[sky, 2012/10/26 21:37:50]

This results in a lot harder to read code.

[Peter Kasting, 2012/10/26 21:50:12]

I agree, but not because of Vector2d, just because of clumsy conversion.  IMO
this should have been written as:

  gfx::Point tabstrip_screen_point(
      ui::GetWidgetScreenPosition(tabstrip->tabstrip_.get()));
  return screen_point - tabstrip_screen_point.DistanceFromOrigin();

(Optionally, one could inline everything into the return statement.)

This becomes even simpler if we change GetWidgetScreenPosition() to
GetWidgetScreenOffset() and have it return a Vector2d -- which, I suspect, is
what every caller is really going to want.

[danakj, 2012/10/26 21:59:43]

I'll look into that, thanks.

There's probably a few clumsy conversions in here.. I tried to be
careful/meticulous but clearly it can be improved.

[danakj, 2012/10/26 23:30:34]

This certainly does clean up things, such as ConvertWidgetPointToScreen in
gtk_util.cc.

The one place that actually wants a point, is duplicating code from
GetWidgetScreenRect, so switched it to that.

+      screen_point.DistanceFrom(tabstrip_screen_point);
+  return gfx::PointAtDistanceFromOrigin(tabstrip_offset);
 }
 
 gfx::Rect DraggedTabControllerGtk::GetDraggedViewTabStripBounds(
     const gfx::Point& screen_point) {
   gfx::Point client_point =
       ConvertScreenPointToTabStripPoint(attached_tabstrip_, screen_point);
   gfx::Size tab_size = dragged_view_->attached_tab_size();
   return gfx::Rect(client_point.x(), client_point.y(),
                    dragged_view_->GetTotalWidthInTabStrip(), tab_size.height());
 }
@@ skipping 290 matching lines @@
   // We use the requested bounds instead of the allocation because the
   // allocation is relative to the first windowed widget ancestor of the tab.
   // Because of this, we can't use the tabs allocation to get the screen bounds.
   std::vector<TabGtk*> tabs = GetTabsMatchingDraggedContents(
       attached_tabstrip_);
   TabGtk* tab = !base::i18n::IsRTL() ? tabs.front() : tabs.back();
   gfx::Rect bounds = tab->GetRequisition();
   GtkWidget* widget = tab->widget();
   GtkWidget* parent = gtk_widget_get_parent(widget);
   gfx::Point point = ui::GetWidgetScreenPosition(parent);
-  bounds.Offset(point);
+  bounds.Offset(point.DistanceFromOrigin());
 
   return gfx::Rect(bounds.x(), bounds.y(),
                    dragged_view_->GetTotalWidthInTabStrip(), bounds.height());
 }
 
 void DraggedTabControllerGtk::HideWindow() {
   GtkWidget* tabstrip = source_tabstrip_->widget();
   GtkWindow* window = platform_util::GetTopLevel(tabstrip);
   gtk_widget_hide(GTK_WIDGET(window));
 }
@@ skipping 58 matching lines @@
 
 bool DraggedTabControllerGtk::AreTabsConsecutive() {
   for (size_t i = 1; i < drag_data_->size(); ++i) {
     if (drag_data_->get(i - 1)->source_model_index_ + 1 !=
         drag_data_->get(i)->source_model_index_) {
       return false;
     }
   }
   return true;
 }