The old autocomplete controller used a trick to avoid flicker when autofilling; it kept an internal placeholder string (see mPlaceholderCompletionString) and used that when the previous result was autofilled and the new search is a substring of the previous value.

Without this fix the autofilled part disappears and reappers, causing visual flicker.

Bug 1531348 and the differences in autofill between nsAutoCompleteController and quantumbar are related to this.

This is a really messy WIP that fixes this bug and bug 1531348. Summarizing, there are several differences between autofill in nsAutoCompleteController and quantumbar:

• In nsAutoCompleteController, the logic that determines whether the new search is a prefix of the old search is only done in HandleText, i.e., on input, not when the value is set programmatically.

• That logic is a lot more complex in nsAutoCompleteController. I tried to avoid it in quantumbar, but I guess it's there for a reason.

• nsAutoCompleteController autofills in a couple of cases where quantumbar doesn't: (1) As Marco says, when completing the "placeholder" string before starting a new search and waiting for the async results (thereby preventing flicker), and (2) when it receives the first result and it's the default index and it should be completed.

• Some nsAutoCompleteController state gets reset each time the awesomebar is focused (see calls to attachController() in the autocomplete binding, which sets the controller's input, which calls ResetInternalState()). That state is important in regard to autofill and the placeholder string. There's nothing like that in quantumbar right now. So e.g. if it's not reset, then the autofill of one search will incorrectly affect the autofill of a later search.

This is the old existing test for flicker, in case you want to check if the new test covers similar functionality: https://searchfox.org/mozilla-central/source/toolkit/content/tests/chrome/test_autocomplete_placehold_last_complete.xul

https://treeherder.mozilla.org/#/jobs?repo=try&revision=795a8774d78ebb24eb2f9526776b152d1edbd6d3

Some failures on try related to how the patch doesn't search when the new search string is the same as the last string.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=83641556ebeaa83899a18932826613a444bc4a25

It looks like the patch resulted in one fewer expected reflows... o_O But try otherwise looks OK.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f209ada0d0a623f3faa02a74c9383328df9cb387

After removing the expected reflow, there's now an unexpected reflow on Windows 10 x64 opt. The awesombar version of this test does have some platform-specific expected reflows, but the quantumbar version didn't need them. I guess now it does for some reason with this patch. I'll look closer tomorrow.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3a3160f4b0bb2600143eecf3de891844ed69abf

The reflow is intermittent on Windows, sigh...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a355b5739060fd68920205f72d85208ab65b4576

https://treeherder.mozilla.org/#/jobs?repo=try&revision=171a5f5a94e8aea68a0953fc5d63d04f9474a34e

The awesomebar version of these tests replaced popup.invalidate and popup.onResultsAdded with functions that first call dirtyFrame(win) before calling the original implementations, because:

  // We need to invalidate the frame tree outside of the normal
  // mechanism since invalidations and result additions to the
  // URL bar occur without firing JS events (which is how we
  // normally know to dirty the frame tree).

I'm starting to think we still need to do that for quantumbar because actually both Linux and Windows are intermittent with the patch, although Linux is a little more stable which is why I thought it was only Windows earlier.

Here's a try push that similarly replaces view.onQueryResults and view.onQueryFinished: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fcf773fd589b843ef92f79b6e55891ee8c6a3d9

But

(1) Why does the patch make this necessary? It's conceivable that the popup is now showing slightly different results from before since it's affecting when autofill happens, at least the heuristic result. Maybe that's it.

(2) Is replacing those functions in the test affecting the test itself by e.g. forcing the reflows to happen? I'll ask Mike about it tomorrow if this try run is green.

Mike, would you mind reviewing these changes to the urlbar reflow performance tests? I previously modified these tests for quantumbar in bug 1528744. I'm now making changes to urlbar autofill and these tests are intermittently failing on try. Sometimes the expected reflows happen, sometimes they don't. The legacy/awesomebar path for these tests replace a couple of popup methods so that they call dirtyFrame(). I've done the same here in this patch, and that has fixed the failures. But I'm not sure it's the right thing to do. In particular I'm not sure whether calling dirtyFrame() this way inappropriately affects the very thing we're testing. But I would guess not since the legacy path does it.

Try with the patch addressing Marco's review comments and the reflow test fixes mentioned:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=330c16b037293df30ec21b6369013fc496e3dd60

Try looks good.

With the patch to bug 1530706 applied + fix for a test in that patch (the searchTwice test task in browser_urlbarSearchFunction.js): https://treeherder.mozilla.org/#/jobs?repo=try&revision=e15dbe1b855d50020e5220234731a3c9bb01d40d

One more try push addressing the latest review comments and with an updated tree before landing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f5b70669b65a5bb17089f787b0ca6280638442d

https://hg.mozilla.org/mozilla-central/rev/5e2a3b886e64
https://hg.mozilla.org/mozilla-central/rev/47e99e138527

I'll request uplift, so here are some STR for QA:

1. Open about:config and set browser.urlbar.quantumbar to true.

2. Type mozilla.org in the urlbar. As you're typing, "mozilla.org/" should be autofilled. The part of the text that is selected/autofilled should not flicker as you type each letter. That is, the selected text should not disappear and then quickly reappear as you're typing. The text and selection should appear steady.

3. Backspace over some of the letters you've typed. It should work as normal, nothing weird should happen.

4. Type only "MOZ" (all caps). It should be autofilled to "MOZilla.org/".

5. Press the down arrow key to select the next result, then press the up arrow key to select the first result again. The text in the urlbar should be "MOZilla.org/" again, with the "MOZ" part not selected and the "illa.org/" part selected.

6. Press the down arrow key until you get to the one-off search buttons at the bottom of the popup. The text in the urlbar should only be "MOZ".

7. Continue pressing the down arrow key through all the search buttons until your selection wraps back around to the first result. The text in the urlbar should be "MOZilla.org/" again, with the "MOZ" part not selected and the "illa.org/" part selected.

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Not a regression, but part of ongoing quantumbar work. Bug 1511530 is the quantumbar autofill metabug.
• User impact if declined: Quantumbar is still preffed off, but we want the ability to run quantumbar experiments in Firefox 67. This bug is an important change to autofill that users running the experiment on 67 should have so that we can get accurate experiment results.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see comment 22
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a large patch, but it only affects a preffed-off feature, quantumbar. It has tests and I've verified it on beta.
• String changes made/needed: None
• Do you want to request approval of these patches as well?: on

User Agent:Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Verified as fixed on the latest nightly build (20190322101035). There is no more flickering on autofill and everything mentioned in comment 22 is as expected.

There is still flickering observed when performing step 4(all caps). I'm not sure if this is expected or not as it is not mentioned.

(In reply to Paul_B from comment #26)

There is still flickering observed when performing step 4(all caps). I'm not
sure if this is expected or not as it is not mentioned.

Thank you for pointing that out. I see it too. I think I may see what the problem is. I'll file a follow-up.