Revert "Add an optional boolean parameter in boundsForRange"

This reverts commit 7d96ee5f.

Reason for revert:

This broke assertions we make in the extensions docserver causing the
docserver to stop updating. Several places (including extension
bindings) depend on callbacks always being the last parameter to a
function.

Original change's description:
> Add an optional boolean parameter in boundsForRange
>
> Current boundsForRange will clip the bounds to ancestors. This CL adds an
> optional boolean parameter, clipped, to the boundsForRange method. The
> default value of the parameter is true. When the clipped is set to false, the
> bounds will not be clipped to the ancestors of the nodes.
>
> This CL is a part of the fix for bug 775659.
>
> Bug: 775659
> Change-Id: I804637a2e6a8ec695dc176b88bd55d12fbc7f8a0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335390
> Commit-Queue: Lei Shi <leileilei@google.com>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#795339}

TBR=dmazzoni@chromium.org,dtseng@chromium.org,leileilei@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 775659, 1114299
Change-Id: If9f19eb87f798afc10734cccd8b533ecd36bdacd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343876
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796202}

chrome/test/data/extensions/api_test/automation/tests/tabs/bounds_for_range.js

@@ -116,53 +116,6 @@ var allTests = [
       assertEq(hiddenParent.location.height, hiddenBounds.height);
     });
 
-    chrome.test.succeed();
-  },
-
-  function boundsForRangeUnclipped() {
-    let overflowText = rootNode.find(
-        { role: 'inlineTextBox', attributes: { name: 'This text overflows' } });
-    overflowText.boundsForRange(
-        0,
-        overflowText.name.length,
-        (unclippedBounds) => {
-          assertTrue(
-              overflowText.parent.location.width <
-              overflowText.unclippedLocation.width
-          );
-          // Since bounds may differ in different platforms, we set a 2 px
-          // tolerance.
-          assertTrue(
-              Math.abs(
-                  overflowText.unclippedLocation.width - unclippedBounds.width
-              ) <= 2
-          );
-        },
-        false /* clipped */
-    );
-
-    // The static text parent has 4 children, one for each word. The small box
-    // size causes the words to be laid out on different lines, creating four
-    // individual inlineTextBox nodes.
-    let hiddenParent = rootNode.find(
-        { role: 'staticText', attributes: { name: 'This text is hidden' } });
-    assertEq(hiddenParent.children.length, 4);
-    let hiddenChild = hiddenParent.children[0];
-    hiddenChild.boundsForRange(0, hiddenChild.name.length, (hiddenBounds) => {
-      assertTrue(
-          hiddenParent.location.width < hiddenChild.unclippedLocation.width);
-      assertTrue(
-          hiddenParent.location.height < hiddenChild.unclippedLocation.height);
-      // Since bounds may differ in different platforms, we set a 2 px
-      // tolerance.
-      assertTrue(
-          Math.abs(
-              hiddenChild.unclippedLocation.width - hiddenBounds.width
-          ) <= 2
-      );
-      assertEq(hiddenChild.unclippedLocation.height, hiddenBounds.height);
-    }, false /* clipped */);
-
     chrome.test.succeed();
   }
 ];

extensions/common/api/automation.idl

