From d3ae179dab3f63625e5dd3ddf0aa33176f8ee65c Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Tue, 16 Jan 2018 15:15:05 +0000 Subject: Cleanup CXFA_Fill This CL cleans up the CXFA_Fill internal getters to handle nullptr returns correctly and moves some of the logic to the specific XFA classes. Change-Id: Icac487105a026a25cc9981d00fbc152e459ad0b8 Reviewed-on: https://pdfium-review.googlesource.com/22770 Reviewed-by: Ryan Harrison Commit-Queue: dsinclair --- xfa/fxfa/parser/cxfa_color.cpp | 20 +++++++ xfa/fxfa/parser/cxfa_color.h | 6 +++ xfa/fxfa/parser/cxfa_fill.cpp | 113 ++++++++++++++++----------------------- xfa/fxfa/parser/cxfa_fill.h | 8 +-- xfa/fxfa/parser/cxfa_linear.cpp | 11 ++++ xfa/fxfa/parser/cxfa_linear.h | 7 +++ xfa/fxfa/parser/cxfa_pattern.cpp | 9 ++++ xfa/fxfa/parser/cxfa_pattern.h | 7 +++ xfa/fxfa/parser/cxfa_radial.cpp | 11 ++++ xfa/fxfa/parser/cxfa_radial.h | 5 ++ xfa/fxfa/parser/cxfa_stipple.cpp | 11 ++++ xfa/fxfa/parser/cxfa_stipple.h | 7 +++ 12 files changed, 143 insertions(+), 72 deletions(-) diff --git a/xfa/fxfa/parser/cxfa_color.cpp b/xfa/fxfa/parser/cxfa_color.cpp index a072d0302b..95c0ad345b 100644 --- a/xfa/fxfa/parser/cxfa_color.cpp +++ b/xfa/fxfa/parser/cxfa_color.cpp @@ -37,3 +37,23 @@ CXFA_Color::CXFA_Color(CXFA_Document* doc, XFA_PacketType packet) pdfium::MakeUnique(this)) {} CXFA_Color::~CXFA_Color() {} + +FX_ARGB CXFA_Color::GetValue() { + Optional val = JSObject()->TryCData(XFA_Attribute::Value, false); + return val ? StringToFXARGB(val->AsStringView()) : 0xFF000000; +} + +FX_ARGB CXFA_Color::GetValueOrDefault(FX_ARGB defaultValue) { + Optional val = JSObject()->TryCData(XFA_Attribute::Value, false); + return val ? StringToFXARGB(val->AsStringView()) : defaultValue; +} + +void CXFA_Color::SetValue(FX_ARGB color) { + int a; + int r; + int g; + int b; + std::tie(a, r, g, b) = ArgbDecode(color); + JSObject()->SetCData(XFA_Attribute::Value, + WideString::Format(L"%d,%d,%d", r, g, b), false, false); +} diff --git a/xfa/fxfa/parser/cxfa_color.h b/xfa/fxfa/parser/cxfa_color.h index 590bcc61da..b15c9d8806 100644 --- a/xfa/fxfa/parser/cxfa_color.h +++ b/xfa/fxfa/parser/cxfa_color.h @@ -11,8 +11,14 @@ class CXFA_Color : public CXFA_Node { public: + static constexpr FX_ARGB kBlackColor = 0xFF000000; + CXFA_Color(CXFA_Document* doc, XFA_PacketType packet); ~CXFA_Color() override; + + FX_ARGB GetValue(); + FX_ARGB GetValueOrDefault(FX_ARGB defaultValue); + void SetValue(FX_ARGB color); }; #endif // XFA_FXFA_PARSER_CXFA_COLOR_H_ diff --git a/xfa/fxfa/parser/cxfa_fill.cpp b/xfa/fxfa/parser/cxfa_fill.cpp index c62006754f..e1e077b5bb 100644 --- a/xfa/fxfa/parser/cxfa_fill.cpp +++ b/xfa/fxfa/parser/cxfa_fill.cpp @@ -60,31 +60,19 @@ bool CXFA_Fill::IsVisible() { } void CXFA_Fill::SetColor(FX_ARGB color) { - CXFA_Color* pNode = + CXFA_Color* pColor = JSObject()->GetOrCreateProperty(0, XFA_Element::Color); - if (!pNode) + if (!pColor) return; - int a; - int r; - int g; - int b; - std::tie(a, r, g, b) = ArgbDecode(color); - pNode->JSObject()->SetCData(XFA_Attribute::Value, - WideString::Format(L"%d,%d,%d", r, g, b), false, - false); + pColor->SetValue(color); } FX_ARGB CXFA_Fill::GetColor(bool bText) { - if (CXFA_Color* pNode = GetChild(0, XFA_Element::Color, false)) { - Optional wsColor = - pNode->JSObject()->TryCData(XFA_Attribute::Value, false); - if (wsColor) - return StringToFXARGB(wsColor->AsStringView()); - } - if (bText) - return 0xFF000000; - return 0xFFFFFFFF; + CXFA_Color* pColor = GetChild(0, XFA_Element::Color, false); + if (!pColor) + return bText ? 0xFF000000 : 0xFFFFFFFF; + return pColor->GetValueOrDefault(bText ? 0xFF000000 : 0xFFFFFFFF); } XFA_Element CXFA_Fill::GetFillType() const { @@ -100,86 +88,75 @@ XFA_Element CXFA_Fill::GetFillType() const { } XFA_AttributeEnum CXFA_Fill::GetPatternType() { - return GetPattern()->JSObject()->GetEnum(XFA_Attribute::Type); + CXFA_Pattern* pattern = GetPatternIfExists(); + return pattern ? pattern->GetType() : CXFA_Pattern::kDefaultType; } FX_ARGB CXFA_Fill::GetPatternColor() { - if (CXFA_Color* pColor = - GetPattern()->GetChild(0, XFA_Element::Color, false)) { - Optional wsColor = - pColor->JSObject()->TryCData(XFA_Attribute::Value, false); - if (wsColor) - return StringToFXARGB(wsColor->AsStringView()); - } - return 0xFF000000; + CXFA_Pattern* pattern = GetPatternIfExists(); + if (!pattern) + return CXFA_Color::kBlackColor; + + CXFA_Color* pColor = pattern->GetColorIfExists(); + return pColor ? pColor->GetValue() : CXFA_Color::kBlackColor; } int32_t CXFA_Fill::GetStippleRate() { - return GetStipple() - ->JSObject() - ->TryInteger(XFA_Attribute::Rate, true) - .value_or(50); + CXFA_Stipple* stipple = GetStippleIfExists(); + if (!stipple) + return CXFA_Stipple::GetDefaultRate(); + return stipple->GetRate(); } FX_ARGB CXFA_Fill::GetStippleColor() { - if (CXFA_Color* pColor = - GetStipple()->GetChild(0, XFA_Element::Color, false)) { - Optional wsColor = - pColor->JSObject()->TryCData(XFA_Attribute::Value, false); - if (wsColor) - return StringToFXARGB(wsColor->AsStringView()); - } - return 0xFF000000; + CXFA_Stipple* stipple = GetStippleIfExists(); + if (!stipple) + return CXFA_Color::kBlackColor; + + CXFA_Color* pColor = stipple->GetColorIfExists(); + return pColor ? pColor->GetValue() : CXFA_Color::kBlackColor; } XFA_AttributeEnum CXFA_Fill::GetLinearType() { - return GetLinear() - ->JSObject() - ->TryEnum(XFA_Attribute::Type, true) - .value_or(XFA_AttributeEnum::ToRight); + CXFA_Linear* linear = GetLinearIfExists(); + return linear ? linear->GetType() : CXFA_Linear::kDefaultType; } FX_ARGB CXFA_Fill::GetLinearColor() { - if (CXFA_Color* pColor = - GetLinear()->GetChild(0, XFA_Element::Color, false)) { - Optional wsColor = - pColor->JSObject()->TryCData(XFA_Attribute::Value, false); - if (wsColor) - return StringToFXARGB(wsColor->AsStringView()); - } - return 0xFF000000; + CXFA_Linear* linear = GetLinearIfExists(); + if (!linear) + return CXFA_Color::kBlackColor; + + CXFA_Color* pColor = linear->GetColorIfExists(); + return pColor ? pColor->GetValue() : CXFA_Color::kBlackColor; } bool CXFA_Fill::IsRadialToEdge() { - return GetRadial() - ->JSObject() - ->TryEnum(XFA_Attribute::Type, true) - .value_or(XFA_AttributeEnum::ToEdge) == XFA_AttributeEnum::ToEdge; + CXFA_Radial* radial = GetRadialIfExists(); + return radial ? radial->IsToEdge() : false; } FX_ARGB CXFA_Fill::GetRadialColor() { - if (CXFA_Color* pColor = - GetRadial()->GetChild(0, XFA_Element::Color, false)) { - Optional wsColor = - pColor->JSObject()->TryCData(XFA_Attribute::Value, false); - if (wsColor) - return StringToFXARGB(wsColor->AsStringView()); - } - return 0xFF000000; + CXFA_Radial* radial = GetRadialIfExists(); + if (!radial) + return CXFA_Color::kBlackColor; + + CXFA_Color* pColor = radial->GetColorIfExists(); + return pColor ? pColor->GetValue() : CXFA_Color::kBlackColor; } -CXFA_Stipple* CXFA_Fill::GetStipple() { +CXFA_Stipple* CXFA_Fill::GetStippleIfExists() { return JSObject()->GetOrCreateProperty(0, XFA_Element::Stipple); } -CXFA_Radial* CXFA_Fill::GetRadial() { +CXFA_Radial* CXFA_Fill::GetRadialIfExists() { return JSObject()->GetOrCreateProperty(0, XFA_Element::Radial); } -CXFA_Linear* CXFA_Fill::GetLinear() { +CXFA_Linear* CXFA_Fill::GetLinearIfExists() { return JSObject()->GetOrCreateProperty(0, XFA_Element::Linear); } -CXFA_Pattern* CXFA_Fill::GetPattern() { +CXFA_Pattern* CXFA_Fill::GetPatternIfExists() { return JSObject()->GetOrCreateProperty(0, XFA_Element::Pattern); } diff --git a/xfa/fxfa/parser/cxfa_fill.h b/xfa/fxfa/parser/cxfa_fill.h index a713047c5d..dc733f5a24 100644 --- a/xfa/fxfa/parser/cxfa_fill.h +++ b/xfa/fxfa/parser/cxfa_fill.h @@ -40,10 +40,10 @@ class CXFA_Fill : public CXFA_Node { FX_ARGB GetRadialColor(); private: - CXFA_Stipple* GetStipple(); - CXFA_Radial* GetRadial(); - CXFA_Linear* GetLinear(); - CXFA_Pattern* GetPattern(); + CXFA_Stipple* GetStippleIfExists(); + CXFA_Radial* GetRadialIfExists(); + CXFA_Linear* GetLinearIfExists(); + CXFA_Pattern* GetPatternIfExists(); }; #endif // XFA_FXFA_PARSER_CXFA_FILL_H_ diff --git a/xfa/fxfa/parser/cxfa_linear.cpp b/xfa/fxfa/parser/cxfa_linear.cpp index 5ca27916ae..f5b605dcb8 100644 --- a/xfa/fxfa/parser/cxfa_linear.cpp +++ b/xfa/fxfa/parser/cxfa_linear.cpp @@ -8,6 +8,7 @@ #include "fxjs/xfa/cjx_linear.h" #include "third_party/base/ptr_util.h" +#include "xfa/fxfa/parser/cxfa_color.h" namespace { @@ -38,3 +39,13 @@ CXFA_Linear::CXFA_Linear(CXFA_Document* doc, XFA_PacketType packet) pdfium::MakeUnique(this)) {} CXFA_Linear::~CXFA_Linear() {} + +XFA_AttributeEnum CXFA_Linear::GetType() { + return JSObject() + ->TryEnum(XFA_Attribute::Type, true) + .value_or(XFA_AttributeEnum::ToRight); +} + +CXFA_Color* CXFA_Linear::GetColorIfExists() { + return GetChild(0, XFA_Element::Color, false); +} diff --git a/xfa/fxfa/parser/cxfa_linear.h b/xfa/fxfa/parser/cxfa_linear.h index af1b8e8a28..d17f7ffcb2 100644 --- a/xfa/fxfa/parser/cxfa_linear.h +++ b/xfa/fxfa/parser/cxfa_linear.h @@ -9,10 +9,17 @@ #include "xfa/fxfa/parser/cxfa_node.h" +class CXFA_Color; + class CXFA_Linear : public CXFA_Node { public: + static constexpr XFA_AttributeEnum kDefaultType = XFA_AttributeEnum::ToRight; + CXFA_Linear(CXFA_Document* doc, XFA_PacketType packet); ~CXFA_Linear() override; + + XFA_AttributeEnum GetType(); + CXFA_Color* GetColorIfExists(); }; #endif // XFA_FXFA_PARSER_CXFA_LINEAR_H_ diff --git a/xfa/fxfa/parser/cxfa_pattern.cpp b/xfa/fxfa/parser/cxfa_pattern.cpp index 7328fd254c..3102f77beb 100644 --- a/xfa/fxfa/parser/cxfa_pattern.cpp +++ b/xfa/fxfa/parser/cxfa_pattern.cpp @@ -8,6 +8,7 @@ #include "fxjs/xfa/cjx_pattern.h" #include "third_party/base/ptr_util.h" +#include "xfa/fxfa/parser/cxfa_color.h" namespace { @@ -38,3 +39,11 @@ CXFA_Pattern::CXFA_Pattern(CXFA_Document* doc, XFA_PacketType packet) pdfium::MakeUnique(this)) {} CXFA_Pattern::~CXFA_Pattern() {} + +CXFA_Color* CXFA_Pattern::GetColorIfExists() { + return GetChild(0, XFA_Element::Color, false); +} + +XFA_AttributeEnum CXFA_Pattern::GetType() { + return JSObject()->GetEnum(XFA_Attribute::Type); +} diff --git a/xfa/fxfa/parser/cxfa_pattern.h b/xfa/fxfa/parser/cxfa_pattern.h index 44fb0af2b7..7e115f2d9d 100644 --- a/xfa/fxfa/parser/cxfa_pattern.h +++ b/xfa/fxfa/parser/cxfa_pattern.h @@ -9,10 +9,17 @@ #include "xfa/fxfa/parser/cxfa_node.h" +class CXFA_Color; + class CXFA_Pattern : public CXFA_Node { public: + static constexpr XFA_AttributeEnum kDefaultType = XFA_AttributeEnum::Unknown; + CXFA_Pattern(CXFA_Document* doc, XFA_PacketType packet); ~CXFA_Pattern() override; + + XFA_AttributeEnum GetType(); + CXFA_Color* GetColorIfExists(); }; #endif // XFA_FXFA_PARSER_CXFA_PATTERN_H_ diff --git a/xfa/fxfa/parser/cxfa_radial.cpp b/xfa/fxfa/parser/cxfa_radial.cpp index 5630943c58..80753b3803 100644 --- a/xfa/fxfa/parser/cxfa_radial.cpp +++ b/xfa/fxfa/parser/cxfa_radial.cpp @@ -8,6 +8,7 @@ #include "fxjs/xfa/cjx_radial.h" #include "third_party/base/ptr_util.h" +#include "xfa/fxfa/parser/cxfa_color.h" namespace { @@ -38,3 +39,13 @@ CXFA_Radial::CXFA_Radial(CXFA_Document* doc, XFA_PacketType packet) pdfium::MakeUnique(this)) {} CXFA_Radial::~CXFA_Radial() {} + +bool CXFA_Radial::IsToEdge() { + return JSObject() + ->TryEnum(XFA_Attribute::Type, true) + .value_or(XFA_AttributeEnum::ToEdge) == XFA_AttributeEnum::ToEdge; +} + +CXFA_Color* CXFA_Radial::GetColorIfExists() { + return GetChild(0, XFA_Element::Color, false); +} diff --git a/xfa/fxfa/parser/cxfa_radial.h b/xfa/fxfa/parser/cxfa_radial.h index 2316c459c7..e3a3645e0c 100644 --- a/xfa/fxfa/parser/cxfa_radial.h +++ b/xfa/fxfa/parser/cxfa_radial.h @@ -9,10 +9,15 @@ #include "xfa/fxfa/parser/cxfa_node.h" +class CXFA_Color; + class CXFA_Radial : public CXFA_Node { public: CXFA_Radial(CXFA_Document* doc, XFA_PacketType packet); ~CXFA_Radial() override; + + bool IsToEdge(); + CXFA_Color* GetColorIfExists(); }; #endif // XFA_FXFA_PARSER_CXFA_RADIAL_H_ diff --git a/xfa/fxfa/parser/cxfa_stipple.cpp b/xfa/fxfa/parser/cxfa_stipple.cpp index 17b64fa80b..24715171b8 100644 --- a/xfa/fxfa/parser/cxfa_stipple.cpp +++ b/xfa/fxfa/parser/cxfa_stipple.cpp @@ -8,6 +8,7 @@ #include "fxjs/xfa/cjx_stipple.h" #include "third_party/base/ptr_util.h" +#include "xfa/fxfa/parser/cxfa_color.h" namespace { @@ -37,3 +38,13 @@ CXFA_Stipple::CXFA_Stipple(CXFA_Document* doc, XFA_PacketType packet) pdfium::MakeUnique(this)) {} CXFA_Stipple::~CXFA_Stipple() {} + +CXFA_Color* CXFA_Stipple::GetColorIfExists() { + return GetChild(0, XFA_Element::Color, false); +} + +int32_t CXFA_Stipple::GetRate() { + return JSObject() + ->TryInteger(XFA_Attribute::Rate, true) + .value_or(GetDefaultRate()); +} diff --git a/xfa/fxfa/parser/cxfa_stipple.h b/xfa/fxfa/parser/cxfa_stipple.h index 2dddf4494d..3211e24d9e 100644 --- a/xfa/fxfa/parser/cxfa_stipple.h +++ b/xfa/fxfa/parser/cxfa_stipple.h @@ -9,10 +9,17 @@ #include "xfa/fxfa/parser/cxfa_node.h" +class CXFA_Color; + class CXFA_Stipple : public CXFA_Node { public: + static int32_t GetDefaultRate() { return 50; } + CXFA_Stipple(CXFA_Document* doc, XFA_PacketType packet); ~CXFA_Stipple() override; + + CXFA_Color* GetColorIfExists(); + int32_t GetRate(); }; #endif // XFA_FXFA_PARSER_CXFA_STIPPLE_H_ -- cgit v1.2.3