Bug 1281061. Do an atom equality compare instead of a string compare on the script filename string in SavedFrame::isSelfHosted. r=fitzgen
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 08 Jul 2016 22:53:53 -0400
changeset 304307 6e8cf178dae89be067d3f16d9fbd92aae08476b9
parent 304306 8b52e519d715082126c2b3caa063d50d8e88c9c3
child 304308 96875d7ae6f2f4cb0f56cd872eaae90345933563
push id30420
push userphilringnalda@gmail.com
push dateSat, 09 Jul 2016 15:52:07 +0000
treeherdermozilla-central@d55b1b1d51cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1281061
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1281061. Do an atom equality compare instead of a string compare on the script filename string in SavedFrame::isSelfHosted. r=fitzgen
devtools/shared/heapsnapshot/DeserializedNode.h
devtools/shared/heapsnapshot/HeapSnapshot.cpp
devtools/shared/heapsnapshot/tests/gtest/DeserializedStackFrameUbiStackFrames.cpp
js/public/UbiNode.h
js/src/jsapi-tests/testSavedStacks.cpp
js/src/vm/CommonPropertyNames.h
js/src/vm/SavedFrame.h
js/src/vm/SavedStacks.cpp
--- a/devtools/shared/heapsnapshot/DeserializedNode.h
+++ b/devtools/shared/heapsnapshot/DeserializedNode.h
@@ -291,17 +291,17 @@ public:
   static void construct(void* storage, DeserializedStackFrame* ptr) {
     new (storage) ConcreteStackFrame(ptr);
   }
 
   uint64_t identifier() const override { return get().id; }
   uint32_t line() const override { return get().line; }
   uint32_t column() const override { return get().column; }
   bool isSystem() const override { return get().isSystem; }
-  bool isSelfHosted() const override { return get().isSelfHosted; }
+  bool isSelfHosted(JSContext* cx) const override { return get().isSelfHosted; }
   void trace(JSTracer* trc) override { }
   AtomOrTwoByteChars source() const override {
     return AtomOrTwoByteChars(get().source);
   }
   AtomOrTwoByteChars functionDisplayName() const override {
     return AtomOrTwoByteChars(get().functionDisplayName);
   }
 
--- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp
+++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp
@@ -1160,17 +1160,17 @@ class MOZ_STACK_CLASS StreamWriter : pub
     auto data = MakeUnique<protobuf::StackFrame_Data>();
     if (!data)
       return nullptr;
 
     data->set_id(id);
     data->set_line(frame.line());
     data->set_column(frame.column());
     data->set_issystem(frame.isSystem());
-    data->set_isselfhosted(frame.isSelfHosted());
+    data->set_isselfhosted(frame.isSelfHosted(cx));
 
     auto dupeSource = TwoByteString::from(frame.source());
     if (!attachTwoByteString(dupeSource,
                              [&] (std::string* source) { data->set_allocated_source(source); },
                              [&] (uint64_t ref) { data->set_sourceref(ref); }))
     {
       return nullptr;
     }
