Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(316)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2154503002: Remove type info from CJS_Value, interrogate v8 instead (Closed)

Created:
4 years, 5 months ago by Tom Sepez
Modified:
4 years, 5 months ago
Reviewers:
Lei Zhang, dsinclair
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Remove type info from CJS_Value, interrogate v8 instead Committed: https://pdfium.googlesource.com/pdfium/+/40faa79da0bc902d7f1f5cc43464b94e6dfa5d22

Patch Set 1 #

Patch Set 2 : Remove alternate type interrogation #

Patch Set 3 : FXJS_GetTypeOf() now unused #

Patch Set 4 : VT_fxobject no longer used #

Total comments: 1

Patch Set 5 : No need to special case CJS_Document, its a CJS_Object already #

Total comments: 2

Patch Set 6 : nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -221 lines) Patch
M fpdfsdk/javascript/Document.cpp View 1 2 3 4 5 3 chunks +23 lines, -30 lines 0 comments Download
M fpdfsdk/javascript/JS_Define.h View 1 5 chunks +4 lines, -6 lines 0 comments Download
M fpdfsdk/javascript/JS_Value.h View 1 2 3 4 5 chunks +6 lines, -11 lines 0 comments Download
M fpdfsdk/javascript/JS_Value.cpp View 1 2 3 4 9 chunks +18 lines, -50 lines 0 comments Download
M fpdfsdk/javascript/app.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M fpdfsdk/javascript/global.cpp View 1 4 chunks +2 lines, -84 lines 0 comments Download
M fxjs/fxjs_v8.cpp View 1 2 2 chunks +0 lines, -29 lines 0 comments Download
M fxjs/include/fxjs_v8.h View 1 2 2 chunks +0 lines, -10 lines 0 comments Download

Messages

Total messages: 26 (19 generated)
Tom Sepez
Dan for review. CJS_Value should be removed in favor of v8::Value.
4 years, 5 months ago (2016-07-14 22:51:17 UTC) #6
Tom Sepez
One last patch, avoid potential type confusion which could arise if the enum said one ...
4 years, 5 months ago (2016-07-15 22:25:52 UTC) #16
Tom Sepez
https://codereview.chromium.org/2154503002/diff/60001/fpdfsdk/javascript/JS_Value.cpp File fpdfsdk/javascript/JS_Value.cpp (left): https://codereview.chromium.org/2154503002/diff/60001/fpdfsdk/javascript/JS_Value.cpp#oldcode199 fpdfsdk/javascript/JS_Value.cpp:199: m_eType = VT_object; e.g., instant type confusion if nullptr ...
4 years, 5 months ago (2016-07-15 22:27:43 UTC) #17
Lei Zhang
lgtm https://codereview.chromium.org/2154503002/diff/80001/fpdfsdk/javascript/Document.cpp File fpdfsdk/javascript/Document.cpp (right): https://codereview.chromium.org/2154503002/diff/80001/fpdfsdk/javascript/Document.cpp#newcode416 fpdfsdk/javascript/Document.cpp:416: if (FXJS_GetObjDefnID(pObj) == CJS_PrintParamsObj::g_nObjDefnID) { Weird indentation.
4 years, 5 months ago (2016-07-15 23:08:23 UTC) #20
Tom Sepez
https://codereview.chromium.org/2154503002/diff/80001/fpdfsdk/javascript/Document.cpp File fpdfsdk/javascript/Document.cpp (right): https://codereview.chromium.org/2154503002/diff/80001/fpdfsdk/javascript/Document.cpp#newcode416 fpdfsdk/javascript/Document.cpp:416: if (FXJS_GetObjDefnID(pObj) == CJS_PrintParamsObj::g_nObjDefnID) { On 2016/07/15 23:08:23, Lei ...
4 years, 5 months ago (2016-07-15 23:12:48 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2154503002/100001
4 years, 5 months ago (2016-07-15 23:13:01 UTC) #24
commit-bot: I haz the power
4 years, 5 months ago (2016-07-16 00:58:05 UTC) #26
Message was sent while issue was closed. 
Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/40faa79da0bc902d7f1f5cc43464b94e6dfa...

Powered by Google App Engine
This is Rietveld 408576698