[Webkit-unassigned] [Bug 53727] Clone WebClipboard to be frame-specific

[bugzilla-daemon at webkit.org]

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


Dmitry Titov <dimich at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #81159|review?                     |review-
               Flag|                            |




--- Comment #8 from Dmitry Titov <dimich at chromium.org>  2011-02-03 18:31:02 PST ---
(From update of attachment 81159)
View in context: https://bugs.webkit.org/attachment.cgi?id=81159&action=review

A few comments (r- to clarify the lifetime of stored Document*):

> Source/WebCore/ChangeLog:16
> +        No new tests.

Lets explain more here. Is it because this code path is not yet enabled and requires more changes in Chromium so when it'll be enabled the existing tests will cover?

> Source/WebCore/platform/chromium/ChromiumDataObject.cpp:60
> +    const Document* document, Clipboard::ClipboardType clipboardType)

WebKit doesn't try to fit in 80 chars. Same for the other files in patch.

> Source/WebCore/platform/chromium/ReadableDataObject.h:67
> +    const Document* m_document;

2 questions here: 
- If it is not a RefPtr, we should have a good theory why the document is guaranteed to be alive for the lifetime of ReadableDataObject, or we should clear it (and check in getClipboard), or make this a RefPtr.
- Why isn't it Frame?

> Source/WebKit/chromium/public/WebFrameClient.h:92
> +    // A frame specific clipboard. May return null, in which case... you're done.

It's be better to say a bit more, "the caller should assume there is no data in clipboard" or something like that?

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