[Webkit-unassigned] [Bug 44992] [chromium] Prepare Clipboard/DragData for transition to new drag-and-drop interface.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44992


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66287|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #16 from Tony Chang <tony at chromium.org>  2010-09-02 15:27:48 PST ---
(From update of attachment 66287)
> +        // |title| can be NULL
> +        if (title)

I realize this is a comment and just moving code, but this should still be 0 instead of NULL.

> diff --git a/WebCore/platform/chromium/DragDataRef.h b/WebCore/platform/chromium/DragDataRef.h
> -    typedef ChromiumDataObject* DragDataRef;
> +struct DragDataRef {
> +    // FIXME: These are deliberately non-explicit for the moment. Once Chromium
> +    // is switched over to the new way of handling drags, revert this change.
> +    DragDataRef(ChromiumDataObject* data) : oldWay(data), newWay(0) {}
> +    DragDataRef(ReadableDataObject* data) : oldWay(0), newWay(data) {}
> +    ChromiumDataObject* oldWay;
> +    ReadableDataObject* newWay;
> +};

Hmm, I think we can do better here.  Here's an idea:
CDO has a member variable for an RDO (maybe an OwnPtr).  We expose an API on CDO that matches the API on RDO and convert DragDataChromium.cpp to use this API.  If we're doing things the "old way", it just uses CDO data.  If we're doing it the new way, it delegates to RDO.  This would just put the ugliness in CDO rather than DragDataChromium.cpp, but would allow us to migrate DragDataChromium.cpp to the new calling conventions.  Later, after we've fully switched to the "new way", we would remove CDO and put RDO in its place.

Does that sound possible?

> diff --git a/WebKit/chromium/src/WebViewImpl.cpp b/WebKit/chromium/src/WebViewImpl.cpp
>  void WebViewImpl::dragTargetDragLeave()
>  {
> +    // FIXME: This is temporary while until Chromium side is transitioned to use
> +    // the new dragging interface.
> +    if (m_currentDragData.get()) {
> +        DragData dragData(
> +            m_currentDragData.get(),
> +            IntPoint(),
> +            IntPoint(),
> +            static_cast<DragOperation>(m_operationsAllowed));
> +
> +        m_dragTargetDispatch = true;
> +        m_page->dragController()->dragExited(&dragData);
> +        m_dragTargetDispatch = false;
> +    } else if (m_currentDragDataNew.get()) {
> +        DragData dragData(
> +            m_currentDragDataNew.get(),
> +            IntPoint(),
> +            IntPoint(),
> +            static_cast<DragOperation>(m_operationsAllowed));

Is it possible to do something like:
DragData dragData(m_currentDragData.get() ? m_currentDragData.get() : m_currentDragDataNew.get(), ...)?

If so, you don't need the huge if statements and duplicate code.  Same for the other methods below.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.