@@ -729,14 +729,9 @@ enum EventMoveDirectionType {
     // Determines the location of the text within the node specified by
     // |startIndex| and |endIndex|, inclusively. Invokes |callback| with the
     // bounding rectangle, in screen coordinates. |callback| can be invoked
-    // either synchronously or asynchronously. By default, |clipped| is true.
-    // If |clipped| is set to false, the bounds will not be clipped to the 
-    // ancestors of the node.
-    static void boundsForRange(
-        long startIndex, 
-        long endIndex, 
-        BoundsForRangeCallback callback, 
-        optional boolean clipped);
+    // either synchronously or asynchronously.
+    static void boundsForRange(long startIndex, long endIndex,
+        BoundsForRangeCallback callback);
 
     // The location (as a bounding box) of this node in global screen
     // coordinates without applying any clipping from ancestors.

extensions/renderer/api/automation/automation_internal_custom_bindings.cc

@@ -305,8 +305,7 @@ typedef void (*NodeIDPlusRangeFunction)(v8::Isolate* isolate,
                                         AutomationAXTreeWrapper* tree_wrapper,
                                         ui::AXNode* node,
                                         int start,
-                                        int end,
-                                        bool clipped);
+                                        int end);
 
 class NodeIDPlusRangeWrapper
     : public base::RefCountedThreadSafe<NodeIDPlusRangeWrapper> {
@@ -317,8 +316,8 @@ class NodeIDPlusRangeWrapper
 
   void Run(const v8::FunctionCallbackInfo<v8::Value>& args) {
     v8::Isolate* isolate = automation_bindings_->GetIsolate();
-    if (args.Length() < 5 || !args[0]->IsString() || !args[1]->IsNumber() ||
-        !args[2]->IsNumber() || !args[3]->IsNumber() || !args[4]->IsBoolean()) {
+    if (args.Length() < 4 || !args[0]->IsString() || !args[1]->IsNumber() ||
+        !args[2]->IsNumber() || !args[3]->IsNumber()) {
       ThrowInvalidArgumentsException(automation_bindings_);
     }
 
@@ -329,7 +328,6 @@ class NodeIDPlusRangeWrapper
     int node_id = args[1]->Int32Value(context).FromMaybe(0);
     int start = args[2]->Int32Value(context).FromMaybe(0);
     int end = args[3]->Int32Value(context).FromMaybe(0);
-    bool clipped = args[4]->BooleanValue(isolate);
 
     AutomationAXTreeWrapper* tree_wrapper =
         automation_bindings_->GetAutomationAXTreeWrapperFromTreeID(tree_id);
@@ -340,8 +338,7 @@ class NodeIDPlusRangeWrapper
     if (!node)
       return;
 
-    function_(isolate, args.GetReturnValue(), tree_wrapper, node, start, end,
-              clipped);
+    function_(isolate, args.GetReturnValue(), tree_wrapper, node, start, end);
   }
 
  private:
@@ -932,7 +929,7 @@ void AutomationInternalCustomBindings::AddRoutes() {
       "GetBoundsForRange",
       [](v8::Isolate* isolate, v8::ReturnValue<v8::Value> result,
          AutomationAXTreeWrapper* tree_wrapper, ui::AXNode* node, int start,
-         int end, bool clipped) {
+         int end) {
         if (node->data().role != ax::mojom::Role::kInlineTextBox)
           return;
 
@@ -976,9 +973,8 @@ void AutomationInternalCustomBindings::AddRoutes() {
         // Convert from local to global coordinates second, after subsetting,
         // because the local to global conversion might involve matrix
         // transformations.
-        gfx::Rect global_bounds =
-            ComputeGlobalNodeBounds(tree_wrapper, node, local_bounds, nullptr,
-                                    clipped /* clip_bounds */);
+        gfx::Rect global_bounds = ComputeGlobalNodeBounds(
+            tree_wrapper, node, local_bounds, nullptr, true /* clip_bounds */);
         result.Set(RectToV8Object(isolate, global_bounds));
       });
 

extensions/renderer/api/automation/automation_internal_custom_bindings.h

@@ -161,8 +161,7 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler {
                        AutomationAXTreeWrapper* tree_wrapper,
                        ui::AXNode* node,
                        int start,
-                       int end,
-                       bool clipped));
+                       int end));
   void RouteNodeIDPlusStringBoolFunction(
       const std::string& name,
       std::function<void(v8::Isolate* isolate,

extensions/renderer/resources/automation/automation_node.js

@@ -618,7 +618,7 @@ AutomationNodeImpl.prototype = {
     return GetLocation(this.treeID, this.id);
   },
 
-  boundsForRange: function(startIndex, endIndex, callback, opt_clipped) {
+  boundsForRange: function(startIndex, endIndex, callback) {
     if (!this.rootImpl)
       return;
 
@@ -632,11 +632,8 @@ AutomationNodeImpl.prototype = {
 
     if (!GetBoolAttribute(this.treeID, this.id, 'supportsTextLocation')) {
       try {
-        // opt_clipped is default to true. Only pass it as false when it is
-        // set to false.
         callback(
-            GetBoundsForRange(this.treeID, this.id, startIndex, endIndex,
-                opt_clipped !== false));
+            GetBoundsForRange(this.treeID, this.id, startIndex, endIndex));
         return;
       } catch (e) {
         logging.WARNING('Error with bounds for range callback' + e);

third_party/closure_compiler/externs/automation.js

@@ -1028,15 +1028,13 @@ chrome.automation.AutomationNode.prototype.location;
  * Determines the location of the text within the node specified by |startIndex|
  * and |endIndex|, inclusively. Invokes |callback| with the bounding rectangle,
  * in screen coordinates. |callback| can be invoked either synchronously or
- * asynchronously. By default, |clipped| is true. If |clipped| is set to false, 
- * the bounds will not be clipped to the ancestors of the node.
+ * asynchronously.
  * @param {number} startIndex
  * @param {number} endIndex
  * @param {function(!chrome.automation.Rect): void} callback
- * @param {boolean=} clipped
  * @see https://developer.chrome.com/extensions/automation#method-boundsForRange
  */
-chrome.automation.AutomationNode.prototype.boundsForRange = function(startIndex, endIndex, callback, clipped) {};
+chrome.automation.AutomationNode.prototype.boundsForRange = function(startIndex, endIndex, callback) {};
 
 /**
  * The location (as a bounding box) of this node in global screen coordinates without applying any clipping from ancestors.