commit 24393db4986d5a87a23848f7ee906fd5853df0fe
author Lei Zhang <thestig@chromium.org> Wed Jan 09 19:29:35 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Jan 09 19:29:35 2019 +0000
tree 89b25fa93670e924bc6ae582b2b89e492321ac42
parent 399958ef38caf620316a70a7ab517d2601264d52

Split CPDF_FormField::SetItemSelection().

Move parts of implementation into helper methods and use early returns
in the helpers.

Change-Id: I8055bf87d09435c4ed52b0daf057de1ec96baf9b
Reviewed-on: https://pdfium-review.googlesource.com/c/47730
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

core/fpdfdoc/cpdf_formfield.cpp

diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index d09a5b5..1c9ea00 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -567,53 +567,72 @@
       !NotifyListOrComboBoxBeforeChange(opt_value)) {
     return false;
   }
-  if (bSelected) {
-    if (GetType() == kListBox) {
-      SelectOption(index, true, NotificationOption::kDoNotNotify);
-      if (!(m_Flags & kFormListMultiSelect)) {
-        m_pDict->SetNewFor<CPDF_String>("V", opt_value);
-      } else {
-        CPDF_Array* pArray = m_pDict->SetNewFor<CPDF_Array>("V");
-        for (int i = 0; i < CountOptions(); i++) {
-          if (i == index || IsItemSelected(i))
-            pArray->AddNew<CPDF_String>(GetOptionValue(i));
-        }
-      }
-    } else {
-      m_pDict->SetNewFor<CPDF_String>("V", opt_value);
-      CPDF_Array* pI = m_pDict->SetNewFor<CPDF_Array>("I");
-      pI->AddNew<CPDF_Number>(index);
-    }
-  } else {
-    const CPDF_Object* pValue = FPDF_GetFieldAttr(m_pDict.Get(), "V");
-    if (pValue) {
-      if (GetType() == kListBox) {
-        SelectOption(index, false, NotificationOption::kDoNotNotify);
-        if (pValue->IsString()) {
-          if (pValue->GetUnicodeText() == opt_value)
-            m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
-        } else if (pValue->IsArray()) {
-          auto pArray = pdfium::MakeUnique<CPDF_Array>();
-          for (int i = 0; i < CountOptions(); i++) {
-            if (i != index && IsItemSelected(i))
-              pArray->AddNew<CPDF_String>(GetOptionValue(i));
-          }
-          if (pArray->size() > 0) {
-            m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
-            m_pDict->SetFor("V", std::move(pArray));
-          }
-        }
-      } else {
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
-        m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
-      }
-    }
-  }
+
+  if (bSelected)
+    SetItemSelectionSelected(index, opt_value);
+  else
+    SetItemSelectionUnselected(index, opt_value);
+
   if (notify == NotificationOption::kNotify)
     NotifyListOrComboBoxAfterChange();
   return true;
 }
 
+void CPDF_FormField::SetItemSelectionSelected(int index,
+                                              const WideString& opt_value) {
+  if (GetType() != kListBox) {
+    m_pDict->SetNewFor<CPDF_String>("V", opt_value);
+    CPDF_Array* pI = m_pDict->SetNewFor<CPDF_Array>("I");
+    pI->AddNew<CPDF_Number>(index);
+    return;
+  }
+
+  SelectOption(index, true, NotificationOption::kDoNotNotify);
+  if (!(m_Flags & kFormListMultiSelect)) {
+    m_pDict->SetNewFor<CPDF_String>("V", opt_value);
+    return;
+  }
+
+  CPDF_Array* pArray = m_pDict->SetNewFor<CPDF_Array>("V");
+  for (int i = 0; i < CountOptions(); i++) {
+    if (i == index || IsItemSelected(i))
+      pArray->AddNew<CPDF_String>(GetOptionValue(i));
+  }
+}
+
+void CPDF_FormField::SetItemSelectionUnselected(int index,
+                                                const WideString& opt_value) {
+  const CPDF_Object* pValue = FPDF_GetFieldAttr(m_pDict.Get(), "V");
+  if (!pValue)
+    return;
+
+  if (GetType() != kListBox) {
+    m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+    m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("I"));
+    return;
+  }
+
+  SelectOption(index, false, NotificationOption::kDoNotNotify);
+  if (pValue->IsString()) {
+    if (pValue->GetUnicodeText() == opt_value)
+      m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+    return;
+  }
+
+  if (!pValue->IsArray())
+    return;
+
+  auto pArray = pdfium::MakeUnique<CPDF_Array>();
+  for (int i = 0; i < CountOptions(); i++) {
+    if (i != index && IsItemSelected(i))
+      pArray->AddNew<CPDF_String>(GetOptionValue(i));
+  }
+  if (pArray->size() > 0) {
+    m_pForm->GetDocument()->AddOrphan(m_pDict->RemoveFor("V"));
+    m_pDict->SetFor("V", std::move(pArray));
+  }
+}
+
 bool CPDF_FormField::IsItemDefaultSelected(int index) const {
   ASSERT(GetType() == kComboBox || GetType() == kListBox);
   if (index < 0 || index >= CountOptions())

core/fpdfdoc/cpdf_formfield.h

diff --git a/core/fpdfdoc/cpdf_formfield.h b/core/fpdfdoc/cpdf_formfield.h
index 2c525cb..e484282 100644
--- a/core/fpdfdoc/cpdf_formfield.h
+++ b/core/fpdfdoc/cpdf_formfield.h
@@ -184,6 +184,8 @@
   bool SetCheckValue(const WideString& value,
                      bool bDefault,
                      NotificationOption notify);
+  void SetItemSelectionSelected(int index, const WideString& opt_value);
+  void SetItemSelectionUnselected(int index, const WideString& opt_value);
   bool NotifyBeforeSelectionChange(const WideString& value);
   void NotifyAfterSelectionChange();
   bool NotifyBeforeValueChange(const WideString& value);