@@ -1223,17 +1223,17 @@ public:
 
   virtual bool writeMetadata(uint64_t timestamp) final {
     protobuf::Metadata metadata;
     metadata.set_timestamp(timestamp);
     return writeMessage(metadata);
   }
 
   virtual bool writeNode(const JS::ubi::Node& ubiNode,
-                         EdgePolicy includeEdges) final {
+                         EdgePolicy includeEdges) override final {
     // NB: de-duplicated string properties must be written in the same order
     // here as they are read in `HeapSnapshot::saveNode` or else indices in
     // references to already serialized strings will be off.
 
     protobuf::Node protobufNode;
     protobufNode.set_id(ubiNode.identifier());
 
     protobufNode.set_coarsetype(JS::ubi::CoarseTypeToUint32(ubiNode.coarseType()));
--- a/devtools/shared/heapsnapshot/tests/gtest/DeserializedStackFrameUbiStackFrames.cpp
+++ b/devtools/shared/heapsnapshot/tests/gtest/DeserializedStackFrameUbiStackFrames.cpp
@@ -41,17 +41,17 @@ DEF_TEST(DeserializedStackFrameUbiStackF
 
     EXPECT_EQ(id, ubiFrame.identifier());
     EXPECT_EQ(JS::ubi::StackFrame(), ubiFrame.parent());
     EXPECT_EQ(line, ubiFrame.line());
     EXPECT_EQ(column, ubiFrame.column());
     EXPECT_EQ(JS::ubi::AtomOrTwoByteChars(source), ubiFrame.source());
     EXPECT_EQ(JS::ubi::AtomOrTwoByteChars(functionDisplayName),
               ubiFrame.functionDisplayName());
-    EXPECT_FALSE(ubiFrame.isSelfHosted());
+    EXPECT_FALSE(ubiFrame.isSelfHosted(cx));
     EXPECT_FALSE(ubiFrame.isSystem());
 
     JS::RootedObject savedFrame(cx);
     EXPECT_TRUE(ubiFrame.constructSavedFrameStack(cx, &savedFrame));
 
     uint32_t frameLine;
     ASSERT_EQ(JS::SavedFrameResult::Ok, JS::GetSavedFrameLine(cx, savedFrame, &frameLine));
     EXPECT_EQ(line, frameLine);
--- a/js/public/UbiNode.h
+++ b/js/public/UbiNode.h
@@ -254,17 +254,17 @@ class BaseStackFrame {
     virtual AtomOrTwoByteChars functionDisplayName() const = 0;
 
     // Returns true if this frame's function is system JavaScript running with
     // trusted principals, false otherwise.
     virtual bool isSystem() const = 0;
 
     // Return true if this frame's function is a self-hosted JavaScript builtin,
     // false otherwise.
-    virtual bool isSelfHosted() const = 0;
+    virtual bool isSelfHosted(JSContext* cx) const = 0;
 
     // Construct a SavedFrame stack for the stack starting with this frame and
     // containing all of its parents. The SavedFrame objects will be placed into
     // cx's current compartment.
     //
     // Note that the process of
     //
     //     SavedFrame
@@ -411,17 +411,17 @@ class StackFrame {
         return id;
     }
     uint32_t line() const { return base()->line(); }
     uint32_t column() const { return base()->column(); }
     AtomOrTwoByteChars source() const { return base()->source(); }
     AtomOrTwoByteChars functionDisplayName() const { return base()->functionDisplayName(); }
     StackFrame parent() const { return base()->parent(); }
     bool isSystem() const { return base()->isSystem(); }
-    bool isSelfHosted() const { return base()->isSelfHosted(); }
+    bool isSelfHosted(JSContext* cx) const { return base()->isSelfHosted(cx); }
     MOZ_MUST_USE bool constructSavedFrameStack(JSContext* cx,
                                                MutableHandleObject outSavedFrameStack) const {
         return base()->constructSavedFrameStack(cx, outSavedFrameStack);
     }
 
     struct HashPolicy {
         using Lookup = JS::ubi::StackFrame;
 
@@ -458,17 +458,17 @@ class ConcreteStackFrame<void> : public 
     }
 
     uint32_t line() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
     uint32_t column() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
     AtomOrTwoByteChars source() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
     AtomOrTwoByteChars functionDisplayName() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
     StackFrame parent() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
     bool isSystem() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
-    bool isSelfHosted() const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
+    bool isSelfHosted(JSContext* cx) const override { MOZ_CRASH("null JS::ubi::StackFrame"); }
 };
 
 MOZ_MUST_USE bool ConstructSavedFrameStackSlow(JSContext* cx, JS::ubi::StackFrame& frame,
                                                MutableHandleObject outSavedFrameStack);
 
 
 /*** ubi::Node ************************************************************************************/
 
--- a/js/src/jsapi-tests/testSavedStacks.cpp
+++ b/js/src/jsapi-tests/testSavedStacks.cpp
@@ -134,17 +134,17 @@ BEGIN_TEST(testSavedStacks_selfHostedFra
 
     CHECK(val.isObject());
     JS::RootedObject obj(cx, &val.toObject());
 
     CHECK(obj->is<js::SavedFrame>());
     JS::Rooted<js::SavedFrame*> savedFrame(cx, &obj->as<js::SavedFrame>());
 
     JS::Rooted<js::SavedFrame*> selfHostedFrame(cx, savedFrame->getParent());
-    CHECK(selfHostedFrame->isSelfHosted());
+    CHECK(selfHostedFrame->isSelfHosted(cx));
 
     // Source
     JS::RootedString str(cx);
     JS::SavedFrameResult result = JS::GetSavedFrameSource(cx, selfHostedFrame, &str,
                                                           JS::SavedFrameSelfHosted::Exclude);
     CHECK(result == JS::SavedFrameResult::Ok);
     JSLinearString* lin = str->ensureLinear(cx);
     CHECK(lin);
--- a/js/src/vm/CommonPropertyNames.h
+++ b/js/src/vm/CommonPropertyNames.h
@@ -316,10 +316,13 @@
     macro(function, function, "function") \
     macro(string, string, "string") \
     macro(number, number, "number") \
     macro(boolean, boolean, "boolean") \
     macro(null, null, "null") \
     macro(symbol, symbol, "symbol") \
     /* 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(selfHosted, selfHosted, "self-hosted") \
 
 #endif /* vm_CommonPropertyNames_h */
--- a/js/src/vm/SavedFrame.h
+++ b/js/src/vm/SavedFrame.h
@@ -44,17 +44,17 @@ class SavedFrame : public NativeObject {
     // Convenient getters for SavedFrame's reserved slots for use from C++.
     JSAtom*       getSource();
     uint32_t      getLine();
     uint32_t      getColumn();
     JSAtom*       getFunctionDisplayName();
     JSAtom*       getAsyncCause();
     SavedFrame*   getParent() const;
     JSPrincipals* getPrincipals();
-    bool          isSelfHosted();
+    bool          isSelfHosted(JSContext* cx);
 
     // Iterators for use with C++11 range based for loops, eg:
     //
     //     SavedFrame* stack = getSomeSavedFrameStack();
     //     for (const SavedFrame* frame : *stack) {
     //         ...
     //     }
     //
@@ -291,17 +291,19 @@ class ConcreteStackFrame<SavedFrame> : p
     void trace(JSTracer* trc) override {
         JSObject* prev = &get();
         JSObject* next = prev;
         js::TraceRoot(trc, &next, "ConcreteStackFrame<SavedFrame>::ptr");
         if (next != prev)
             ptr = next;
     }
 
-    bool isSelfHosted() const override { return get().isSelfHosted(); }
+    bool isSelfHosted(JSContext* cx) const override {
+        return get().isSelfHosted(cx);
+    }
 
     bool isSystem() const override;
 
     MOZ_MUST_USE bool constructSavedFrameStack(JSContext* cx,
                                                MutableHandleObject outSavedFrameStack)
         const override;
 };
 
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -506,20 +506,20 @@ SavedFrame::create(JSContext* cx)
                                                       TenuredObject));
     if (!frameObj)
         return nullptr;
 
     return &frameObj->as<SavedFrame>();
 }
 
 bool
