Skip to content

Commit

Permalink
Bug 1847469 - Part 9: Use column number types in SavedFrame API. r=iain
Browse files Browse the repository at this point in the history
  • Loading branch information
arai-a committed Aug 16, 2023
1 parent f2974ab commit 0ceff7d
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ DEF_TEST(DeserializedStackFrameUbiStackFrames, {
JS::GetSavedFrameLine(cx, principals, savedFrame, &frameLine));
EXPECT_EQ(line, frameLine);

uint32_t frameColumn;
JS::TaggedColumnNumberOneOrigin frameColumn;
ASSERT_EQ(JS::SavedFrameResult::Ok,
JS::GetSavedFrameColumn(cx, principals, savedFrame, &frameColumn));
EXPECT_EQ(column.oneOriginValue(), frameColumn);
EXPECT_EQ(column, frameColumn);

JS::Rooted<JSObject*> parent(cx);
ASSERT_EQ(JS::SavedFrameResult::Ok,
Expand Down
8 changes: 5 additions & 3 deletions dom/base/ChromeUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "JSOracleParent.h"
#include "js/CallAndConstruct.h" // JS::Call
#include "js/ColumnNumber.h" // JS::TaggedColumnNumberOneOrigin
#include "js/CharacterEncoding.h"
#include "js/Object.h" // JS::GetClass
#include "js/PropertyAndElement.h" // JS_DefineProperty, JS_DefinePropertyById, JS_Enumerate, JS_GetProperty, JS_GetPropertyById, JS_SetProperty, JS_SetPropertyById, JS::IdVector
Expand Down Expand Up @@ -1596,7 +1597,7 @@ void ChromeUtils::CreateError(const GlobalObject& aGlobal,
{
JS::Rooted<JSString*> fileName(cx, JS_GetEmptyString(cx));
uint32_t line = 0;
uint32_t column = 0;
JS::TaggedColumnNumberOneOrigin column;

Maybe<JSAutoRealm> ar;
JS::Rooted<JSObject*> stack(cx);
Expand Down Expand Up @@ -1626,8 +1627,9 @@ void ChromeUtils::CreateError(const GlobalObject& aGlobal,
}

JS::Rooted<JS::Value> err(cx);
if (!JS::CreateError(cx, JSEXN_ERR, stack, fileName, line, column, nullptr,
message, JS::NothingHandleValue, &err)) {
if (!JS::CreateError(cx, JSEXN_ERR, stack, fileName, line,
column.oneOriginValue(), nullptr, message,
JS::NothingHandleValue, &err)) {
return;
}

Expand Down
7 changes: 4 additions & 3 deletions dom/bindings/Exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "mozilla/dom/Exceptions.h"

#include "js/ColumnNumber.h" // JS::TaggedColumnNumberOneOrigin
#include "js/RootingAPI.h"
#include "js/TypeDecls.h"
#include "jsapi.h"
Expand Down Expand Up @@ -518,7 +519,7 @@ int32_t JSStackFrame::GetColumnNumber(JSContext* aCx) {
return 0;
}

uint32_t col;
JS::TaggedColumnNumberOneOrigin col;
bool canCache = false, useCachedValue = false;
GetValueIfNotCached(aCx, mStack, JS::GetSavedFrameColumn, mColNoInitialized,
&canCache, &useCachedValue, &col);
Expand All @@ -528,11 +529,11 @@ int32_t JSStackFrame::GetColumnNumber(JSContext* aCx) {
}

if (canCache) {
mColNo = col;
mColNo = col.oneOriginValue();
mColNoInitialized = true;
}

return col;
return col.oneOriginValue();
}

NS_IMETHODIMP
Expand Down
6 changes: 3 additions & 3 deletions js/public/SavedFrameAPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include "jstypes.h" // JS_PUBLIC_API

#include "js/ColumnNumber.h" // JS::TaggedColumnNumberOneOrigin
#include "js/TypeDecls.h"

struct JSPrincipals;
Expand Down Expand Up @@ -82,12 +83,11 @@ extern JS_PUBLIC_API SavedFrameResult GetSavedFrameLine(
SavedFrameSelfHosted selfHosted = SavedFrameSelfHosted::Include);

/**
* Given a SavedFrame JSObject, get its column property (1-origin).
* Defaults to 0.
* Given a SavedFrame JSObject, get its column property. Defaults to 0.
*/
extern JS_PUBLIC_API SavedFrameResult GetSavedFrameColumn(
JSContext* cx, JSPrincipals* principals, Handle<JSObject*> savedFrame,
uint32_t* columnp,
JS::TaggedColumnNumberOneOrigin* columnp,
SavedFrameSelfHosted selfHosted = SavedFrameSelfHosted::Include);

/**
Expand Down
14 changes: 8 additions & 6 deletions js/src/jsapi-tests/testSavedStacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "mozilla/Utf8.h" // mozilla::Utf8Unit

#include "builtin/TestingFunctions.h"
#include "js/ColumnNumber.h" // JS::LimitedColumnNumberOneOrigin, JS::TaggedColumnNumberOneOrigin
#include "js/CompilationAndEvaluation.h" // JS::Evaluate
#include "js/Exception.h"
#include "js/SavedFrameAPI.h"
Expand Down Expand Up @@ -46,10 +47,10 @@ BEGIN_TEST(testSavedStacks_ApiDefaultValues) {
CHECK(line == 0);

// Column
uint32_t column = 123;
JS::TaggedColumnNumberOneOrigin column(JS::LimitedColumnNumberOneOrigin(123));
result = JS::GetSavedFrameColumn(cx, principals, savedFrame, &column);
CHECK(result == JS::SavedFrameResult::AccessDenied);
CHECK(column == 0);
CHECK(column == JS::TaggedColumnNumberOneOrigin());

// Function display name
result =
Expand Down Expand Up @@ -256,11 +257,11 @@ BEGIN_TEST(testSavedStacks_selfHostedFrames) {
CHECK_EQUAL(line, 3U);

// Column
uint32_t column = 123;
JS::TaggedColumnNumberOneOrigin column(JS::LimitedColumnNumberOneOrigin(123));
result = JS::GetSavedFrameColumn(cx, principals, selfHostedFrame, &column,
JS::SavedFrameSelfHosted::Exclude);
CHECK(result == JS::SavedFrameResult::Ok);
CHECK_EQUAL(column, 9U);
CHECK_EQUAL(column.oneOriginValue(), 9U);

// Function display name
result = JS::GetSavedFrameFunctionDisplayName(
Expand Down Expand Up @@ -360,11 +361,12 @@ BEGIN_TEST(test_GetPendingExceptionStack) {
CHECK_EQUAL(line, expected[i].line);

// Column
uint32_t column = 123;
JS::TaggedColumnNumberOneOrigin column(
JS::LimitedColumnNumberOneOrigin(123));
result = JS::GetSavedFrameColumn(cx, principals, frame, &column,
JS::SavedFrameSelfHosted::Exclude);
CHECK(result == JS::SavedFrameResult::Ok);
CHECK_EQUAL(column, expected[i].column);
CHECK_EQUAL(column.oneOriginValue(), expected[i].column);

// Source
JS::RootedString str(cx);
Expand Down
10 changes: 5 additions & 5 deletions js/src/vm/SavedStacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ JS_PUBLIC_API SavedFrameResult GetSavedFrameLine(

JS_PUBLIC_API SavedFrameResult GetSavedFrameColumn(
JSContext* cx, JSPrincipals* principals, HandleObject savedFrame,
uint32_t* columnp,
JS::TaggedColumnNumberOneOrigin* columnp,
SavedFrameSelfHosted selfHosted /* = SavedFrameSelfHosted::Include */) {
js::AssertHeapIsIdle();
CHECK_THREAD(cx);
Expand All @@ -840,10 +840,10 @@ JS_PUBLIC_API SavedFrameResult GetSavedFrameColumn(
Rooted<js::SavedFrame*> frame(cx, UnwrapSavedFrame(cx, principals, savedFrame,
selfHosted, skippedAsync));
if (!frame) {
*columnp = 0;
*columnp = JS::TaggedColumnNumberOneOrigin();
return SavedFrameResult::AccessDenied;
}
*columnp = frame->getColumn().rawValue();
*columnp = frame->getColumn();
return SavedFrameResult::Ok;
}

Expand Down Expand Up @@ -1225,10 +1225,10 @@ bool SavedFrame::lineProperty(JSContext* cx, unsigned argc, Value* vp) {
bool SavedFrame::columnProperty(JSContext* cx, unsigned argc, Value* vp) {
THIS_SAVEDFRAME(cx, argc, vp, "(get column)", args, frame);
JSPrincipals* principals = cx->realm()->principals();
uint32_t column;
JS::TaggedColumnNumberOneOrigin column;
if (JS::GetSavedFrameColumn(cx, principals, frame, &column) ==
JS::SavedFrameResult::Ok) {
args.rval().setNumber(column);
args.rval().setNumber(column.oneOriginValue());
} else {
args.rval().setNull();
}
Expand Down

0 comments on commit 0ceff7d

Please sign in to comment.