Bugzilla

Comment 2

•

4 years ago

(In reply to Andreea Pavel [:apavel] from comment #1)

This is from https://hg.mozilla.org/mozilla-central/rev/e454c9f14044 bug 1519486

Alexander can you please take a look?

Oh, does it fail on beta or release? If so, then how do you send jobs at try server for that branch, so I can reproduce it and play with it?

Flags: needinfo?(surkov.alexander)

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 3

•

4 years ago

Attached patch patch (obsolete) — Details — Splinter Review

this should help

Assignee: nobody → surkov.alexander

Comment 4

•

4 years ago

(In reply to alexander :surkov (:asurkov) from comment #2)

Oh, does it fail on beta or release? If so, then how do you send jobs at try server for that branch, so I can reproduce it and play with it?
Similar to https://wiki.mozilla.org/Sheriffing/How_To/Beta_simulations#TRUNK_AS_EARLY_BETA

Get beta configuration: Run ./mach try release -v 67.0b12 --tasks release-sim --migration central-to-beta --migration early-to-late-beta --no-push
Commit the changes: hg commit -m "Some beta configuration changes"
Select the job you want: ./mach try chooser - PGO builds are needed on Linux and Windows to get builds which behave like real beta builds.

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 5

•

4 years ago

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #4)

(In reply to alexander :surkov (:asurkov) from comment #2)

Oh, does it fail on beta or release? If so, then how do you send jobs at try server for that branch, so I can reproduce it and play with it?
Similar to https://wiki.mozilla.org/Sheriffing/How_To/Beta_simulations#TRUNK_AS_EARLY_BETA

Get beta configuration: Run ./mach try release -v 67.0b12 --tasks release-sim --migration central-to-beta --migration early-to-late-beta --no-push

Commit the changes: hg commit -m "Some beta configuration changes"

Select the job you want: ./mach try chooser - PGO builds are needed on Linux and Windows to get builds which behave like real beta builds.

I've got those all red https://treeherder.mozilla.org/#/jobs?repo=try&revision=41e489d63bcaaa39292d688276dbfb58640e2557&selectedJob=226195742. Ideas what I do wrong?

Matthew N. [:MattN]

Updated

•

4 years ago

Priority: -- → P1

Comment 6

•

4 years ago

Sorry, the pgo optimization is broken by bug 1523750. Request the normal "opt" jobs to get builds with branding, preferences etc. like beta, only the profile guided optimization will be missing (which likely is irrelevant for this bug).

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 7

•

4 years ago

(In reply to alexander :surkov (:asurkov) from comment #5)

I've got those all red https://treeherder.mozilla.org/#/jobs?repo=try&revision=41e489d63bcaaa39292d688276dbfb58640e2557&selectedJob=226195742. Ideas what I do wrong?
Opt builds have been requested and are green.

Updated

•

4 years ago

Attachment #9041156 - Flags: review?(MattN+bmo)

Comment 8

•

4 years ago

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #7)

(In reply to alexander :surkov (:asurkov) from comment #5)

I've got those all red https://treeherder.mozilla.org/#/jobs?repo=try&revision=41e489d63bcaaa39292d688276dbfb58640e2557&selectedJob=226195742. Ideas what I do wrong?
Opt builds have been requested and are green.

cool, here's another try with more greens :) https://treeherder.mozilla.org/#/jobs?repo=try&revision=948b21b06dd92ebedc5051bd86eb6554cf177010

Matthew N. [:MattN]

Comment 9

•

4 years ago

Comment on attachment 9041156 [details] [diff] [review]
patch

Review of attachment 9041156 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/performance/browser_urlbar_keyed_search.js
@@ +93,5 @@
>    },
>    {
>      stack: [
> +      "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
> +      "_adjustAcItem@chrome://global/content/bindings/autocomplete.xml",

Don't we need a patch that works for both m-c and beta? Or what am I missing? I'm not familiar with this test. Maybe :mak should review?

Comment 10

•

4 years ago

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #9)

::: browser/base/content/test/performance/browser_urlbar_keyed_search.js
@@ +93,5 @@
},
{
stack: [
 "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
 "_adjustAcItem@chrome://global/content/bindings/autocomplete.xml",
Don't we need a patch that works for both m-c and beta? Or what am I
missing? I'm not familiar with this test. Maybe :mak should review?

I think I changed lines added for beta/release, while I wasn't supposed to do so, because autocomplete.js is not yet on beta, thus I just reverted the change.

Comment 11

•

4 years ago

Comment on attachment 9041156 [details] [diff] [review]
patch

Review of attachment 9041156 [details] [diff] [review]:
-----------------------------------------------------------------

redirecting to :mak as mattN suggested

Attachment #9041156 - Flags: review?(MattN+bmo) → review?(mak77)

Updated

•

4 years ago

Summary: Perma browser/base/content/test/performance/browser_urlbar_keyed_search.js | Unused expected reflow at _handleOverflow@chrome://global/content/bindings/autocomplete.xml: is going to permafail when Gecko version number is bumped to 68 on 2019-03-18 → Perma browser/base/content/test/performance/browser_urlbar_keyed_search.js | Unused expected reflow at _handleOverflow@chrome://global/content/bindings/autocomplete.xml: when Gecko 67 merges to Beta on 2019-03-11

Comment 12

•

4 years ago

•

Edited

This is still happening on today beta-sim with the patch @comment 11 imported: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=fa12ca1befe286e9130117139c05759ad157d71c&searchStr=chrome&selectedJob=227181779

Flags: needinfo?(surkov.alexander)

Assignee

Updated

•

4 years ago

Blocks: 1519486

Assignee

Comment 13

•

4 years ago

Comment on attachment 9041156 [details] [diff] [review]
patch

Review of attachment 9041156 [details] [diff] [review]:
-----------------------------------------------------------------

I am missing some context, is autocomplete-richlistitem.js nightly-only behind a pref? Because otherwise I don't see why the test shoulk change to autocomplete.xml when we simulate merge of central to beta.
Ideally this needs to change only if something is enabled in Nightly only through a pref or such.
As such, I don't think this patch is correct, when we'll merge nightly to beta, autocomplete-richlistitem.js will be there, so the nightly stack will apply to beta.

I rather think you should change these lines https://searchfox.org/mozilla-central/rev/03ebbdab952409640c6857d835d3040bf6f9e2db/browser/base/content/test/performance/browser_urlbar_keyed_search.js#80-89 to point to autocomplete-richlistitem.js

As a side note:
"async*set popup@chrome://browser/content/search/search-one-offs.js",
Bug 1493536 made this not async, but bug 1524593 changed it back to async*... maybe a merge problem, anyway it's commented out so it has no visible effect (it's possible now the stack applies since there's no more async in the middle, maybe worth filing a separate bug to investigate that).

Attachment #9041156 - Flags: review?(mak77) → review-

Comment hidden (Intermittent Failures Robot)

Comment 15

•

4 years ago

(In reply to Marco Bonardo [::mak] (OoO 11-14 Feb) from comment #13)

I am missing some context, is autocomplete-richlistitem.js nightly-only
behind a pref? Because otherwise I don't see why the test shoulk change to
autocomplete.xml when we simulate merge of central to beta.
Ideally this needs to change only if something is enabled in Nightly only
through a pref or such.
As such, I don't think this patch is correct, when we'll merge nightly to
beta, autocomplete-richlistitem.js will be there, so the nightly stack will
apply to beta.

I am not sure either about this test section. So if nightly becomes a beta, and that's the tests are running for, then does it mean we should delete BETA_OR_RELEASE section all together, because the stack is no longer applied to beta?

I rather think you should change these lines
https://searchfox.org/mozilla-central/rev/
03ebbdab952409640c6857d835d3040bf6f9e2db/browser/base/content/test/
performance/browser_urlbar_keyed_search.js#80-89 to point to
autocomplete-richlistitem.js

those lines are not affected, i.e. those methods were located and are still located in autocomplete.xml (both in nightly and beta).

Flags: needinfo?(surkov.alexander)

Assignee

Comment 16

•

4 years ago

(In reply to alexander :surkov (:asurkov) from comment #15)

I am not sure either about this test section. So if nightly becomes a beta, and that's the tests are running for, then does it mean we should delete BETA_OR_RELEASE section all together, because the stack is no longer applied to beta?

I'm not sure, those lines exist because beta and nightly cause more/different reflows than Nightly, because the default bookmarls are different. They should still apply, I guess.

I rather think you should change these lines
https://searchfox.org/mozilla-central/rev/
03ebbdab952409640c6857d835d3040bf6f9e2db/browser/base/content/test/
performance/browser_urlbar_keyed_search.js#80-89 to point to
autocomplete-richlistitem.js

those lines are not affected, i.e. those methods were located and are still located in autocomplete.xml (both in nightly and beta).

Hm, if I search for _handleOverflow and _onUnderflow they are NO MORE in autocomplete.xml and apparently is what the failures are pointing out.

Comment 17

•

4 years ago

(In reply to Marco Bonardo [::mak] (OoO 11-14 Feb) from comment #16)

(In reply to alexander :surkov (:asurkov) from comment #15)

I am not sure either about this test section. So if nightly becomes a beta, and that's the tests are running for, then does it mean we should delete BETA_OR_RELEASE section all together, because the stack is no longer applied to beta?

I'm not sure, those lines exist because beta and nightly cause more/different reflows than Nightly, because the default bookmarls are different. They should still apply, I guess.

ok, I'm not familiar with the area, I take your word on it.

I rather think you should change these lines
https://searchfox.org/mozilla-central/rev/
03ebbdab952409640c6857d835d3040bf6f9e2db/browser/base/content/test/
performance/browser_urlbar_keyed_search.js#80-89 to point to
autocomplete-richlistitem.js

those lines are not affected, i.e. those methods were located and are still located in autocomplete.xml (both in nightly and beta).

Hm, if I search for _handleOverflow and _onUnderflow they are NO MORE in autocomplete.xml and apparently is what the failures are pointing out.

you're right, those lines should be updated, however not sure I get why you think the lines I changed in the patch should not be changed or did you mean it just wasn't enough to fix the bug?

Comment 18

•

4 years ago

(In reply to alexander :surkov (:asurkov) from comment #17)

(In reply to Marco Bonardo [::mak] (OoO 11-14 Feb) from comment #16)

however not sure I get why you think the lines I changed in the patch should not be changed or did you mean it just wasn't enough to fix the bug?

ignore it

Comment 19

•

4 years ago

Attached patch patch (obsolete) — Details — Splinter Review

Attachment #9041156 - Attachment is obsolete: true

Attachment #9043010 - Flags: review?(mak77)

https://hg.mozilla.org/try/rev/a9ee9990109e1ea7410fb3ef6fecac565198cfa5

Comment 20

•

4 years ago

This still fails on beta-sims with the patch @comment 19 imported: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&revision=4e5f61921ae313ef68e61a6cb8e1a1147c2ae49e&searchStr=browser%2Cchrome&selectedJob=228159212

Flags: needinfo?(surkov.alexander)

Assignee

Comment 21

•

4 years ago

Comment on attachment 9043010 [details] [diff] [review]
patch

Review of attachment 9043010 [details] [diff] [review]:
-----------------------------------------------------------------

The patch looks better, and the log is more readable.
There is still a failure, because apparently the stacks are now different, the third entry seems to be

MozAutocompleteRichlistitem/<@chrome://global/content/elements/autocomplete-richlistitem.js

instead of 
      onoverflow@chrome://browser/content/browser.xul
or
      onunderflow@chrome://browser/content/browser.xul

Attachment #9043010 - Flags: review?(mak77) → review-

Assignee

Comment 22

•

4 years ago

And since the stacks are now identical in the 2 cases, you can likely unify those two 6-instances reflows into a 12-instances reflow with a single stack signature.

Comment 23

•

4 years ago

Attached patch patch (obsolete) — Details — Splinter Review

Attachment #9043010 - Attachment is obsolete: true

Flags: needinfo?(surkov.alexander)

Attachment #9043691 - Flags: review?(mak77)

Raul Gurzau (:RaulG)

Comment 24

•

4 years ago

Alexander, seem like the failure is still there even if the patch was imported for Beta Simulation: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=0d9e35367944d97278d6d8b925e733e1f1c245be&selectedJob=228399413

Flags: needinfo?(surkov.alexander)

Assignee

Comment 25

•

4 years ago

Comment on attachment 9043691 [details] [diff] [review]
patch

Review of attachment 9043691 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, it looks like I was wrong in comment 22. Anyway, now that I'm back from travel I can make a patch for this, Alexander had far too much patience. Unfortunately these logs are not always nice, we should try to improve them.

Let me try to attach a patch to test.

Attachment #9043691 - Flags: review?(mak77)

Assignee

Updated

•

4 years ago

Flags: needinfo?(surkov.alexander)

Assignee

Comment 26

•

4 years ago

Attached file Bug 1524780 - Perma failure in browser_urlbar_keyed_search.js for Beta. r=surkov! — Details

Assignee

Updated

•

4 years ago

Attachment #9043691 - Attachment is obsolete: true

Updated

•

4 years ago

Assignee: surkov.alexander → mak77

Comment hidden (Intermittent Failures Robot)

Pulsebot

Comment 28

•

4 years ago

Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/aed64b74928b
Perma failure in browser_urlbar_keyed_search.js for Beta. r=surkov

https://hg.mozilla.org/mozilla-central/rev/aed64b74928b

Comment 29

•

4 years ago

bugherder

Status: NEW → RESOLVED

Closed: 4 years ago

status-firefox67: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 67