-SavedFrame::isSelfHosted()
+SavedFrame::isSelfHosted(JSContext* cx)
 {
     JSAtom* source = getSource();
-    return StringEqualsAscii(source, "self-hosted");
+    return source == cx->names().selfHosted;
 }
 
 /* static */ bool
 SavedFrame::construct(JSContext* cx, unsigned argc, Value* vp)
 {
     JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NO_CONSTRUCTOR,
                          "SavedFrame");
     return false;
@@ -555,17 +555,18 @@ SavedFrameSubsumedByCaller(JSContext* cx
 static SavedFrame*
 GetFirstSubsumedFrame(JSContext* cx, HandleSavedFrame frame, JS::SavedFrameSelfHosted selfHosted,
                       bool& skippedAsync)
 {
     skippedAsync = false;
 
     RootedSavedFrame rootedFrame(cx, frame);
     while (rootedFrame) {
-        if ((selfHosted == JS::SavedFrameSelfHosted::Include || !rootedFrame->isSelfHosted()) &&
+        if ((selfHosted == JS::SavedFrameSelfHosted::Include ||
+             !rootedFrame->isSelfHosted(cx)) &&
             SavedFrameSubsumedByCaller(cx, rootedFrame))
         {
             return rootedFrame;
         }
 
         if (rootedFrame->getAsyncCause())
             skippedAsync = true;
 
@@ -897,17 +898,17 @@ BuildStackString(JSContext* cx, HandleOb
         if (!frame) {
             stringp.set(cx->runtime()->emptyString);
             return true;
         }
 
         js::RootedSavedFrame parent(cx);
         do {
             MOZ_ASSERT(SavedFrameSubsumedByCaller(cx, frame));
-            MOZ_ASSERT(!frame->isSelfHosted());
+            MOZ_ASSERT(!frame->isSelfHosted(cx));
 
             RootedString asyncCause(cx, frame->getAsyncCause());
             if (!asyncCause && skippedAsync)
                 asyncCause.set(cx->names().Async);
 
             js::RootedAtom name(cx, frame->getFunctionDisplayName());
             if ((indent && !sb.appendN(' ', indent))
                 || (asyncCause && (!sb.append(asyncCause) || !sb.append('*')))