1427635 - Fix DoCompareTreePosition frame tree version with null aCommonAncestor.

Assignee

Description

•

6 years ago

Right now we don't use it but it's broken. We need to use it for bug 1414303, so this is a preliminar patch for that.

Comment hidden (mozreview-request)

It doesn't fill the ancestors of the first frame if aCommonAncestor is null,
which means that we get garbage afterwards.

Review commit: https://reviewboard.mozilla.org/r/209748/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209748/

Xidorn Quan [:xidorn] UTC+11

Comment 2

•

6 years ago

mozreview-review

Comment on attachment 8939410 [details]
Bug 1427635: Fix DoCompareTreePosition frame tree version with null aCommonAncestor.

https://reviewboard.mozilla.org/r/209748/#review215294

::: layout/base/nsLayoutUtils.cpp:1916
(Diff revision 1)
> -  if (aCommonAncestor &&
> -      !FillAncestors(aFrame1, aCommonAncestor, &frame1Ancestors)) {
> +  if (!FillAncestors(aFrame1, aCommonAncestor, &frame1Ancestors) &&
> +      aCommonAncestor) {

Consider adding a comment here stating that we are relying on the side effect of `FillAncestors` to fill the array, so the order is important... in case anyone wonders and tries to "optimize" it in the future.

Attachment #8939410 - Flags: review?(xidorn+moz) → review+

Comment hidden (mozreview-request)

Comment on attachment 8939410 [details]
Bug 1427635: Fix DoCompareTreePosition frame tree version with null aCommonAncestor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209748/diff/1-2/

Emilio Cobos Álvarez (:emilio)

Assignee

Comment 4

•

6 years ago

mozreview-review-reply

Comment on attachment 8939410 [details]
Bug 1427635: Fix DoCompareTreePosition frame tree version with null aCommonAncestor.

https://reviewboard.mozilla.org/r/209748/#review215294

> Consider adding a comment here stating that we are relying on the side effect of `FillAncestors` to fill the array, so the order is important... in case anyone wonders and tries to "optimize" it in the future.

Agreed, done! Thanks for the review :)

Pulsebot

Comment 5

•

6 years ago

Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9a95b205f3e8
Fix DoCompareTreePosition frame tree version with null aCommonAncestor. r=xidorn

Arthur Iakab [arthur_iakab]

Comment 6

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/9a95b205f3e8

Status: NEW → RESOLVED

Closed: 6 years ago

status-firefox59: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla59

Pulsebot

Comment 7

•

6 years ago

Backout by csabou@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/2b0691e2e31b
Backed out changeset 9a95b205f3e8 for cl failures at dom/browser-element/mochitest/test_browserElement_oop_CopyPaste.html r=backout a=backout

Cosmin Sabou [:CosminS]

Comment 8

•

6 years ago

Backed out for cl failures at dom/browser-element/mochitest/test_browserElement_oop_CopyPaste.html

Treeeherder link with the push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9a95b205f3e8df9cd967c3f8a0fae52a54dac6a9&filter-resultStatus=testfailed&filter-resultStatus=runnable&selectedJob=153757245

Backout link: https://hg.mozilla.org/mozilla-central/rev/2b0691e2e31bb2d30d74de99818d20856479587d

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=153757245&repo=autoland&lineNumber=14590

Status: RESOLVED → REOPENED

status-firefox59: fixed → ---

Flags: needinfo?(emilio)

Resolution: FIXED → ---

Target Milestone: mozilla59 → ---

Pulsebot

Comment 9

•

6 years ago

Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9eb8652389f2
Fix DoCompareTreePosition frame tree version with null aCommonAncestor. r=xidorn

Emilio Cobos Álvarez (:emilio)

Assignee

Comment 10

•

6 years ago

I tried to make a sense of it, but I couldn't. There is a single caller of this function, which is:

  https://searchfox.org/mozilla-central/rev/5bbbbe17bc262ded6d4f0e0906ed84aafb4ca0c8/layout/base/AccessibleCaretManager.cpp#1221

That's loosely related to that test, but that only means that that caller is completely busted. I pushed to try today and didn't see that failure, so I pushed again for now... Otherwise I guess disabling that test in win7 debug may be worth doing.

Emilio Cobos Álvarez (:emilio)

Assignee

Updated

•

6 years ago

Flags: needinfo?(emilio)

Cristina Coroiu [:ccoroiu]

Comment 11

•

6 years ago

Backed out changeset 9eb8652389f2 (bug 1427635) for clipboard failures at dom/browser-element/mochitest/test_browserElement_oop_CopyPaste.html

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9eb8652389f242ca583005e5eabdc1629d3982e8&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=161669203&repo=autoland&lineNumber=8389

Backout: https://hg.mozilla.org/integration/autoland/rev/b3c010d9f5d7ec4e64246e425d271270ae78ce40

Flags: needinfo?(emilio)

Jet Villegas (inactive)

Comment 12

•

6 years ago

[ Triage 2017/02/20: P3 ]

Priority: -- → P3

Emilio Cobos Álvarez (:emilio)

Assignee

Updated

•

6 years ago

No longer blocks: shadowdom-layout

Emilio Cobos Álvarez (:emilio)

Assignee

Updated

•

2 years ago

Flags: needinfo?(emilio)

Pulsebot

Comment 13

•

2 years ago

Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/dc6dc0fd83e9
Fix DoCompareTreePosition frame tree version with null aCommonAncestor. r=xidorn

bszekely

Comment 14

•

2 years ago

Backed out for causing geckoview-junit failures.

Failure line: TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.SelectionActionDelegateTest#selectAll[#text] | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms

Push with failures

Failure log

Backout link

[task 2022-07-19T13:00:28.221Z] 13:00:28     INFO -  07-19 13:00:04.810 I/Gecko   ( 3426): nsWindow 0x7702c0052300 destructor
[task 2022-07-19T13:00:28.221Z] 13:00:28     INFO -  07-19 13:00:04.810 I/Gecko   ( 3426): nsWindow 0x7702c0052f00 destructor
[task 2022-07-19T13:00:28.221Z] 13:00:28     INFO -  07-19 13:00:04.810 I/Gecko   ( 3426): nsWindow 0x7702c0051100 destructor
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:04.810 I/Gecko   ( 3426): nsWindow 0x7702c0052c00 destructor
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): Error
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutRunnable.run(UiThreadUtils.java:49)
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.os.Handler.handleCallback(Handler.java:751)
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2022-07-19T13:00:28.222Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.util.UiThreadUtils.waitForCondition(UiThreadUtils.java:158)
[task 2022-07-19T13:00:28.223Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1778)
[task 2022-07-19T13:00:28.223Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1675)
[task 2022-07-19T13:00:28.223Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1629)
[task 2022-07-19T13:00:28.223Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.SelectionActionDelegateTest$changesSelectionTo$1.invoke(SelectionActionDelegateTest.kt:539)
[task 2022-07-19T13:00:28.223Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.SelectionActionDelegateTest$changesSelectionTo$1.invoke(SelectionActionDelegateTest.kt:538)
[task 2022-07-19T13:00:28.224Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.SelectionActionDelegateTest.testThat(SelectionActionDelegateTest.kt:296)
[task 2022-07-19T13:00:28.224Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.SelectionActionDelegateTest.selectAll(SelectionActionDelegateTest.kt:185)
[task 2022-07-19T13:00:28.224Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at java.lang.reflect.Method.invoke(Native Method)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$org-mozilla-geckoview-test-rule-GeckoSessionTestRule$2(GeckoSessionTestRule.java:1463)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2$$ExternalSyntheticLambda0.run(Unknown Source)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:428)
[task 2022-07-19T13:00:28.225Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at java.util.concurrent.FutureTask.run(FutureTask.java:237)
[task 2022-07-19T13:00:28.226Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
[task 2022-07-19T13:00:28.226Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.os.Handler.handleCallback(Handler.java:751)
[task 2022-07-19T13:00:28.226Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2022-07-19T13:00:28.226Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.os.Looper.loop(Looper.java:154)
[task 2022-07-19T13:00:28.226Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at android.app.ActivityThread.main(ActivityThread.java:6077)
[task 2022-07-19T13:00:28.226Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at java.lang.reflect.Method.invoke(Native Method)
[task 2022-07-19T13:00:28.227Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
[task 2022-07-19T13:00:28.227Z] 13:00:28     INFO -  07-19 13:00:27.015 E/GeckoSessionTestRule( 3426): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
[task 2022-07-19T13:00:28.227Z] 13:00:28     INFO -  07-19 13:00:27.019 D/GeckoViewConsole( 3426): onEvent GeckoView:WebExtension:PortMessageFromApp {"message":{"id":"67b90ed8-960b-404b-a831-1b127bbb9e21","args":{"oldPrefs":{"geckoview.selection_action.show_on_focus":null,"layout.accessiblecaret.script_change_update_mode":1}},"type":"RestorePrefs"}}
[task 2022-07-19T13:00:28.227Z] 13:00:28     INFO -  07-19 13:00:27.035 D/EGL_emulation( 8080): eglMakeCurrent: 0x770309331500: ver 3 0 (tinfo 0x7703093712c0)
[task 2022-07-19T13:00:28.227Z] 13:00:28     INFO -  07-19 13:00:27.041 D/GeckoViewContent( 3426): handleEvent: DOMWindowClose
[task 2022-07-19T13:00:28.227Z] 13:00:28     INFO -  07-19 13:00:27.044 D/GeckoViewConsole( 3426): onEvent GeckoView:WebExtension:List null
[task 2022-07-19T13:00:28.228Z] 13:00:28     INFO -  07-19 13:00:27.049 E/GeckoSessionTestRule( 3426): test_end c5ee677f-bc83-49bd-9e28-2d35f3d0f059 selectAll[#text](org.mozilla.geckoview.test.SelectionActionDelegateTest)
[task 2022-07-19T13:00:28.228Z] 13:00:28  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.SelectionActionDelegateTest#selectAll[#text] | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2022-07-19T13:00:28.228Z] 13:00:28     INFO -  TEST-INFO took 30697ms
[task 2022-07-19T13:00:28.228Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=932
[task 2022-07-19T13:00:28.228Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2022-07-19T13:00:28.228Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2022-07-19T13:00:28.228Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=request_noClipboard[#text]
[task 2022-07-19T13:00:28.229Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.SelectionActionDelegateTest
[task 2022-07-19T13:00:28.229Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=564
[task 2022-07-19T13:00:28.229Z] 13:00:28     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: 1
[task 2022-07-19T13:00:28.229Z] 13:00:28     INFO -  TEST-START | org.mozilla.geckoview.test.SelectionActionDelegateTest#request_noClipboard[#text]

Flags: needinfo?(emilio)

BMO Automation

Updated

•

2 years ago

Severity: normal → S3