> Is the license and copyright header in
> qt/core/browser/oxide_qt_clipboard.[h|cc] correct?
> 
> In qt/core/browser/oxide_qt_browser_platform_integration.h:
>  - shouldn’t the override of GetClipboardOxideFactory() be annotated with
> 'override' instead of 'final' ?
> 
> In qt/core/browser/oxide_qt_clipboard.h:
>  - is "friend class Clipboard;" needed?
>  - does the 'override' keywoard makes sense for a destructor?

yes,

> In qt/core/browser/oxide_qt_clipboard.cc:
>  - ClipboardChangedListener being a QObject, it would be more consistent to
> use Q_DISABLE_COPY instead of DISALLOW_COPY_AND_ASSIGN
>  - In ClipboardChangedListener’s class declaration, the alignment of the
> visibility keywords is inconsistent
>  - In ClipboardChangedListener::OnClipboardDataChanged, can you add an
> indentation level for 'case' statements and their contents?
>  - shouldn’t ClipboardQt::WriteWebSmartPaste() be marked NOTIMPLEMENTED ?

it not actually, this is a special tag (mostly like a shell 'touch') that touches
the clipboard w/ null data

>  - it seems the checks on the nullness of QGuiApplication::clipboard() are not
> necessary, we can safely assume it will never be null (other code throughout
> Qt makes this assumption already)
> 
> In qt/tests/qmltests/core/tst_Clipboard.html:
>  - div with id "blabla" is unused, can it be removed?
> 
> In qt/tests/qmltests/core/tst_Clipboard.qml:
>  - in expect_has_file(), [result = result == 'true'] is incomplete: if
> 'expected' is false and 'result' is anything but "false" (including the empty
> string), the function should return false
>  - indentation should use 2 spaces, not 4
>  - test_copy() takes an unused 'data' parameter, can it be removed?
> 
> In qt/tests/qmltests/oxide_qml_testing_plugin.cc:
>  - Qt includes are not ordered alphabetically
>  - the newline in the middle of the first call to setData in copyToClipboard()
> is not necessary

addressed all the above (unless otherwise noted)

> There are a few of my previous comments that haven’t been addressed/answered,
> copying them here for reference:
> 
> > We’re missing unit tests for (at least) HTML markup.
> 
> >> The image we get from QImage and the one we pasted are slightly
> >> different but overall is the same image. QImage does some "processing"
> >> on the raw image content that slightly alters it and make it hard to
> >> have an exact match.
> > How is that even possible? We’re talking about an uncompressed,
> > lossless PNG, right?
> > And I don’t see where QImage is involved in test_paste(), only raw data
> > is passed around as QByteArrays.

as I said in my previous comment, there is a conversion happening, from ARGB -> RGBA

> > In ClipboardQt::ReadAvailableTypes():
> >   - why not push types directly to the passed vector, instead of using
> >     an temporary vector and swapping its contents afterwards?

it is mostly an exception safety reflex, not being destructive of args, etc. but
it obviously does not apply here

> >   - shouldn’t contains_filenames be set accordingly if one of the
> >     formats is "chromium/filename" ?
> > In ClipboardQt::ReadHTML():
> >   - do we really need to clear markup and src_url prior to setting
> >     their content?