Revert "Fixes Select elements drop-down doesn't follow update of the element position/size"

This reverts commit 95a35640.

Reason for revert: crbug.com/1057025

After revert, I'm planning to reland this patch after M82 branch and
work on fix to crbug.com/1057025

Original change's description:
> Fixes Select elements drop-down doesn't follow update of the element position/size
>
> Prior to this fix, when a select element's popup is open, a script
> in the background can modify the page's DOM that changes the position
> of select element on the screen. However, select popup remains
> stationary thus appearing disconnect from the select element. The select
> popup also doesn't update to the correct size if the script adds
> an new option with a long text.
>
> The reason is that select element doesn't check to see if the position
> or size of the select element popup relative to the document changed.
> For example, after the popup is displayed, a script could modify the
> document that changes the position of the select element. It doesn't
> notify its popup, thus makes them appear as disconnected.
>
> The fix is to update the select popup, its position and size, at the
> end of frame's life-cycle phase. This will guarantee that all elements
> in the frame have been styled and layout correctly. At this point,
> we can update the popup if its owner's position and size has been
> changed. The fix also removes the PostTask as a performance and
> simplicity optimization.
>
> An alternative proposal, use intersection observer on the owner element,
> was discussed. However, it still has the draw backs of post task. Plus,
> it may not capture all cases.
>
> In addition, I also considered and tried using PostTask within the
> UpdateLifecycle method. However, that's not optimal since within the
> popup's update method, it requires another call to UpdateStyleAndLayout
> because the layout could have been dirtied since the time PostTask was
> added into the queue. InternalPopupMenu::Update() also updates the
> select element's internal state, options. I saw some intermittent
> failures in couple of tests that validate options index. These tests
> use SetTimeout(), which may partly be aggravate by PostTask. I didn't
> investigate further as this is not the approach to take.
>
> This fix works for both Linux and Windows. I updated test
> popup-menu-resize-after-open to reflect that. In addition, I removed
> the (TODO) in listPicker.js. This fixes select popup when placed inside
> an iframe.
> For platforms that use external_popup_menu, mac and android, will be
> migrated to use internal_popup_menu when forms-refresh is rolled out to
> those platforms in M83.
>
> One of the biggest problem I encountered when doing this fix is that popup
> content doesn't updated when the popup is moved only. This is the same
> issue as crbug.com/633140. This review,
> https://codereview.chromium.org/2228093003, goes into more details on
> popup bounds synchronization between browser and renderer. Regardless,
> I had to make a similar fix in  web_page_popup_impl.cc,
>  calling SetWindowRect() when force_update is true.
>
>
> Bug: 137495
> Change-Id: I7ef4c09a6d3d7bee3ef8988c815e07efa72d4a79
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2013470
> Commit-Queue: Yu Han <yuzhehan@chromium.org>
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#740964}

TBR=chrishtr@chromium.org,yuzhehan@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 137495
Change-Id: I403683f2b9bd05784cd6a5384a92c14ac8869ccb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095839Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Yu Han <yuzhehan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748519}

third_party/blink/renderer/core/exported/web_page_popup_impl.cc

@@ -52,7 +52,6 @@
 #include "third_party/blink/renderer/core/frame/visual_viewport.h"
 #include "third_party/blink/renderer/core/frame/web_frame_widget_base.h"
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
-#include "third_party/blink/renderer/core/geometry/dom_rect.h"
 #include "third_party/blink/renderer/core/input/event_handler.h"
 #include "third_party/blink/renderer/core/layout/layout_view.h"
 #include "third_party/blink/renderer/core/loader/empty_clients.h"
@@ -324,8 +323,6 @@ void WebPagePopupImpl::Initialize(WebViewImpl* web_view,
   frame->SetPageZoomFactor(popup_client_->ZoomFactor());
   frame->ForceSynchronousDocumentInstall("text/html", std::move(data));
 
-  popup_owner_client_rect_ =
-      popup_client_->OwnerElement().getBoundingClientRect();
   WidgetClient()->Show(WebNavigationPolicy());
   SetFocus(true);
 }
@@ -344,20 +341,6 @@ void WebPagePopupImpl::PostMessageToPopup(const String& message) {
   MainFrame().DomWindow()->DispatchEvent(*MessageEvent::Create(message));
 }
 
