Lots of "JavaScript error: resource://gre/components/nsLivemarkService.js, line 299: NS_ERROR_ILLEGAL_VALUE: Invalid arguments" spam in mochitest-bc logs

RESOLVED FIXED in Firefox 48

Status

()

P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: RyanVM, Assigned: gasolin, Mentored)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [good first bug][lang=js])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
I was looking at a random mochitest-bc log today and noticed a lot of spam in the browser/components/places/tests/browser/ directory along the lines of:
JavaScript error: resource://gre/components/nsLivemarkService.js, line 299: NS_ERROR_ILLEGAL_VALUE: Invalid arguments

Figured I should file it :)
yeah, it's code invoking getLivemark with invalid argument in some specific case.
We need a stack.
Do you have some of the test names causing it?
Priority: -- → P2
(Reporter)

Comment 2

2 years ago
browser/components/places/tests/browser/browser_0_library_left_pane_migration.js
browser/components/places/tests/browser/browser_410196_paste_into_tags.js
browser/components/places/tests/browser/browser_435851_copy_query.js
browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
browser/components/places/tests/browser/browser_forgetthissite_single.js
browser/components/places/tests/browser/browser_library_batch_delete.js
browser/components/places/tests/browser/browser_library_commands.js
browser/components/places/tests/browser/browser_library_infoBox.js
browser/components/places/tests/browser/browser_library_panel_leak.js
browser/components/places/tests/browser/browser_library_search.js
sounds like related to a tree view then.
I'm seeing this locally - happens when I search in the Library window. I tweaked the code a little to get a stack trace printed:

S frame :: file:///d:/mozilla/central/objdir-frontend/dist/bin/components/nsLivemarkService.js :: getLivemark :: line 300
JS frame :: chrome://browser/content/places/treeView.js :: PTV_containerStateChanged :: line 886
JS frame :: chrome://browser/content/places/treeView.js :: PTV__finishInit :: line 68
JS frame :: chrome://browser/content/places/treeView.js :: PTV_setTree :: line 1495
JS frame :: chrome://browser/content/places/tree.xml :: set_view :: line 55
JS frame :: chrome://browser/content/places/tree.xml :: load :: line 119
JS frame :: chrome://browser/content/places/tree.xml :: applyFilter :: line 90
JS frame :: chrome://browser/content/places/places.js :: PSB_search :: line 820
JS frame :: chrome://browser/content/places/places.xul :: oncommand :: line 1
JS frame :: chrome://global/content/bindings/textbox.xml :: _fireCommand :: line 372
JS frame :: chrome://global/content/bindings/textbox.xml :: _enterSearch :: line 389
JS frame :: chrome://global/content/bindings/textbox.xml :: onxblkeypress :: line 427
(In reply to Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) from comment #4)
> file:///d:/mozilla/central/objdir-frontend/dist/bin/components/
> nsLivemarkService.js :: getLivemark :: line 300
> JS frame :: chrome://browser/content/places/treeView.js ::
> PTV_containerStateChanged :: line 886
> JS frame :: chrome://browser/content/places/treeView.js :: PTV__finishInit
> :: line 68

oh yes, _finishInit passes _rootNode to containerStateChanged, that does

PlacesUtils.livemarks.getLivemark({ id: aNode.itemId })

the root node doesn't always have an itemId, so getLivemark properly throws.
We just need to check aNode.itemId before invoking getLivemark.
Thanks for the stack!
Mentor: mak77
Whiteboard: [good first bug][lang=js]
(Assignee)

Comment 6

2 years ago
took it
Assignee: nobody → gasolin
(Assignee)

Comment 7

2 years ago
Created attachment 8743191 [details]
MozReview Request: Bug 1262639 - Fix NS_ERROR_ILLEGAL_VALUE spam by not run getLivemark without valid node id; r?mak

Review commit: https://reviewboard.mozilla.org/r/47635/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47635/
Attachment #8743191 - Flags: review?(mak77)
Attachment #8743191 - Flags: review?(mak77) → review+
Comment on attachment 8743191 [details]
MozReview Request: Bug 1262639 - Fix NS_ERROR_ILLEGAL_VALUE spam by not run getLivemark without valid node id; r?mak

https://reviewboard.mozilla.org/r/47635/#review44463

Thank you!
I used autoland from mozReview for the first time, let's see if it works :)
Keywords: checkin-needed
Keywords: checkin-needed

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c20f12a0e9ad
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.