Dragging panels near screen edge

chrome/browser/extensions/api/tabs/tabs_api.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/extensions/api/tabs/tabs_api.h"
 
 #include <algorithm>
 #include <limits>
 #include <vector>
 
@@ skipping 742 matching lines @@
   else
     bounds = controller->window()->GetBounds();
   bool set_bounds = false;
 
   // Any part of the bounds can optionally be set by the caller.
   int bounds_val;
   if (update_props->HasKey(keys::kLeftKey)) {
     EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
         keys::kLeftKey,
         &bounds_val));
-    bounds.set_x(bounds_val);
-    set_bounds = true;
+    if (bounds.x() != bounds_val) {
+      bounds.set_x(bounds_val);
+      set_bounds = true;
+    }
   }
 
   if (update_props->HasKey(keys::kTopKey)) {
     EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
         keys::kTopKey,
         &bounds_val));
-    bounds.set_y(bounds_val);
-    set_bounds = true;
+    if (bounds.y() != bounds_val) {
+      bounds.set_y(bounds_val);
+      set_bounds = true;
+    }
   }
 
   if (update_props->HasKey(keys::kWidthKey)) {
     EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
         keys::kWidthKey,
         &bounds_val));
-    bounds.set_width(bounds_val);
-    set_bounds = true;
+    if (bounds.width() != bounds_val) {
+      bounds.set_width(bounds_val);
+      set_bounds = true;
+    }
   }
 
   if (update_props->HasKey(keys::kHeightKey)) {
     EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
         keys::kHeightKey,
         &bounds_val));
-    bounds.set_height(bounds_val);
-    set_bounds = true;
+    if (bounds.height() != bounds_val) {
+      bounds.set_height(bounds_val);
+      set_bounds = true;
+    }
   }
 
   if (set_bounds) {
     if (show_state == ui::SHOW_STATE_MINIMIZED ||
         show_state == ui::SHOW_STATE_MAXIMIZED ||
         show_state == ui::SHOW_STATE_FULLSCREEN) {
       error_ = keys::kInvalidWindowStateError;
       return false;
     }
     controller->window()->SetBounds(bounds);

[flackr, 2013/06/19 15:26:46]

Avoiding this SetBounds call if the values don't change may work for hangouts,
but I suspect if an application actually changes its bounds (uses a value other
than its current bounds) during startup the error will still occur. I think the
right thing to do is either revert the drag if we see a setbounds call during a
drag or prevent the bounds update if the window is being dragged. I think the
former makes more sense.

[varkha, 2013/06/19 15:49:22]

True, but this has wider UX impact and probably no easy answer. For example when
only the size changes (especially if it increases or shrinks but only
vertically) reverting the drag may not be what a user would expect. Preventing
bounds update may also be unexpected. In reality most drags would be fine so
closing most glaring troubles could work.
Because this is not bulletproof I had to set the attached property in the
Relayout.

[flackr, 2013/06/19 16:21:45]

Since this should be fine for the simple case where none of the values are
changed we can do this, but we should add a TODO with a bug and start a
discussion about what the right thing to do in the general case is, when the
values actually are changed.

[varkha, 2013/06/19 17:54:09]

Done. See http://crbug.com/251813.

   }
 
   bool active_val = false;
   if (update_props->HasKey(keys::kFocusedKey)) {
     EXTENSION_FUNCTION_VALIDATE(update_props->GetBoolean(
         keys::kFocusedKey, &active_val));
     if (active_val) {
       if (show_state == ui::SHOW_STATE_MINIMIZED) {
         error_ = keys::kInvalidWindowStateError;
         return false;
@@ skipping 1258 matching lines @@
   execute_tab_id_ = tab_id;
   details_ = details.Pass();
   return true;
 }
 
 bool TabsInsertCSSFunction::ShouldInsertCSS() const {
   return true;
 }
 
 }  // namespace extensions