New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 898608 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 859152



Sign in to add a comment

RemoteMacViews: Drag-drop does not work

Project Member Reported by ccameron@chromium.org, Oct 24

Issue description

Drag-drop behavior has not been added to RemoteMacViews.

This is in two places. In RemoteMacViews, we don't even instantiate a WebContentsViewCocoa in the app shim process, so we don't have any of the drag-drop behavior hooked up.

In MacViews we instantiate the drag drop client only when the BridgedNativeWidget is instantiated in the same process as the BridgedNativeWidgetHost [0]

I would argue that the more critical drag-drop behavior is WebContentsViewCocoa, because such PWAs as Google Photos rely heavily on this behavior.

[0] https://cs.chromium.org/chromium/src/ui/views/cocoa/bridged_native_widget_host_impl.mm?rcl=f409b1d98282ab42a12628376f0910d21b3c4073&l=253
 
Labels: proj-MacPwa
Components: UI>Browser>WebAppInstalls
Labels: -proj-MacPwa OS-Mac
Cc: ccameron@chromium.org
 Issue 904033  has been merged into this issue.
Labels: -Pri-3 M-73 Pri-1
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 6

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

commit ec34e18cd852699fe8a2b2df2825d6e1d41a8e03
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Dec 06 05:45:54 2018

Split web_contents_view_mac into Cocoa and C++ files

Strictly cut-and-paste patch, should have no functional changes.

At present, we don't instantiate a WebContentsViewCocoa in the PWA
process (rather, we just create a vanilla NSView in
WebContentsNSViewBridge's constructor). To add support for drag-drop
in PWAs we'll use a WebContentsViewCocoa in the PWA process, and then
mojo-ify the boundaries needed for drag-drop support.

Bug: 898608
Change-Id: I90da19a0508872db8f39123d0e55d4bd24afe696
Reviewed-on: https://chromium-review.googlesource.com/c/1363966
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614278}
[modify] https://crrev.com/ec34e18cd852699fe8a2b2df2825d6e1d41a8e03/content/browser/BUILD.gn
[add] https://crrev.com/ec34e18cd852699fe8a2b2df2825d6e1d41a8e03/content/browser/web_contents/web_contents_view_cocoa.h
[add] https://crrev.com/ec34e18cd852699fe8a2b2df2825d6e1d41a8e03/content/browser/web_contents/web_contents_view_cocoa.mm
[modify] https://crrev.com/ec34e18cd852699fe8a2b2df2825d6e1d41a8e03/content/browser/web_contents/web_contents_view_mac.h
[modify] https://crrev.com/ec34e18cd852699fe8a2b2df2825d6e1d41a8e03/content/browser/web_contents/web_contents_view_mac.mm
[modify] https://crrev.com/ec34e18cd852699fe8a2b2df2825d6e1d41a8e03/content/browser/web_contents/web_contents_view_mac_unittest.mm

Cc: mgiuca@chromium.org sdy@chromium.org
This needs to be done for RemoteMacViews (I had considered it optional for the MVP, but I've changed my mind -- most PWAs I want to use need drag-and-drop).

I'm going to start by splitting out WebContentsView the way that the other NSViews are, and then I'll start playing with the dnd interface -- sadly I know nothing about the API :(
I consider this a lower priority than other blockers for MVP. It would be nice to have, but isn't as important as alt+tab behaviour (i.e., RMV itself), crashes, etc. Since many apps either don't use this, or it's non-core functionality.
Labels: Pri-2
Bulk downgrading some bugs to P2.
Owner: sdy@chromium.org
Status: Assigned (was: Available)
Hoping to get drop-into-web-contents in for 73 -- it doesn't look particularly complicated.

Haven't looked too carefully into drag-from-web-contents yet.
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 4

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

commit 17b37758651d64fcacc77459f6b90ddb7826593b
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Jan 04 19:47:57 2019

RemoveWebContentsMac: Don't reconstruct DropData repeatedly

There exists a TODO to avoid reconstructing the DropData while dragging
(and to instead populate it only once). We will be moving construction
of the DropData into another process that won't know when it will need
to be repopulated (which would naively require reconstructing the
DropData at every mouse move event, which would be a bit much).

Avoid this by adding a message to specify the DropData before a drop
operation begins, and re-filter this data every time it would have
been reconstructed.

Also delete unused arguments from draggingExited.

Bug: 898608
Change-Id: I0adc87210552f0d5454716e5ee1c8d6eab4b5b35
Reviewed-on: https://chromium-review.googlesource.com/c/1395982
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620035}
[modify] https://crrev.com/17b37758651d64fcacc77459f6b90ddb7826593b/content/browser/web_contents/web_contents_view_cocoa.mm
[modify] https://crrev.com/17b37758651d64fcacc77459f6b90ddb7826593b/content/browser/web_contents/web_drag_dest_mac.h
[modify] https://crrev.com/17b37758651d64fcacc77459f6b90ddb7826593b/content/browser/web_contents/web_drag_dest_mac.mm
[modify] https://crrev.com/17b37758651d64fcacc77459f6b90ddb7826593b/content/browser/web_contents/web_drag_dest_mac_unittest.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 6

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

commit 621867c9c0a404a9a6073c71f1fd25c59931aa6d
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sun Jan 06 23:54:10 2019

MacPWAs drag-drop: Create a structure for drag parameters

Create a DraggingInfo structure with all parameters that are needed for
WebDragDest's  draggingEntered, draggingUpdated, and
performDragOperation methods.

This structure will be mojo-ified and then passed from the
WebContentsViewCocoa (in the PWA process) to the WebDragDest (in the
browser process).

Bug: 898608
Change-Id: I1c0935fa907185a02b6f86a90fc3f2708276004b
Reviewed-on: https://chromium-review.googlesource.com/c/1397323
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620235}
[modify] https://crrev.com/621867c9c0a404a9a6073c71f1fd25c59931aa6d/content/browser/web_contents/web_contents_view_cocoa.mm
[modify] https://crrev.com/621867c9c0a404a9a6073c71f1fd25c59931aa6d/content/browser/web_contents/web_drag_dest_mac.h
[modify] https://crrev.com/621867c9c0a404a9a6073c71f1fd25c59931aa6d/content/browser/web_contents/web_drag_dest_mac.mm
[modify] https://crrev.com/621867c9c0a404a9a6073c71f1fd25c59931aa6d/content/browser/web_contents/web_drag_dest_mac_unittest.mm

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 8

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

