Code review comment for lp:~abreu-alexandre/oxide/clipboard

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?

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 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

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.

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