Factor out the duplicated logic in Field::SetDisplay

fpdfsdk/javascript/Field.cpp

Index: fpdfsdk/javascript/Field.cpp
diff --git a/fpdfsdk/javascript/Field.cpp b/fpdfsdk/javascript/Field.cpp
index 1f4a18375adfe9ca2355375070c97d8c28cda630..bf20769be43b70353686ab1f2723235230d9436b 100644
--- a/fpdfsdk/javascript/Field.cpp
+++ b/fpdfsdk/javascript/Field.cpp
@@ -26,6 +26,44 @@
 #include "fpdfsdk/javascript/cjs_runtime.h"
 #include "fpdfsdk/javascript/color.h"
 
+static bool SetWidgetDisplayStatus(CPDFSDK_Widget* pWidget, int value) {

[Tom Sepez, 2016/08/17 17:33:31]

nit: prefer

namespace {

bool SetW...

}  // Namespace

[tonikitoo, 2016/08/17 17:51:53]

Done.

+  if (!pWidget)
+    return false;
+
+  uint32_t dwFlag = pWidget->GetFlags();
+  switch (value) {
+    case 0:
+      dwFlag &= (~ANNOTFLAG_INVISIBLE);

[Tom Sepez, 2016/08/17 17:33:31]

nit: the () aren't strictly necessary here.

[tonikitoo, 2016/08/17 17:51:53]

Done.

+      dwFlag &= (~ANNOTFLAG_HIDDEN);
+      dwFlag &= (~ANNOTFLAG_NOVIEW);
+      dwFlag |= ANNOTFLAG_PRINT;
+      break;
+    case 1:
+      dwFlag &= (~ANNOTFLAG_INVISIBLE);
+      dwFlag &= (~ANNOTFLAG_NOVIEW);
+      dwFlag |= (ANNOTFLAG_HIDDEN | ANNOTFLAG_PRINT);
+      break;
+    case 2:
+      dwFlag &= (~ANNOTFLAG_INVISIBLE);
+      dwFlag &= (~ANNOTFLAG_PRINT);
+      dwFlag &= (~ANNOTFLAG_HIDDEN);
+      dwFlag &= (~ANNOTFLAG_NOVIEW);
+      break;
+    case 3:
+      dwFlag |= ANNOTFLAG_NOVIEW;
+      dwFlag |= ANNOTFLAG_PRINT;
+      dwFlag &= (~ANNOTFLAG_HIDDEN);
+      break;
+  }
+
+  if (dwFlag != pWidget->GetFlags()) {
+    pWidget->SetFlags(dwFlag);
+    return true;
+  }
+
+  return false;
+}
+
 BEGIN_JS_STATIC_CONST(CJS_Field)
 END_JS_STATIC_CONST()
 
@@ -1244,39 +1282,8 @@ void Field::SetDisplay(CPDFSDK_Document* pDocument,
         CPDF_FormControl* pFormControl = pFormField->GetControl(i);
         ASSERT(pFormControl);
 
-        if (CPDFSDK_Widget* pWidget =
-                pInterForm->GetWidget(pFormControl, true)) {
-          uint32_t dwFlag = pWidget->GetFlags();
-          switch (number) {
-            case 0:
-              dwFlag &= (~ANNOTFLAG_INVISIBLE);
-              dwFlag &= (~ANNOTFLAG_HIDDEN);
-              dwFlag &= (~ANNOTFLAG_NOVIEW);
-              dwFlag |= ANNOTFLAG_PRINT;
-              break;
-            case 1:
-              dwFlag &= (~ANNOTFLAG_INVISIBLE);
-              dwFlag &= (~ANNOTFLAG_NOVIEW);
-              dwFlag |= (ANNOTFLAG_HIDDEN | ANNOTFLAG_PRINT);
-              break;
-            case 2:
-              dwFlag &= (~ANNOTFLAG_INVISIBLE);
-              dwFlag &= (~ANNOTFLAG_PRINT);
-              dwFlag &= (~ANNOTFLAG_HIDDEN);
-              dwFlag &= (~ANNOTFLAG_NOVIEW);
-              break;
-            case 3:
-              dwFlag |= ANNOTFLAG_NOVIEW;
-              dwFlag |= ANNOTFLAG_PRINT;
-              dwFlag &= (~ANNOTFLAG_HIDDEN);
-              break;
-          }
-
-          if (dwFlag != pWidget->GetFlags()) {
-            pWidget->SetFlags(dwFlag);
-            bSet = TRUE;
-          }
-        }
+        CPDFSDK_Widget* pWidget = pInterForm->GetWidget(pFormControl, true);
+        bSet = SetWidgetDisplayStatus(pWidget, number);

[Tom Sepez, 2016/08/17 17:38:40]

actually, we don't want to loose  previous TRUE value, so we have to do
if (...)
  bSet = TRUE;

[tonikitoo, 2016/08/17 17:51:53]

Done.

       }
 
       if (bSet)
@@ -1284,41 +1291,14 @@ void Field::SetDisplay(CPDFSDK_Document* pDocument,
     } else {
       if (nControlIndex >= pFormField->CountControls())
         return;
-      if (CPDF_FormControl* pFormControl =
-              pFormField->GetControl(nControlIndex)) {
-        if (CPDFSDK_Widget* pWidget =
-                pInterForm->GetWidget(pFormControl, true)) {
-          uint32_t dwFlag = pWidget->GetFlags();
-          switch (number) {
-            case 0:
-              dwFlag &= (~ANNOTFLAG_INVISIBLE);
-              dwFlag &= (~ANNOTFLAG_HIDDEN);
-              dwFlag &= (~ANNOTFLAG_NOVIEW);
-              dwFlag |= ANNOTFLAG_PRINT;
-              break;
-            case 1:
-              dwFlag &= (~ANNOTFLAG_INVISIBLE);
-              dwFlag &= (~ANNOTFLAG_NOVIEW);
-              dwFlag |= (ANNOTFLAG_HIDDEN | ANNOTFLAG_PRINT);
-              break;
-            case 2:
-              dwFlag &= (~ANNOTFLAG_INVISIBLE);
-              dwFlag &= (~ANNOTFLAG_PRINT);
-              dwFlag &= (~ANNOTFLAG_HIDDEN);
-              dwFlag &= (~ANNOTFLAG_NOVIEW);
-              break;
-            case 3:
-              dwFlag |= ANNOTFLAG_NOVIEW;
-              dwFlag |= ANNOTFLAG_PRINT;
-              dwFlag &= (~ANNOTFLAG_HIDDEN);
-              break;
-          }
-          if (dwFlag != pWidget->GetFlags()) {
-            pWidget->SetFlags(dwFlag);
-            UpdateFormControl(pDocument, pFormControl, TRUE, FALSE, TRUE);
-          }
-        }
-      }
+
+      CPDF_FormControl* pFormControl = pFormField->GetControl(nControlIndex);
+      if (!pFormControl)
+        return;
+
+      CPDFSDK_Widget* pWidget = pInterForm->GetWidget(pFormControl, true);
+      if (SetWidgetDisplayStatus(pWidget, number))
+        UpdateFormControl(pDocument, pFormControl, TRUE, FALSE, TRUE);
     }
   }
 }