commit 703c46b0263412c8805a8681989b49ef2c887000
Author: Avi Drissman <avi@chromium.org>
Date: Tue Jan 08 22:20:35 2019

Revert "MacPWAs drag-drop: Create a structure for drag parameters"

This reverts commit 621867c9c0a404a9a6073c71f1fd25c59931aa6d.

Reason for revert: Breaks drag in chrome://apps,  https://crbug.com/919871 

Original change's description:
> MacPWAs drag-drop: Create a structure for drag parameters
> 
> Create a DraggingInfo structure with all parameters that are needed for
> WebDragDest's  draggingEntered, draggingUpdated, and
> performDragOperation methods.
> 
> This structure will be mojo-ified and then passed from the
> WebContentsViewCocoa (in the PWA process) to the WebDragDest (in the
> browser process).
> 
> Bug: 898608
> Change-Id: I1c0935fa907185a02b6f86a90fc3f2708276004b
> Reviewed-on: https://chromium-review.googlesource.com/c/1397323
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: ccameron <ccameron@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620235}

TBR=avi@chromium.org,ccameron@chromium.org

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

Bug: 898608
Change-Id: Ie2424ddcee56784ff6c22dd8fb7a71708b70607b
Reviewed-on: https://chromium-review.googlesource.com/c/1400778
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620904}
[modify] https://crrev.com/703c46b0263412c8805a8681989b49ef2c887000/content/browser/web_contents/web_contents_view_cocoa.mm
[modify] https://crrev.com/703c46b0263412c8805a8681989b49ef2c887000/content/browser/web_contents/web_drag_dest_mac.h
[modify] https://crrev.com/703c46b0263412c8805a8681989b49ef2c887000/content/browser/web_contents/web_drag_dest_mac.mm
[modify] https://crrev.com/703c46b0263412c8805a8681989b49ef2c887000/content/browser/web_contents/web_drag_dest_mac_unittest.mm

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 12

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

commit 49175a793f69c78e3f8fb3906d1fc78a59ed8bd5
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sat Jan 12 08:34:46 2019

Add mojo support for content::DropData

Cut-and-paste IPC traits for content::DropData from its non-public home
in drag_traits.h to the public common_param_traits_macros.h.

Add mojo definitions and typemaps.

Bug: 898608
Change-Id: I6f9f3d5f2ff479a84dae20deacc34513611929e1
Reviewed-on: https://chromium-review.googlesource.com/c/1397421
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622300}
[modify] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/common/content_message_generator.h
[modify] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/common/drag_traits.h
[modify] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/public/common/BUILD.gn
[modify] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/public/common/common_param_traits_macros.h
[add] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/public/common/drop_data.mojom
[add] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/public/common/drop_data.typemap
[modify] https://crrev.com/49175a793f69c78e3f8fb3906d1fc78a59ed8bd5/content/public/common/typemaps.gni

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 12

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

commit 6df50f51111a088ee6ac7427e1b193073f5f7efe
Author: Christopher Cameron <ccameron@chromium.org>
Date: Sat Jan 12 18:18:38 2019

MacPWAs drag-drop: Create a structure for drag parameters

Create a DraggingInfo structure with all parameters that are needed for
WebDragDest's  draggingEntered, draggingUpdated, and
performDragOperation methods.

This structure will be mojo-ified and then passed from the
WebContentsViewCocoa (in the PWA process) to the WebDragDest (in the
browser process).

