Make Paste Popup use selection rect for positioning

third_party/WebKit/Source/web/ContextMenuClientImpl.cpp

 /*
  * Copyright (C) 2009, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 384 matching lines @@
     if (element->type() == InputTypeNames::password)
       data.input_field_type = WebContextMenuData::kInputFieldTypePassword;
     else if (element->IsTextField())
       data.input_field_type = WebContextMenuData::kInputFieldTypePlainText;
     else
       data.input_field_type = WebContextMenuData::kInputFieldTypeOther;
   } else {
     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;
 
   selected_web_frame->SetContextMenuNode(r.InnerNodeOrImageMapImage());
   if (!selected_web_frame->Client())
     return false;
 
   selected_web_frame->Client()->ShowContextMenu(data);
   return true;
 }
@@ skipping 51 matching lines @@
   sub_menu_items.Swap(output_items);
 }
 
 void ContextMenuClientImpl::PopulateCustomMenuItems(
     const ContextMenu* default_menu,
     WebContextMenuData* data) {
   PopulateSubMenuItems(default_menu->Items(), data->custom_items);
 }
 
 }  // namespace blink