This does not occur when <iframe> is in-process. For the case of nested <iframe>'s this happens only if at least one <iframe> is OOPIF.

It is noticeable that the displacement is exactly equal to the displacement of the OOPIF on page.

Thanks for filing!  This affects M56 as well, right?  Just noting it in case we want to merge the fix.

ekaramad@: Do you know what it would take to fix this, or should we look for another owner?

It is in M56. I am not quite sure about the fix yet. I can take a look if no one else is interested.

I would guess its something around here:

https://cs.chromium.org/chromium/src/chrome/browser/renderer_context_menu/render_view_context_menu.cc?l=518

Maybe the bounds of the WebContents isn't the correct place to calculate the offset? Perhaps you need the bounds of the embedder frame?

I had poked at it back then and tried to include the bounds from embedder frame but then scrolling would change the position if I remember correctly.

I found a fix for it but it needs some polishing so for now I am assigning it to myself.

I have a WIP CL which fixes this. But I looks like some polishing is needed.

1- We need |guest_view_rect_| to consider the displacement due to OOPIFs view. Since it is updated in BrowserPluginGuest::OnUpdateGeomteory(), we will have to transform the reported rect's origin coordinates to the owner window. This is the actual fix.

2- I noticed in BrowserPlugin::updateGeometry() we convert the coordinates to viewport using the RenderViewImpl which seems wrong to me; at least when PDF is inside OOPIF. While it might not affect the single PDF case, I suspect when we have two PDFs embedded it will be wrong. But I am not sure yet. Nonetheless, making the conversion through RenderWidget for local root seems like the better solution (it will be compatible with other guests as well as PDF in OOPIF).

3- This is the puzzling part for me: I noticed we do not send the updated geometry when the guest is attached. There is one call to BrowserPlugin::updateGeometry (which will send the IPC) and that happens before |attached_| = true. But even after that, when I call container()->reportGeometry() (which will make another call to updateGeometry()) we still do not update. So in the WIP CL I have added a boolean to make sure we do send updates at least once.

cc-ing Lucas and James for potential feedback on (3).

This is the CL to the fix:
https://codereview.chromium.org/2544223002

Regarding (3) in comment 7: I realized that we make a first call to updateGeometry() but at that time, guest is not attached. However, the window
rects are sent to browser during attaching. That fixes my issue with forming updates.

ekaramad@: Can we revisit this to try to get the fix in?  Thanks!

I will take another look today. The issue right now is the strange purple/red Windows 7 bot for the test.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aeddbee1c18102e37b5dc083c99a337e12afa759

commit aeddbee1c18102e37b5dc083c99a337e12afa759
Author: ekaramad <ekaramad@chromium.org>
Date: Thu Jan 26 21:17:24 2017

Fix the position of context menu for BrowserPlugins inside OOPIF

