Closed Bug 1262639 Opened 8 years ago Closed 8 years ago

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

Categories

(Toolkit :: Places, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: RyanVM, Assigned: gasolin, Mentored)

Details

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

Attachments

(1 file)

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
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]
took it
Assignee: nobody → gasolin
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
https://hg.mozilla.org/mozilla-central/rev/c20f12a0e9ad
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.