-void WebPagePopupImpl::Update() {
-  if (!page_ && !popup_client_)
-    return;
-
-  DOMRect* dom_rect = popup_client_->OwnerElement().getBoundingClientRect();
-  bool forced_update = (*dom_rect != *popup_owner_client_rect_);
-  if (forced_update)
-    popup_owner_client_rect_ = dom_rect;
-
-  popup_client_->Update(forced_update);
-  if (forced_update)
-    SetWindowRect(WindowRectInScreen());
-}
-
 void WebPagePopupImpl::DestroyPage() {
   page_->WillCloseAnimationHost(nullptr);
   page_->WillBeDestroyed();

third_party/blink/renderer/core/exported/web_page_popup_impl.h

@@ -52,7 +52,6 @@ class PagePopupChromeClient;
 class PagePopupClient;
 class WebViewImpl;
 class LocalDOMWindow;
-class DOMRect;
 
 class CORE_EXPORT WebPagePopupImpl final : public WebPagePopup,
                                            public PageWidgetEventHandler,
@@ -105,7 +104,6 @@ class CORE_EXPORT WebPagePopupImpl final : public WebPagePopup,
 
   // PagePopup implementation.
   void PostMessageToPopup(const String& message) override;
-  void Update() override;
 
   // PageWidgetEventHandler implementation.
   WebInputEventResult HandleKeyEvent(const WebKeyboardEvent&) override;
@@ -169,7 +167,6 @@ class CORE_EXPORT WebPagePopupImpl final : public WebPagePopup,
   base::TimeTicks raf_aligned_input_start_time_;
 
   bool suppress_next_keypress_event_ = false;
-  Persistent<DOMRect> popup_owner_client_rect_;
 
   // Base functionality all widgets have. This is a member as to avoid
   // complicated inheritance structures.

third_party/blink/renderer/core/exported/web_view_impl.cc

@@ -1184,11 +1184,6 @@ void WebViewImpl::CleanupPagePopup() {
   DisablePopupMouseWheelEventListener();
 }
 
-void WebViewImpl::UpdatePagePopup() {
-  if (page_popup_)
-    page_popup_->Update();
-}
-
 void WebViewImpl::EnablePopupMouseWheelEventListener(
     WebLocalFrameImpl* local_root) {
   DCHECK(!popup_mouse_wheel_event_listener_);
@@ -1613,8 +1608,6 @@ void WebViewImpl::UpdateLifecycle(WebWidget::LifecycleUpdate requested_update,
   if (requested_update != WebWidget::LifecycleUpdate::kAll)
     return;
 
-  UpdatePagePopup();
-
   // There is no background color for non-composited WebViews (eg printing).
   if (does_composite_) {
     MainFrameImpl()->FrameWidgetImpl()->SetBackgroundColor(BackgroundColor());

third_party/blink/renderer/core/exported/web_view_impl.h

@@ -313,9 +313,6 @@ class CORE_EXPORT WebViewImpl final : public WebView,
   // Callback from PagePopup when it is closed, which it can be done directly
   // without coming through WebViewImpl.
   void CleanupPagePopup();
-  // Ensure popup's size and position is correct based on its owner element's
-  // dimensions.
-  void UpdatePagePopup();
   LocalDOMWindow* PagePopupWindow() const;
 
   PageScheduler* Scheduler() const override;

third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

@@ -373,7 +373,6 @@ void WebFrameWidgetImpl::UpdateLifecycle(LifecycleUpdate requested_update,
       LocalRootImpl()->GetFrame()->GetDocument()->Lifecycle());
   PageWidgetDelegate::UpdateLifecycle(*GetPage(), *LocalRootImpl()->GetFrame(),
                                       requested_update, reason);
-  View()->UpdatePagePopup();
 }
 
 void WebFrameWidgetImpl::ThemeChanged() {

third_party/blink/renderer/core/geometry/dom_rect.h

@@ -34,13 +34,6 @@ class CORE_EXPORT DOMRect final : public DOMRectReadOnly {
   void setHeight(double height) { height_ = height; }
 };
 
-constexpr bool operator==(const DOMRect& lhs, const DOMRect& rhs) {
-  return lhs.x() == rhs.x() && lhs.y() == rhs.y() &&
-         lhs.width() == rhs.width() && lhs.height() == rhs.height();
-}
-constexpr bool operator!=(const DOMRect& lhs, const DOMRect& rhs) {
-  return !(lhs == rhs);
-}
 }  // namespace blink
 
-#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_GEOMETRY_DOM_RECT_H_
+#endif

third_party/blink/renderer/core/html/forms/internal_popup_menu.cc

@@ -508,12 +508,20 @@ void InternalPopupMenu::Hide() {
 }
 
 void InternalPopupMenu::UpdateFromElement(UpdateReason) {
+  if (needs_update_)
+    return;
   needs_update_ = true;
+  OwnerElement()
+      .GetDocument()
+      .GetTaskRunner(TaskType::kUserInteraction)
+      ->PostTask(FROM_HERE,
+                 WTF::Bind(&InternalPopupMenu::Update, WrapPersistent(this)));
 }
 
-void InternalPopupMenu::Update(bool force_update) {
-  if (!popup_ || !owner_element_ || (!needs_update_ && !force_update))
+void InternalPopupMenu::Update() {
+  if (!popup_ || !owner_element_)
     return;
+  OwnerElement().GetDocument().UpdateStyleAndLayoutTree();
   // disconnectClient() might have been called.
   if (!owner_element_)
     return;

third_party/blink/renderer/core/html/forms/internal_popup_menu.h

@@ -28,7 +28,7 @@ class CORE_EXPORT InternalPopupMenu final : public PopupMenu,
   ~InternalPopupMenu() override;
   void Trace(Visitor*) override;
 
-  void Update(bool force_update) override;
+  void Update();
 
   void Dispose();
 

third_party/blink/renderer/core/html/forms/resources/listPicker.js

@@ -100,8 +100,12 @@ ListPicker.prototype._handleWindowMessage = function(event) {
             window.updateData.anchorRectInScreen.width ||
         this._config.anchorRectInScreen.height !==
             window.updateData.anchorRectInScreen.height) {
+      // TODO(tkent): Don't fix window size here due to a bug of Aura or
+      // compositor. crbug.com/863770
+      if (!navigator.platform.startsWith('Win')) {
         this._config.anchorRectInScreen = window.updateData.anchorRectInScreen;
         this._fixWindowSize();
+      }
     }
   }
   delete window.updateData;

third_party/blink/renderer/core/page/page_popup.h

@@ -45,7 +45,6 @@ class PagePopup {
   virtual AXObject* RootAXObject() = 0;
   virtual void SetWindowRect(const IntRect&) = 0;
   virtual void PostMessageToPopup(const String& message) = 0;
-  virtual void Update() = 0;
 
  protected:
   virtual ~PagePopup() = default;

third_party/blink/renderer/core/page/page_popup_client.h

@@ -85,9 +85,6 @@ class CORE_EXPORT PagePopupClient {
   // This is called whenever a PagePopup was closed.
   virtual void DidClosePopup() = 0;
 
-  // This is called when popup content or its owner's position changed.
-  virtual void Update(bool force_update) {}
-
   virtual ~PagePopupClient() = default;
 
   // Helper functions to be used in PagePopupClient::writeDocument().

third_party/blink/web_tests/fast/forms/select-popup/popup-menu-move-after-open.html

-<!DOCTYPE html>
-<title>Test popup menu positioned correctly after layout change</title>
-<div id="err" style="height:50px; display:block">
-  Test select popup positions correctly after an event causes update to select element positioning.
-</div>
-<select id="menu">
-    <option value="one">One</option>
-    <option value="two">Two</option>
-</select>
-<script>
-  menu.focus();
-  menu.onkeydown = removeError;
-
-  function removeError() {
-    err.style.display = 'none';
-    if (window.testRunner){
-      testRunner.notifyDone();
-    }
-  }
-
-  if (window.testRunner) {
-    testRunner.waitUntilDone();
-  }
-  eventSender.keyDown(" ", null);
-</script>
\ No newline at end of file

third_party/blink/web_tests/platform/linux/fast/forms/select-popup/popup-menu-move-after-open-expected.html

-<!DOCTYPE html>
-<select id="menu">
-    <option value="one">One</option>
-    <option value="two">Two</option>
-</select>
-<script>
-   menu.focus();
-   eventSender.keyDown(" ", null);
-</script>

third_party/blink/web_tests/platform/win/fast/forms/select-popup/popup-menu-move-after-open-expected.html

-<!DOCTYPE html>
-<select id="menu">
-    <option value="one">One</option>
-    <option value="two">Two</option>
-</select>
-<script>
-    menu.focus();
-    eventSender.keyDown(" ", null);
-</script>

third_party/blink/web_tests/platform/win/fast/forms/select-popup/popup-menu-resize-after-open-expected.txt

-PASS popupWindow.innerWidth is 100
-PASS popupWindow.innerWidth is 150
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
-o1
-o2