Make Paste Popup use selection rect for positioning

third_party/WebKit/Source/web/ContextMenuClientImpl.cpp

Index: third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
diff --git a/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
index 63214655706a8c3468d99ef8c4ad55f781be68cd..290f9b08eda567b8246ee8401201cf8cb9fe568e 100644
--- a/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
+++ b/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp
@@ -402,6 +402,19 @@ bool ContextMenuClientImpl::ShowContextMenu(const ContextMenu* default_menu,
     data.input_field_type = WebContextMenuData::kInputFieldTypeNone;
   }
 
+  WebRect focus_webrect;
+  WebRect anchor_webrect;
+  web_view_->SelectionBounds(focus_webrect, anchor_webrect);

[aelias_OOO_until_Jul13, 2017/05/06 01:18:39]

The rect you're computing here would be useful to position the selection
handles, but I don't think it's the right rect for avoiding occluding selected
text.  Imagine a multi-line selection with the handles at the same X coordinate,
then you end up with a very narrow rect, and the floating action popup could
position itself to the side, covering selected text.  I think we should somehow
be getting a rect that encloses the entire selection.

Also, please make sure the algorithm works well when the selection is very large
and one handle is offscreen.  That case might currently be broken judging from
the screenshot on http://b/37850392

[amaralp, 2017/05/11 18:46:55]

This is already a problem since in CompositorFrameMetadata only sends the handle
positions. I've changed this to give the correct rect, but there is still the
issue when the selection is repositioned (like from a scroll). A possible
solution is to add the rect to CompositorFrameMetadata.
This is fixed for the initial positioning, but as I said above it would still be
broken when a scroll happens.

[aelias_OOO_until_Jul13, 2017/05/11 21:16:17]

It looks like RenderWidget::UpdateSelectionBounds is for similar purposes,
although it's currently disabled on Android by
WebRuntimeFeatures::IsCompositedSelectionUpdateEnabled().  I guess now that the
long-term plan is not to send CompositorFrameMetadata to the browser, we can
start to use this again.  But computing selection bounds is too costly to send
every WillBeginCompositorFrame() (has been observed to be expensive with large
selections or complex non-English text).  I'd suggest adding the rect to that
data structure, and add a new message FrameMsg_RequestSelectionBounds that would
be sent to the frame that has an active selection when the popup needs to
unhide, and wait to unhide until it's received.

+
+  int left = std::min(focus_webrect.x, anchor_webrect.x);
+  int top = std::min(focus_webrect.y, anchor_webrect.y);
+  int right = std::max(focus_webrect.x + focus_webrect.width,
+                       anchor_webrect.x + anchor_webrect.width);
+  int bottom = std::max(focus_webrect.y + focus_webrect.height,
+                        anchor_webrect.y + anchor_webrect.height);
+
+  data.selection_rect = WebRect(left, top, right - left, bottom - top);
+
   if (from_touch && !ShouldShowContextMenuFromTouch(data))
     return false;