Fix the way to access marked content.

Fix the way to access marked content.

When there is no dictionary for marked content, it potientially may cause crash. But it is not happening now since 1) we now check for the returned dict parameter 2) the alloc function in pdfium does zero initialization.

BUG=pdfium:67
R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/42a1bc02c0810c039afbcb62170c326f0e717320

[Wei Li, 2016-02-12 23:19:50]

Description was changed from

==========
Fix the way to access marked content.

When there is no dictionary for marked content, it potientially may
cause crash. But it is not happening now since 1) we now check for the
return dict parameter 2) the alloc function in pdfium does zero
initialization.

BUG=pdfium:67
==========

to

==========
Fix the way to access marked content.

When there is no dictionary for marked content, it potientially may cause crash.
But it is not happening now since 1) we now check for the returned dict
parameter 2) the alloc function in pdfium does zero initialization.

BUG=pdfium:67
==========

[Wei Li, 2016-02-12 23:21:10]

weili@chromium.org changed reviewers:
+ thestig@chromium.org

[Wei Li, 2016-02-12 23:21:10]

PTAL, thanks

[Lei Zhang, 2016-02-12 23:53:27]

lgtm

https://codereview.chromium.org/1695633004/diff/20001/core/include/fpdfapi/fp...
File core/include/fpdfapi/fpdf_pageobj.h (right):

https://codereview.chromium.org/1695633004/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_pageobj.h:268: typedef enum { None, PropertiesDict,
DirectDict } ParamType;
enum ParamType { FOO, BAR };

https://codereview.chromium.org/1695633004/diff/20001/core/src/fpdftext/fpdf_...
File core/src/fpdftext/fpdf_text_int.cpp (right):

https://codereview.chromium.org/1695633004/diff/20001/core/src/fpdftext/fpdf_...
core/src/fpdftext/fpdf_text_int.cpp:1144: pDict =
ToDictionary(static_cast<CPDF_Object*>(item.GetParam()));
BTW, can we change GetParam() to return a CPDF_Object*, instead of casting
everywhere?

https://codereview.chromium.org/1695633004/diff/20001/core/src/fpdftext/fpdf_...
core/src/fpdftext/fpdf_text_int.cpp:1170: if (-1 !=
pFont->CharCodeFromUnicode(actText.GetAt(i))) {
Flip this to foo != -1 while we are here?

[Wei Li, 2016-02-13 01:29:19]

thanks

https://codereview.chromium.org/1695633004/diff/20001/core/include/fpdfapi/fp...
File core/include/fpdfapi/fpdf_pageobj.h (right):

https://codereview.chromium.org/1695633004/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_pageobj.h:268: typedef enum { None, PropertiesDict,
DirectDict } ParamType;
On 2016/02/12 23:53:27, Lei Zhang wrote:
> enum ParamType { FOO, BAR };

Done.

https://codereview.chromium.org/1695633004/diff/20001/core/src/fpdftext/fpdf_...
File core/src/fpdftext/fpdf_text_int.cpp (right):

https://codereview.chromium.org/1695633004/diff/20001/core/src/fpdftext/fpdf_...
core/src/fpdftext/fpdf_text_int.cpp:1144: pDict =
ToDictionary(static_cast<CPDF_Object*>(item.GetParam()));
On 2016/02/12 23:53:27, Lei Zhang wrote:
> BTW, can we change GetParam() to return a CPDF_Object*, instead of casting
> everywhere?

Good idea. Now the code looks much cleaner. :)

https://codereview.chromium.org/1695633004/diff/20001/core/src/fpdftext/fpdf_...
core/src/fpdftext/fpdf_text_int.cpp:1170: if (-1 !=
pFont->CharCodeFromUnicode(actText.GetAt(i))) {
On 2016/02/12 23:53:27, Lei Zhang wrote:
> Flip this to foo != -1 while we are here?

Done.

[Lei Zhang, 2016-02-13 01:34:55]

https://codereview.chromium.org/1695633004/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_page/fpdf_page_graph_state.cpp (right):

https://codereview.chromium.org/1695633004/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_page/fpdf_page_graph_state.cpp:597: m_pParam =
ToDictionary(src.m_pParam->Clone());
Do you still need the ToDictionary?

[Wei Li, 2016-02-13 01:41:20]

https://codereview.chromium.org/1695633004/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_page/fpdf_page_graph_state.cpp (right):

https://codereview.chromium.org/1695633004/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_page/fpdf_page_graph_state.cpp:597: m_pParam =
ToDictionary(src.m_pParam->Clone());
On 2016/02/13 01:34:55, Lei Zhang wrote:
> Do you still need the ToDictionary?

Either ToDictionary or static_cast since Clone() returns PDF_Object*.

[Lei Zhang, 2016-02-13 01:43:24]

On 2016/02/13 01:41:20, Wei Li wrote:
>
https://codereview.chromium.org/1695633004/diff/40001/core/src/fpdfapi/fpdf_p...
> File core/src/fpdfapi/fpdf_page/fpdf_page_graph_state.cpp (right):
> 
>
https://codereview.chromium.org/1695633004/diff/40001/core/src/fpdfapi/fpdf_p...
> core/src/fpdfapi/fpdf_page/fpdf_page_graph_state.cpp:597: m_pParam =
> ToDictionary(src.m_pParam->Clone());
> On 2016/02/13 01:34:55, Lei Zhang wrote:
> > Do you still need the ToDictionary?
> 
> Either ToDictionary or static_cast since Clone() returns PDF_Object*.

Ah, ok, n/m.

[Wei Li, 2016-02-13 02:21:30]

Description was changed from

==========
Fix the way to access marked content.

When there is no dictionary for marked content, it potientially may cause crash.
But it is not happening now since 1) we now check for the returned dict
parameter 2) the alloc function in pdfium does zero initialization.

BUG=pdfium:67
==========

to

==========
Fix the way to access marked content.

When there is no dictionary for marked content, it potientially may cause crash.
But it is not happening now since 1) we now check for the returned dict
parameter 2) the alloc function in pdfium does zero initialization.

BUG=pdfium:67
R=thestig@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/42a1bc02c0810c039afbcb62170c326f0e71...
==========

[Wei Li, 2016-02-13 02:21:31]

Committed patchset #3 (id:40001) manually as
42a1bc02c0810c039afbcb62170c326f0e717320 (presubmit successful).