Currently, we do not consider the displacement of OOPIF view on page when
update the geomtery of BrowserPluginGuest. This causes the context menu
offset (the displacement between WebContents's) to be wrong.

This CL will make the following changes to fix this issue:
1- The reported rect in OnUpdateGeometry() is transformed to owner view's
coordinate space.

2- We make sure BrowserPlugin does its own conversions using RenderWidget
of the local root containing the BrowserPlugin (as opposed to the RenderView
which seems wrong).

3- We make sure BrowserPlugin updates geometry when it is attached.

BUG= 670433 

Review-Url: https://codereview.chromium.org/2544223002
Cr-Commit-Position: refs/heads/master@{#446448}

[modify] https://crrev.com/aeddbee1c18102e37b5dc083c99a337e12afa759/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/aeddbee1c18102e37b5dc083c99a337e12afa759/chrome/test/data/page_with_embedded_pdf.html
[modify] https://crrev.com/aeddbee1c18102e37b5dc083c99a337e12afa759/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/aeddbee1c18102e37b5dc083c99a337e12afa759/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/aeddbee1c18102e37b5dc083c99a337e12afa759/content/browser/web_contents/web_contents_view_guest.cc
[modify] https://crrev.com/aeddbee1c18102e37b5dc083c99a337e12afa759/content/renderer/browser_plugin/browser_plugin.cc

For posterity, here's a page to repro the bug:
http://csreis.github.io/tests/cross-site-iframe-pdf.html

We should be able to verify that it's fixed in tomorrow's Canary.

Marking as fixed (verified on Canary 58.0.3007.0).

ekaramad@: Thanks for the fix!  Do you think this is safe to merge to M57?

I believe the fix is quite safe on the browser side, as in:
 1- It only affects PDFs inside OOPIF (which fixes the context menu issue ~= does not make it worse than what is already is in M-57).
 2- Would not cause any new crashes (new code is checked agains nullptr).

The renderer side seems fine. I wish we were checking agains nullptr for container() or frame() or RenderFrameImpl here:
https://cs.chromium.org/chromium/src/content/renderer/browser_plugin/browser_plugin.cc?rcl=c8e27a009a07233002d0397cf8c0134e09b572f5&l=395
but this is not worse than what it was before:
RenderViewImpl::FromWebView(container()->frame()->view()).

So all that being discussed, I guess the fix is quite safe to land. I will go over the crash reports just in case and confirm here.

Great!  Please add the merge request label if you don't see crashes there, since I think this will be a worthwhile fix to include in M57.

I could not find any crashes. Adding the label.

Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

If possible, could you please merge your CL into M57 branch 2987 before 4 PM PT tomorrow, Wednesday (02/15/17). Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4d8f2c9b39a7fc569beede339aff9f2520604cf

commit b4d8f2c9b39a7fc569beede339aff9f2520604cf
Author: ekaramad <ekaramad@chromium.org>
Date: Tue Feb 14 23:24:52 2017

Fix the position of context menu for BrowserPlugins inside OOPIF (Merge to M-57)

Currently, we do not consider the displacement of OOPIF view on page when
update the geomtery of BrowserPluginGuest. This causes the context menu
offset (the displacement between WebContents's) to be wrong.

This CL will make the following changes to fix this issue:
1- The reported rect in OnUpdateGeometry() is transformed to owner view's
coordinate space.

2- We make sure BrowserPlugin does its own conversions using RenderWidget
of the local root containing the BrowserPlugin (as opposed to the RenderView
which seems wrong).

3- We make sure BrowserPlugin updates geometry when it is attached.

BUG= 670433 
NOTRY=TRUE
NOPRESUBMIT=TRUE

Review-Url: https://codereview.chromium.org/2544223002
Cr-Commit-Position: refs/heads/master@{#446448}
(cherry picked from commit aeddbee1c18102e37b5dc083c99a337e12afa759)

Review-Url: https://codereview.chromium.org/2695103003
Cr-Commit-Position: refs/branch-heads/2987@{#512}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/b4d8f2c9b39a7fc569beede339aff9f2520604cf/chrome/browser/site_per_process_interactive_browsertest.cc
[modify] https://crrev.com/b4d8f2c9b39a7fc569beede339aff9f2520604cf/chrome/test/data/page_with_embedded_pdf.html
[modify] https://crrev.com/b4d8f2c9b39a7fc569beede339aff9f2520604cf/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/b4d8f2c9b39a7fc569beede339aff9f2520604cf/content/browser/browser_plugin/browser_plugin_guest.h
[modify] https://crrev.com/b4d8f2c9b39a7fc569beede339aff9f2520604cf/content/browser/web_contents/web_contents_view_guest.cc
[modify] https://crrev.com/b4d8f2c9b39a7fc569beede339aff9f2520604cf/content/renderer/browser_plugin/browser_plugin.cc

Rechecked the issue on chrome version 57.0.2987.54 on Windows 10, MAC 10.12.2, Ubuntu 14.04 using the test example URL: http://csreis.github.io/tests/cross-site-iframe-pdf.html

Right Clicked on the pdf page and the context menu is displayed at the point right click occurred. Adding TE-Verified labels.

Verified that fix is working fine on Chrome OS 9202.28.0/57.0.2987.54 - Candy Device as well.