We have an atom, so should probably just do an atom compare.
Comment on attachment 8763694 [details] [diff] [review] Do an atom equality compare instead of a string compare on the script filename string in SavedFrame::isSelfHosted Review of attachment 8763694 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! Since the JS::ubi::StackFrame interface changed, you will need to update the other implementation as well: https://dxr.mozilla.org/mozilla-central/rev/2ab57664bf7cafdd64e136e341527c275fc8c3aa/devtools/shared/heapsnapshot/DeserializedNode.h#299 ::: js/src/vm/CommonPropertyNames.h @@ +322,5 @@ > /* Function names for properties named by symbols. */ \ > macro(Symbol_iterator_fun, Symbol_iterator_fun, "[Symbol.iterator]") \ > + /* Not really a property name, but we use this to compare to atomized > + script source strings */ \ > + macro(self_hosted, self_hosted, "self-hosted") \ s/self_hosted/selfHosted/ We turn "foo-bar" into "fooBar" for other property names, so we should be consistent here.  https://dxr.mozilla.org/mozilla-central/rev/2ab57664bf7cafdd64e136e341527c275fc8c3aa/js/src/vm/CommonPropertyNames.h#120
Attachment #8763694 - Flags: review?(nfitzgerald) → review+
> you will need to update the other implementation as well Huh. Could have sworn I'd done a full-tree compile with this, but clearly not! > s/self_hosted/selfHosted/ Done.
But ok, now I see that HeapSnapshot.cpp has no cx in sight and wants to call isSelfHosted().. is that correct? :(
`frame.isSelfHosted()` `-> is called by `getProtobufStackFrame` `-> is called by `StreamWriter::writeNode` as part of the `CoreDumpWriter` interface `-> is called by `HeapSnapshotTraversalHandler::operator()` `-> `HeapSnapshotTraversalHandler` is initialized here and has access to a cx at that time So, that's a lot of threading cx through functions and classes to add, but is mostly furious typing without thinking.  https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/devtools/shared/heapsnapshot/HeapSnapshot.cpp#1395 One potential complication I thought of that turns out to be ok: we do NOT need to recanonicalize a deserialized heap snapshot's stack frame's source to the "self-hosted" atom if the stack frame's source is "self-hosted" because we actually serialize and deserialize the isSelfHosted data separately and we use that for JS::ubi::Concrete<DeserializedStackFrame>::isSelfHosted.  https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/devtools/shared/heapsnapshot/DeserializedNode.h#299
Oh, I'm an idiot. StreamWriter has a JSContext* _member_ that I can use. I'll just do that.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6e8cf178dae8 Do an atom equality compare instead of a string compare on the script filename string in SavedFrame::isSelfHosted. r=fitzgen
You need to log in before you can comment on or make changes to this bug.