From 5e3dc0d76599cc8f8fc78463cf70f761762af862 Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Mon, 19 Jun 2023 14:18:17 +0900 Subject: [PATCH 1/7] feat: add snapshotMaxDepth --- .../uiautomator2/core/AxNodeInfoHelper.java | 7 ++- .../uiautomator2/model/settings/Settings.java | 3 +- .../model/settings/SnapshotMaxDepth.java | 43 +++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java diff --git a/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java b/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java index 8c0b215c1..b278b3066 100644 --- a/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java +++ b/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java @@ -37,6 +37,7 @@ import io.appium.uiautomator2.model.internal.CustomUiDevice; import io.appium.uiautomator2.model.settings.Settings; import io.appium.uiautomator2.model.settings.SimpleBoundsCalculation; +import io.appium.uiautomator2.model.settings.SnapshotMaxDepth; import io.appium.uiautomator2.utils.Logger; import static io.appium.uiautomator2.utils.Device.getUiDevice; @@ -48,8 +49,6 @@ * This class contains static helper methods to work with {@link AccessibilityNodeInfo} */ public class AxNodeInfoHelper { - // https://github.com/appium/appium/issues/12892 - private static final int MAX_DEPTH = 70; private static final long UNDEFINED_NODE_ID = (((long) Integer.MAX_VALUE) << 32) | Integer.MAX_VALUE; private static final int UNDEFINED_WINDOW_ID = -1; @@ -233,7 +232,7 @@ public static int calculateIndex(AccessibilityNodeInfo node) { } /** - * Returns the node's bounds clipped to the size of the display, limited by the MAX_DEPTH + * Returns the node's bounds clipped to the size of the display, limited by the SnapshotMaxDepth * The implementation is borrowed from `getVisibleBounds` method of `UiObject2` class * * @return Empty rect if node is null, else a Rect containing visible bounds @@ -263,7 +262,7 @@ private static Rect getBounds(@Nullable AccessibilityNodeInfo node, Rect display Set ancestors = new HashSet<>(); AccessibilityNodeInfo ancestor = node.getParent(); // An erroneous situation is possible where node parent equals to the node itself - while (++currentDepth < MAX_DEPTH && ancestor != null && !ancestors.contains(ancestor)) { + while (++currentDepth < Settings.get(SnapshotMaxDepth.class).getValue() && ancestor != null && !ancestors.contains(ancestor)) { // If this ancestor is scrollable if (ancestor.isScrollable()) { // Trim any portion of the bounds that are hidden by the non-visible portion of our diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/Settings.java b/app/src/main/java/io/appium/uiautomator2/model/settings/Settings.java index dc2fdd1b0..3069c2c6d 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/settings/Settings.java +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/Settings.java @@ -44,7 +44,8 @@ public enum Settings { MJPEG_SCALING_FACTOR(new MjpegScalingFactor()), MJPEG_SERVER_SCREENSHOT_QUALITY(new MjpegServerScreenshotQuality()), MJPEG_BILINEAR_FILTERING(new MjpegBilinearFiltering()), - USE_RESOURCES_FOR_ORIENTATION_DETECTION(new UseResourcesForOrientationDetection()); + USE_RESOURCES_FOR_ORIENTATION_DETECTION(new UseResourcesForOrientationDetection()), + SNAPSHOT_MAX_DEPTH(new SnapshotMaxDepth()); private final ISetting setting; diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java new file mode 100644 index 000000000..5329210c5 --- /dev/null +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java @@ -0,0 +1,43 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.appium.uiautomator2.model.settings; + +public class SnapshotMaxDepth extends AbstractSetting { + public static final String SETTING_NAME = "snapshotMaxDepth"; + // https://github.com/appium/appium/issues/12892 + private static final Integer DEFAULT_VALUE = 70; + private Integer snapshotMaxDepth = DEFAULT_VALUE; + + public SnapshotMaxDepth() { + super(Integer.class, SETTING_NAME); + } + + @Override + public Integer getValue() { + return snapshotMaxDepth; + } + + @Override + public Integer getDefaultValue() { + return DEFAULT_VALUE; + } + + @Override + protected void apply(Integer value) { + snapshotMaxDepth = value; + } +} From bdeca65fc8065c4aa508e1dc53fb1d94796ec00c Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Mon, 19 Jun 2023 15:53:42 +0900 Subject: [PATCH 2/7] fix: use SnapshotMaxDepth in UiElementSnapshot --- .../io/appium/uiautomator2/model/UiElementSnapshot.java | 7 +++---- .../uiautomator2/model/settings/SnapshotMaxDepth.java | 8 +++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/io/appium/uiautomator2/model/UiElementSnapshot.java b/app/src/main/java/io/appium/uiautomator2/model/UiElementSnapshot.java index 7517a55bb..47db7eb34 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/UiElementSnapshot.java +++ b/app/src/main/java/io/appium/uiautomator2/model/UiElementSnapshot.java @@ -39,6 +39,7 @@ import io.appium.uiautomator2.core.AxNodeInfoHelper; import io.appium.uiautomator2.model.settings.AllowInvisibleElements; import io.appium.uiautomator2.model.settings.IncludeExtrasInPageSource; +import io.appium.uiautomator2.model.settings.SnapshotMaxDepth; import io.appium.uiautomator2.model.settings.Settings; import io.appium.uiautomator2.utils.Attribute; import io.appium.uiautomator2.utils.Logger; @@ -55,8 +56,6 @@ @TargetApi(18) public class UiElementSnapshot extends UiElement { private final static String ROOT_NODE_NAME = "hierarchy"; - // https://github.com/appium/appium/issues/12545 - private final static int DEFAULT_MAX_DEPTH = 70; // The same order will be used for node attributes in xml page source public final static Attribute[] SUPPORTED_ATTRIBUTES = new Attribute[]{ Attribute.INDEX, Attribute.PACKAGE, Attribute.CLASS, Attribute.TEXT, @@ -95,7 +94,7 @@ private UiElementSnapshot(AccessibilityNodeInfo node, int index, int depth, int private UiElementSnapshot(AccessibilityNodeInfo node, int index, int depth, Set includedAttributes) { - this(node, index, depth, DEFAULT_MAX_DEPTH, includedAttributes); + this(node, index, depth, Settings.get(SnapshotMaxDepth.class).getValue(), includedAttributes); } private UiElementSnapshot(AccessibilityNodeInfo[] childNodes, @@ -103,7 +102,7 @@ private UiElementSnapshot(AccessibilityNodeInfo[] childNodes, super(null); this.depth = 0; this.index = 0; - this.maxDepth = DEFAULT_MAX_DEPTH; + this.maxDepth = Settings.get(SnapshotMaxDepth.class).getValue(); Map attribs = new LinkedHashMap<>(); putAttribute(attribs, Attribute.INDEX, this.index); putAttribute(attribs, Attribute.CLASS, ROOT_NODE_NAME); diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java index 5329210c5..9a1fbaa73 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java @@ -18,9 +18,11 @@ public class SnapshotMaxDepth extends AbstractSetting { public static final String SETTING_NAME = "snapshotMaxDepth"; + // Set DEFAULT_VALUE as 70 to avoid StackOverflow from infinite recursion + // https://github.com/appium/appium/issues/12545 // https://github.com/appium/appium/issues/12892 private static final Integer DEFAULT_VALUE = 70; - private Integer snapshotMaxDepth = DEFAULT_VALUE; + private Integer value = DEFAULT_VALUE; public SnapshotMaxDepth() { super(Integer.class, SETTING_NAME); @@ -28,7 +30,7 @@ public SnapshotMaxDepth() { @Override public Integer getValue() { - return snapshotMaxDepth; + return value; } @Override @@ -38,6 +40,6 @@ public Integer getDefaultValue() { @Override protected void apply(Integer value) { - snapshotMaxDepth = value; + this.value = value; } } From 1abd4fb463fa85cfe9daf18781edec0bb7111703 Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Tue, 20 Jun 2023 09:14:38 +0900 Subject: [PATCH 3/7] fix: split the long line --- .../java/io/appium/uiautomator2/core/AxNodeInfoHelper.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java b/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java index b278b3066..3cca03e8d 100644 --- a/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java +++ b/app/src/main/java/io/appium/uiautomator2/core/AxNodeInfoHelper.java @@ -262,7 +262,8 @@ private static Rect getBounds(@Nullable AccessibilityNodeInfo node, Rect display Set ancestors = new HashSet<>(); AccessibilityNodeInfo ancestor = node.getParent(); // An erroneous situation is possible where node parent equals to the node itself - while (++currentDepth < Settings.get(SnapshotMaxDepth.class).getValue() && ancestor != null && !ancestors.contains(ancestor)) { + while (++currentDepth < Settings.get(SnapshotMaxDepth.class).getValue() + && ancestor != null && !ancestors.contains(ancestor)) { // If this ancestor is scrollable if (ancestor.isScrollable()) { // Trim any portion of the bounds that are hidden by the non-visible portion of our From f6cd5310bbca980c9bd60752311d513b0285b7f9 Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Tue, 20 Jun 2023 09:14:57 +0900 Subject: [PATCH 4/7] fix: use int for DEFAULT_VALUE --- .../io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java index 9a1fbaa73..41364e466 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java @@ -21,7 +21,7 @@ public class SnapshotMaxDepth extends AbstractSetting { // Set DEFAULT_VALUE as 70 to avoid StackOverflow from infinite recursion // https://github.com/appium/appium/issues/12545 // https://github.com/appium/appium/issues/12892 - private static final Integer DEFAULT_VALUE = 70; + private static final int DEFAULT_VALUE = 70; private Integer value = DEFAULT_VALUE; public SnapshotMaxDepth() { From 8f855e284e8b53f932b97b8249c308770e1562b3 Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Tue, 20 Jun 2023 09:32:53 +0900 Subject: [PATCH 5/7] fix: add validation for an apply value --- .../uiautomator2/model/settings/SnapshotMaxDepth.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java index 41364e466..4f0b52f3a 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java @@ -16,6 +16,8 @@ package io.appium.uiautomator2.model.settings; +import io.appium.uiautomator2.common.exceptions.InvalidArgumentException; + public class SnapshotMaxDepth extends AbstractSetting { public static final String SETTING_NAME = "snapshotMaxDepth"; // Set DEFAULT_VALUE as 70 to avoid StackOverflow from infinite recursion @@ -40,6 +42,13 @@ public Integer getDefaultValue() { @Override protected void apply(Integer value) { + if (value == null || value < 10 || value > 200) { + throw new InvalidArgumentException(String.format( + "Invalid %s value specified, must be in range 10..200. %s was given", + SETTING_NAME, + value + )); + } this.value = value; } } From 45d27012546a28b0fd05badd8d7b28098f281ba0 Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Tue, 20 Jun 2023 18:44:20 +0900 Subject: [PATCH 6/7] fix: add constants instead of magic number --- .../uiautomator2/model/settings/SnapshotMaxDepth.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java index 4f0b52f3a..b4cbe6668 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java @@ -24,6 +24,8 @@ public class SnapshotMaxDepth extends AbstractSetting { // https://github.com/appium/appium/issues/12545 // https://github.com/appium/appium/issues/12892 private static final int DEFAULT_VALUE = 70; + private static final int MIN_DEPTH = 10; + private static final int MAX_DEPTH = 500; private Integer value = DEFAULT_VALUE; public SnapshotMaxDepth() { @@ -42,10 +44,12 @@ public Integer getDefaultValue() { @Override protected void apply(Integer value) { - if (value == null || value < 10 || value > 200) { + if (value == null || value < MIN_DEPTH || value > MAX_DEPTH) { throw new InvalidArgumentException(String.format( - "Invalid %s value specified, must be in range 10..200. %s was given", + "Invalid %s value specified, must be in range %s..%s. %s was given", SETTING_NAME, + MIN_DEPTH, + MAX_DEPTH, value )); } From e800feb652039e4d006bc80e55dfbae50d61ea26 Mon Sep 17 00:00:00 2001 From: yoshitaka-ogata <98735971+yoshitaka-ogata@users.noreply.github.com> Date: Tue, 20 Jun 2023 18:46:59 +0900 Subject: [PATCH 7/7] fix: min value to 1 --- .../io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java index b4cbe6668..b47b9a4f2 100644 --- a/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java +++ b/app/src/main/java/io/appium/uiautomator2/model/settings/SnapshotMaxDepth.java @@ -24,7 +24,7 @@ public class SnapshotMaxDepth extends AbstractSetting { // https://github.com/appium/appium/issues/12545 // https://github.com/appium/appium/issues/12892 private static final int DEFAULT_VALUE = 70; - private static final int MIN_DEPTH = 10; + private static final int MIN_DEPTH = 1; private static final int MAX_DEPTH = 500; private Integer value = DEFAULT_VALUE;