Bug: 898608
Change-Id: I2f88c081599d2e89c5a4a3117fb8b867968b18af
Reviewed-on: https://chromium-review.googlesource.com/c/1407969
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622319}
[modify] https://crrev.com/6df50f51111a088ee6ac7427e1b193073f5f7efe/content/browser/web_contents/web_contents_view_cocoa.mm
[modify] https://crrev.com/6df50f51111a088ee6ac7427e1b193073f5f7efe/content/browser/web_contents/web_drag_dest_mac.h
[modify] https://crrev.com/6df50f51111a088ee6ac7427e1b193073f5f7efe/content/browser/web_contents/web_drag_dest_mac.mm
[modify] https://crrev.com/6df50f51111a088ee6ac7427e1b193073f5f7efe/content/browser/web_contents/web_drag_dest_mac_unittest.mm

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 17 (5 days ago)

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

commit 6d6d3adf6fe76581e388245c073640fa7d73596b
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Jan 17 18:47:21 2019

Mojo-ify calls from WebContentsViewCocoa to WebContentsViewMac

WebContentsViewCocoa is in the app shim process and WebContentsViewMac
is in the browser process, so calls from one to the other must go
through mojo.

Move WebDragDest from being owned by WebContentsViewCocoa to being
owned by WebContentsViewMac.

Don't convert the calls for acting as a drag-drop source, since that
will need further refactoring in the future.

Bug: 898608
Change-Id: Id85c2259daf74ecea3d145923b0a2f2974d182ed
Reviewed-on: https://chromium-review.googlesource.com/c/1416470
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623769}
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/browser/web_contents/web_contents_view_cocoa.h
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/browser/web_contents/web_contents_view_cocoa.mm
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/browser/web_contents/web_contents_view_mac.h
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/browser/web_contents/web_contents_view_mac.mm
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/browser/web_contents/web_drag_dest_mac.h
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/browser/web_contents/web_drag_dest_mac.mm
[modify] https://crrev.com/6d6d3adf6fe76581e388245c073640fa7d73596b/content/public/common/web_contents_ns_view_bridge.mojom

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit 124330a4c9ae231daa17ec1262727abf046cfe80
Author: Christopher Cameron <ccameron@chromium.org>
Date: Fri Jan 18 19:23:28 2019

Clean up usage of WebContentsNSViewBridge

In NSViewBridgeFactoryImpl::CreateWebContentsNSViewBridge, use
mojo::MakeStrongAssociatedBinding to make the message pipe own the
WebContentsNSViewBridge (the prior implementation was a "roll your
own" of this).

Make WebContentsViewMac have a WebContentsNSViewBridge instead of
having the WebContentsViewCocoa.

Bug: 898608
Change-Id: I9070d22ccedc3107a533b4e8a085c861a382226a
Reviewed-on: https://chromium-review.googlesource.com/c/1419697
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624230}
[modify] https://crrev.com/124330a4c9ae231daa17ec1262727abf046cfe80/content/browser/ns_view_bridge_factory_impl.mm
[modify] https://crrev.com/124330a4c9ae231daa17ec1262727abf046cfe80/content/browser/web_contents/web_contents_ns_view_bridge.h
[modify] https://crrev.com/124330a4c9ae231daa17ec1262727abf046cfe80/content/browser/web_contents/web_contents_ns_view_bridge.mm
[modify] https://crrev.com/124330a4c9ae231daa17ec1262727abf046cfe80/content/browser/web_contents/web_contents_view_mac.h
[modify] https://crrev.com/124330a4c9ae231daa17ec1262727abf046cfe80/content/browser/web_contents/web_contents_view_mac.mm

Project Member

Comment 18 by bugdroid, Today (9 hours ago)

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

commit e53b61685fb4aa4321c944fedf7a4c5181a5b13a
Author: Christopher Cameron <ccameron@chromium.org>
Date: Tue Jan 22 22:35:01 2019

Clean up WebContentsNSView mojo methods

Some methods (e.g, Show) are a strange in that they do multiple
only-semi-related things. Un-tangle those methods.

Add an accessibility argument to SetParentNSView, and add an explicit
ResetParentNSView to detach (because detach and destroy are not always
coupled).

Add a mojo method for selection iteration.

Bug: 898608
Change-Id: I5f9d2f3bd204d7d3a06dff9e3f4259f30011d6c2
Reviewed-on: https://chromium-review.googlesource.com/c/1427785
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624968}
[modify] https://crrev.com/e53b61685fb4aa4321c944fedf7a4c5181a5b13a/content/browser/web_contents/web_contents_ns_view_bridge.h
[modify] https://crrev.com/e53b61685fb4aa4321c944fedf7a4c5181a5b13a/content/browser/web_contents/web_contents_ns_view_bridge.mm
[modify] https://crrev.com/e53b61685fb4aa4321c944fedf7a4c5181a5b13a/content/browser/web_contents/web_contents_view_mac.mm
[modify] https://crrev.com/e53b61685fb4aa4321c944fedf7a4c5181a5b13a/content/public/common/web_contents_ns_view_bridge.mojom

Sign in to add a comment