Clarify IPWL_Filler_Notify::QueryWherePopup() parameters.

Update implementation and caller, and clean up surrounding code.

Change-Id: I40975f79a5d4f560d6a7ee0d327b2e97241d29bd
Reviewed-on: https://pdfium-review.googlesource.com/6172
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

5 files changed, 101 insertions, 153 deletions

diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
index 5a35852005..491186e39c 100644
--- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
+++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
@@ -23,6 +23,7 @@
 #include "fpdfsdk/formfiller/cffl_radiobutton.h"
 #include "fpdfsdk/formfiller/cffl_textfield.h"
 #include "fpdfsdk/pdfwindow/PWL_Utils.h"
+#include "third_party/base/stl_util.h"
 
 #define FFL_MAXLISTBOXHEIGHT 140.0f
 
@@ -528,8 +529,8 @@ void CFFL_InteractiveFormFiller::UnRegisterFormFiller(CPDFSDK_Annot* pAnnot) {
 void CFFL_InteractiveFormFiller::QueryWherePopup(void* pPrivateData,
                                                  float fPopupMin,
                                                  float fPopupMax,
-                                                 int32_t& nRet,
-                                                 float& fPopupRet) {
+                                                 bool* bBottom,
+                                                 float* fPopupRet) {
   CFFL_PrivateData* pData = (CFFL_PrivateData*)pPrivateData;
 
   CFX_FloatRect rcPageView(0, 0, 0, 0);
@@ -563,39 +564,28 @@ void CFFL_InteractiveFormFiller::QueryWherePopup(void* pPrivateData,
       break;
   }
 
-  float fFactHeight = 0;
-  bool bBottom = true;
-  float fMaxListBoxHeight = 0;
-  if (fPopupMax > FFL_MAXLISTBOXHEIGHT) {
-    if (fPopupMin > FFL_MAXLISTBOXHEIGHT) {
-      fMaxListBoxHeight = fPopupMin;
-    } else {
-      fMaxListBoxHeight = FFL_MAXLISTBOXHEIGHT;
-    }
-  } else {
-    fMaxListBoxHeight = fPopupMax;
-  }
+  const float fMaxListBoxHeight =
+      pdfium::clamp(FFL_MAXLISTBOXHEIGHT, fPopupMin, fPopupMax);
 
   if (fBottom > fMaxListBoxHeight) {
-    fFactHeight = fMaxListBoxHeight;
-    bBottom = true;
-  } else {
-    if (fTop > fMaxListBoxHeight) {
-      fFactHeight = fMaxListBoxHeight;
-      bBottom = false;
-    } else {
-      if (fTop > fBottom) {
-        fFactHeight = fTop;
-        bBottom = false;
-      } else {
-        fFactHeight = fBottom;
-        bBottom = true;
-      }
-    }
+    *fPopupRet = fMaxListBoxHeight;
+    *bBottom = true;
+    return;
+  }
+
+  if (fTop > fMaxListBoxHeight) {
+    *fPopupRet = fMaxListBoxHeight;
+    *bBottom = false;
+    return;
   }
 
-  nRet = bBottom ? 0 : 1;
-  fPopupRet = fFactHeight;
+  if (fTop > fBottom) {
+    *fPopupRet = fTop;
+    *bBottom = false;
+  } else {
+    *fPopupRet = fBottom;
+    *bBottom = true;
+  }
 }
 
 void CFFL_InteractiveFormFiller::OnKeyStrokeCommit(
diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.h b/fpdfsdk/formfiller/cffl_interactiveformfiller.h
index 751abf6e79..d2f59bfe38 100644
--- a/fpdfsdk/formfiller/cffl_interactiveformfiller.h
+++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.h
@@ -142,8 +142,8 @@ class CFFL_InteractiveFormFiller : public IPWL_Filler_Notify {
   void QueryWherePopup(void* pPrivateData,
                        float fPopupMin,
                        float fPopupMax,
-                       int32_t& nRet,
-                       float& fPopupRet) override;
+                       bool* bBottom,
+                       float* fPopupRet) override;
   void OnBeforeKeyStroke(void* pPrivateData,
                          CFX_WideString& strChange,
                          const CFX_WideString& strChangeEx,
diff --git a/fpdfsdk/pdfwindow/PWL_ComboBox.cpp b/fpdfsdk/pdfwindow/PWL_ComboBox.cpp
index b6c8d2f921..a4dbeabacf 100644
--- a/fpdfsdk/pdfwindow/PWL_ComboBox.cpp
+++ b/fpdfsdk/pdfwindow/PWL_ComboBox.cpp
@@ -6,6 +6,8 @@
 
 #include "fpdfsdk/pdfwindow/PWL_ComboBox.h"
 
+#include <algorithm>
+
 #include "core/fxge/cfx_pathdata.h"
 #include "core/fxge/cfx_renderdevice.h"
 #include "fpdfsdk/fxedit/fxet_list.h"
@@ -185,7 +187,7 @@ CPWL_ComboBox::CPWL_ComboBox()
       m_pButton(nullptr),
       m_pList(nullptr),
       m_bPopup(false),
-      m_nPopupWhere(0),
+      m_bBottom(true),
       m_nSelectItem(-1),
       m_pFillerNotify(nullptr) {}
 
@@ -335,60 +337,25 @@ void CPWL_ComboBox::CreateListBox(const PWL_CREATEPARAM& cp) {
 }
 
 void CPWL_ComboBox::RePosChildWnd() {
-  CFX_FloatRect rcClient = GetClientRect();
-
+  const CFX_FloatRect rcClient = GetClientRect();
   if (m_bPopup) {
-    CFX_FloatRect rclient = GetClientRect();
-    CFX_FloatRect rcButton = rclient;
-    CFX_FloatRect rcEdit = rcClient;
-    CFX_FloatRect rcList = CPWL_Wnd::GetWindowRect();
-
-    float fOldWindowHeight = m_rcOldWindow.Height();
-    float fOldClientHeight = fOldWindowHeight - GetBorderWidth() * 2;
-
-    switch (m_nPopupWhere) {
-      case 0:
-        rcButton.left = rcButton.right - PWL_COMBOBOX_BUTTON_WIDTH;
-
-        if (rcButton.left < rclient.left)
-          rcButton.left = rclient.left;
-
-        rcButton.bottom = rcButton.top - fOldClientHeight;
-
-        rcEdit.right = rcButton.left - 1.0f;
-
-        if (rcEdit.left < rclient.left)
-          rcEdit.left = rclient.left;
-
-        if (rcEdit.right < rcEdit.left)
-          rcEdit.right = rcEdit.left;
-
-        rcEdit.bottom = rcEdit.top - fOldClientHeight;
-
-        rcList.top -= fOldWindowHeight;
-
-        break;
-      case 1:
-        rcButton.left = rcButton.right - PWL_COMBOBOX_BUTTON_WIDTH;
-
-        if (rcButton.left < rclient.left)
-          rcButton.left = rclient.left;
-
-        rcButton.top = rcButton.bottom + fOldClientHeight;
+    const float fOldWindowHeight = m_rcOldWindow.Height();
+    const float fOldClientHeight = fOldWindowHeight - GetBorderWidth() * 2;
 
-        rcEdit.right = rcButton.left - 1.0f;
-
-        if (rcEdit.left < rclient.left)
-          rcEdit.left = rclient.left;
-
-        if (rcEdit.right < rcEdit.left)
-          rcEdit.right = rcEdit.left;
-
-        rcEdit.top = rcEdit.bottom + fOldClientHeight;
-
-        rcList.bottom += fOldWindowHeight;
-
-        break;
+    CFX_FloatRect rcList = CPWL_Wnd::GetWindowRect();
+    CFX_FloatRect rcButton = rcClient;
+    rcButton.left =
+        std::max(rcButton.right - PWL_COMBOBOX_BUTTON_WIDTH, rcClient.left);
+    CFX_FloatRect rcEdit = rcClient;
+    rcEdit.right = std::max(rcButton.left - 1.0f, rcEdit.left);
+    if (m_bBottom) {
+      rcButton.bottom = rcButton.top - fOldClientHeight;
+      rcEdit.bottom = rcEdit.top - fOldClientHeight;
+      rcList.top -= fOldWindowHeight;
+    } else {
+      rcButton.top = rcButton.bottom + fOldClientHeight;
+      rcEdit.top = rcEdit.bottom + fOldClientHeight;
+      rcList.bottom += fOldWindowHeight;
     }
 
     if (m_pButton)
@@ -402,32 +369,24 @@ void CPWL_ComboBox::RePosChildWnd() {
       m_pList->Move(rcList, true, false);
       m_pList->ScrollToListItem(m_nSelectItem);
     }
-  } else {
-    CFX_FloatRect rcButton = rcClient;
-
-    rcButton.left = rcButton.right - PWL_COMBOBOX_BUTTON_WIDTH;
-
-    if (rcButton.left < rcClient.left)
-      rcButton.left = rcClient.left;
-
-    if (m_pButton)
-      m_pButton->Move(rcButton, true, false);
+    return;
+  }
 
-    CFX_FloatRect rcEdit = rcClient;
-    rcEdit.right = rcButton.left - 1.0f;
+  CFX_FloatRect rcButton = rcClient;
+  rcButton.left =
+      std::max(rcButton.right - PWL_COMBOBOX_BUTTON_WIDTH, rcClient.left);
 
-    if (rcEdit.left < rcClient.left)
-      rcEdit.left = rcClient.left;
+  if (m_pButton)
+    m_pButton->Move(rcButton, true, false);
 
-    if (rcEdit.right < rcEdit.left)
-      rcEdit.right = rcEdit.left;
+  CFX_FloatRect rcEdit = rcClient;
+  rcEdit.right = std::max(rcButton.left - 1.0f, rcEdit.left);
 
-    if (m_pEdit)
-      m_pEdit->Move(rcEdit, true, false);
+  if (m_pEdit)
+    m_pEdit->Move(rcEdit, true, false);
 
-    if (m_pList)
-      m_pList->SetVisible(false);
-  }
+  if (m_pList)
+    m_pList->SetVisible(false);
 }
 
 void CPWL_ComboBox::SelectAll() {
@@ -448,53 +407,50 @@ void CPWL_ComboBox::SetPopup(bool bPopup) {
   if (!IsFloatBigger(fListHeight, 0.0f))
     return;
 
-  if (bPopup) {
-    if (m_pFillerNotify) {
+  if (!bPopup) {
+    m_bPopup = bPopup;
+    Move(m_rcOldWindow, true, true);
+    return;
+  }
+
+  if (!m_pFillerNotify)
+    return;
+
 #ifdef PDF_ENABLE_XFA
-      bool bExit = false;
-      m_pFillerNotify->OnPopupPreOpen(GetAttachedData(), bExit, 0);
-      if (bExit)
-        return;
+  bool bExit = false;
+  m_pFillerNotify->OnPopupPreOpen(GetAttachedData(), bExit, 0);
+  if (bExit)
+    return;
 #endif  // PDF_ENABLE_XFA
-      int32_t nWhere = 0;
-      float fPopupRet = 0.0f;
-      float fPopupMin = 0.0f;
-      if (m_pList->GetCount() > 3)
-        fPopupMin =
-            m_pList->GetFirstHeight() * 3 + m_pList->GetBorderWidth() * 2;
-      float fPopupMax = fListHeight + m_pList->GetBorderWidth() * 2;
-      m_pFillerNotify->QueryWherePopup(GetAttachedData(), fPopupMin, fPopupMax,
-                                       nWhere, fPopupRet);
-
-      if (IsFloatBigger(fPopupRet, 0.0f)) {
-        m_bPopup = bPopup;
-
-        CFX_FloatRect rcWindow = CPWL_Wnd::GetWindowRect();
-        m_rcOldWindow = rcWindow;
-        switch (nWhere) {
-          default:
-          case 0:
-            rcWindow.bottom -= fPopupRet;
-            break;
-          case 1:
-            rcWindow.top += fPopupRet;
-            break;
-        }
 
-        m_nPopupWhere = nWhere;
-        Move(rcWindow, true, true);
+  float fBorderWidth = m_pList->GetBorderWidth() * 2;
+  float fPopupMin = 0.0f;
+  if (m_pList->GetCount() > 3)
+    fPopupMin = m_pList->GetFirstHeight() * 3 + fBorderWidth;
+  float fPopupMax = fListHeight + fBorderWidth;
+
+  bool bBottom;
+  float fPopupRet;
+  m_pFillerNotify->QueryWherePopup(GetAttachedData(), fPopupMin, fPopupMax,
+                                   &bBottom, &fPopupRet);
+  if (!IsFloatBigger(fPopupRet, 0.0f))
+    return;
+
+  m_rcOldWindow = CPWL_Wnd::GetWindowRect();
+  m_bPopup = bPopup;
+  m_bBottom = bBottom;
+
+  CFX_FloatRect rcWindow = m_rcOldWindow;
+  if (bBottom)
+    rcWindow.bottom -= fPopupRet;
+  else
+    rcWindow.top += fPopupRet;
+
+  Move(rcWindow, true, true);
 #ifdef PDF_ENABLE_XFA
-        bExit = false;
-        m_pFillerNotify->OnPopupPostOpen(GetAttachedData(), bExit, 0);
-        if (bExit)
-          return;
+  bExit = false;
+  m_pFillerNotify->OnPopupPostOpen(GetAttachedData(), bExit, 0);
 #endif  // PDF_ENABLE_XFA
-      }
-    }
-  } else {
-    m_bPopup = bPopup;
-    Move(m_rcOldWindow, true, true);
-  }
 }
 
 bool CPWL_ComboBox::OnKeyDown(uint16_t nChar, uint32_t nFlag) {
diff --git a/fpdfsdk/pdfwindow/PWL_ComboBox.h b/fpdfsdk/pdfwindow/PWL_ComboBox.h
index 27cdb61b1a..9211777656 100644
--- a/fpdfsdk/pdfwindow/PWL_ComboBox.h
+++ b/fpdfsdk/pdfwindow/PWL_ComboBox.h
@@ -96,9 +96,9 @@ class CPWL_ComboBox : public CPWL_Wnd {
   CFX_UnownedPtr<CPWL_CBEdit> m_pEdit;
   CFX_UnownedPtr<CPWL_CBButton> m_pButton;
   CFX_UnownedPtr<CPWL_CBListBox> m_pList;
-  bool m_bPopup;
   CFX_FloatRect m_rcOldWindow;
-  int32_t m_nPopupWhere;
+  bool m_bPopup;
+  bool m_bBottom;
   int32_t m_nSelectItem;
   CFX_UnownedPtr<IPWL_Filler_Notify> m_pFillerNotify;
   CFX_UnownedPtr<CFFL_FormFiller> m_pFormFiller;
diff --git a/fpdfsdk/pdfwindow/PWL_Edit.h b/fpdfsdk/pdfwindow/PWL_Edit.h
index 82c1f19629..6f8c37baa5 100644
--- a/fpdfsdk/pdfwindow/PWL_Edit.h
+++ b/fpdfsdk/pdfwindow/PWL_Edit.h
@@ -22,11 +22,13 @@ class IFX_Edit_UndoItem;
 class IPWL_Filler_Notify {
  public:
   virtual ~IPWL_Filler_Notify() {}
+
+  // Must write to |bBottom| and |fPopupRet|.
   virtual void QueryWherePopup(void* pPrivateData,
                                float fPopupMin,
                                float fPopupMax,
-                               int32_t& nRet,
-                               float& fPopupRet) = 0;  // nRet: (0:bottom 1:top)
+                               bool* bBottom,
+                               float* fPopupRet) = 0;
   virtual void OnBeforeKeyStroke(void* pPrivateData,
                                  CFX_WideString& strChange,
                                  const CFX_WideString& strChangeEx,