Closed Bug 1358643 Opened 7 years ago Closed 5 years ago

Assertion failure: mSearchesOngoing > 0 && mSearches.Contains(aSearch), at toolkit/components/autocomplete/nsAutoCompleteController.cpp:860 - classify if failining test path doesn't contains "formautofill"

Categories

(Firefox :: Bookmarks & History, defect, P3)

defect
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 70
Iteration:
70.1 - Jul 8 - 21
Tracking Status
firefox70 --- fixed

People

(Reporter: botond, Assigned: mak)

Details

(Keywords: reproducible)

Attachments

(1 file)

STR:
  1. Run Nightly in debug mode
  2. Show All History (Ctrl+Shift+H)
  3. Select a date
  4. In the tags field, type any tag name followed by a comma

The assertion in the description is triggered.
Keywords: reproducible
This is a bug in the tagging autocomplete code, likely a DUPE since I seem to remember another bug about this.
Component: Autocomplete → Places
Whiteboard: [DUPEME
Priority: -- → P3
Whiteboard: [DUPEME → [DUPEME]
Keywords: dupeme
Whiteboard: [DUPEME]

In the last 7 days there were 83 failures associated with this bug: https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?startday=2019-07-07&endday=2019-07-14&tree=all&bug=1358643
Recent failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256408773&repo=autoland&lineNumber=4747
Occurs on windows7-32, macosx1014-64, windows10-64-qr, windows7-32, macosx1014-64 build types.

03:52:45     INFO - TEST-OK | browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html | took 3552ms
03:52:45     INFO - GECKO(4272) | ++DOMWINDOW == 9 (0A59DC00) [pid = 1584] [serial = 18] [outer = 0863D700]
03:52:45     INFO - GECKO(4272) | Assertion failure: mSearchesOngoing > 0 && mSearches.Contains(aSearch), at z:/build/build/src/toolkit/components/autocomplete/nsAutoCompleteController.cpp:808
03:53:11     INFO - GECKO(4272) | #01: NS_InvokeByIndex
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #02: CallMethodHelper::Call() [js/xpconnect/src/XPCWrappedNative.cpp:1195]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #03: XPCWrappedNative::CallMethod(XPCCallContext &,XPCWrappedNative::CallMode) [js/xpconnect/src/XPCWrappedNative.cpp:1157]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #04: XPC_WN_CallMethod(JSContext *,unsigned int,JS::Value *) [js/xpconnect/src/XPCWrappedNativeJSOps.cpp:943]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #05: CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/vm/Interpreter.cpp:448]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #06: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:540]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #07: static bool InternalCall(struct JSContext *, const class js::AnyInvokeArgs & const) [js/src/vm/Interpreter.cpp:595]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #08: js::jit::DoCallFallback(JSContext *,js::jit::BaselineFrame *,js::jit::ICCall_Fallback *,unsigned int,JS::Value *,JS::MutableHandle<JS::Value>) [js/src/jit/BaselineIC.cpp:0]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #09: ??? (???:???)
03:53:11     INFO - GECKO(4272) | #10: ??? (???:???)
03:53:11     INFO - GECKO(4272) | #11: ??? (???:???)
03:53:11     INFO - GECKO(4272) | #12: js::jit::MaybeEnterJit(JSContext *,js::RunState &) [js/src/jit/Jit.cpp:196]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #13: js::RunScript(JSContext *,js::RunState &) [js/src/vm/Interpreter.cpp:410]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #14: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:568]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #15: static bool InternalCall(struct JSContext *, const class js::AnyInvokeArgs & const) [js/src/vm/Interpreter.cpp:595]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #16: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::AnyInvokeArgs const &,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:611]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #17: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.h:98]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #18: static bool PromiseReactionJob(struct JSContext *, unsigned int, union JS::Value *) [js/src/builtin/Promise.cpp:1704]
03:53:11     INFO - 
03:53:11     INFO - GECKO(4272) | #19: CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/vm/Interpreter.cpp:448]
Component: Places → Autocomplete

Marco can you assign someone to take a look at this?

there are 85 total failures in the last 7 days on windows7-32 debug. The assertion occurs in different tests, cannot be disabled.

Flags: needinfo?(mak77)

This is a bug in the consumer of Toolkit/Autocomplete, that pretty much means the search provider is sending some results after a query has been canceled/stopped.
Considered we don't use this widget for the urlbar anymore, I'd also be ok converting this assertion to a warning, if form fill doesn't care that much. considered the complication of current form fill code (spread across 2 processes), it may also be acceptable.

Component: Autocomplete → Form Manager
Flags: needinfo?(mak77) → needinfo?(MattN+bmo)

we could pretty much replace the assertion with a warning and an early return

(In reply to Daniel Varga [:dvarga] from comment #59)

03:52:45 INFO - TEST-OK | browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html | took 3552ms

This bug is supposed to be for assertions not triggered by formautofill as they have their own bug: bug 1454211

(In reply to Marco Bonardo [::mak] from comment #62)

This is a bug in the consumer of Toolkit/Autocomplete, that pretty much means the search provider is sending some results after a query has been canceled/stopped.
Considered we don't use this widget for the urlbar anymore, I'd also be ok converting this assertion to a warning, if form fill doesn't care that much.

Skimming the bugs it seems like a cause may be the bookmark tag autocomplete as I see that panel being tested. See above how this bug isn't supposed to be related to form autofill. Also, nothing has changed in form autofill code recently. Perhaps this was related to de-XBL of autocomplete elements.

Flags: needinfo?(MattN+bmo)

So, it sounds like the failures are now being accounted all here, because all the most recent logs I opened should be in bug 1454211.

The signature will look the same, and I don't think treeherder distinguishes them based on the starting test

Component: Form Manager → Bookmarks & History
Product: Toolkit → Firefox

Both suggestions used to show in TH… maybe the sheriffs are choosing the wrong one sometimes.

Treeherder doesn't suggest bugs for assertions in general. That in most logs I checked the executed test path is not mentioned and the log has to be checked makes this time consuming and error prone.

Summary: Assertion failure: mSearchesOngoing > 0 && mSearches.Contains(aSearch), at toolkit/components/autocomplete/nsAutoCompleteController.cpp:860 → Assertion failure: mSearchesOngoing > 0 && mSearches.Contains(aSearch), at toolkit/components/autocomplete/nsAutoCompleteController.cpp:860 - classify if failining test path doesn't contains "formautofill"

The reason for this assertion is mostly that if a consumer doesn't pay attention to its own async behavior, it risks to push a result for a previous search into the next search. The assertion doesn't even catch all the possible cases, it's just a partial cover.

For the tags case I can probably use a method similar to the one we used in old urlbar.

Assignee: nobody → mak77
Status: NEW → ASSIGNED
Iteration: --- → 70.1 - Jul 8 - 21
Points: --- → 2
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/d55853ca133a
Avoid leaking results to the next autocomplete search in tags autocomplete. r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

This should reduce the likelihood of mis-assigning the failures, so hopefully it will help sheriffs.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: