There is already indexesFromTimeRange that you can use instead of adding new method. Also, timestamp is integer and not double.

This is completely wrong, it can actually lead to invalid results now due to possibility of this reference always having the same address (because it is on stack).
Do you know what this code does now?

Rename to clear()

Don't use C-style casts, use static_cast<int> or int(value)

This is not system include

You can just declare result as const instead of using qAsConst.

You should be able to get engine with qmlEngine(this) instead of needing to pass the plugin path manually.

Do you want it like this -
void QmlHistory::deleteRange(const QVariantMap &map)
{

if (!map.contains(QSL("startTime")) || !map.contains(QSL("endTime"))) {
    qWarning() << "Error:" << "wrong arguments passed to" << __FUNCTION__;
    return;
}
const qlonglong startTime = map.value(QSL("startTime")).toLongLong();
const qlonglong endTime = map.value(QSL("endTime")).toLongLong();

QSqlQuery query(SqlDatabase::instance()->database());
query.prepare(QSL("DELETE FROM history WHERE date BETWEEN ? AND ?"));
query.bindValue(0, startTime);
query.bindValue(1, endTime);
query.exec();

}

Here deleteRange is removed from history.h. Also if indexFromTimeRange is used then it wil have to execute query twice (once to get the list of ids to remove and other to remove the ids)

Also if indexFromTimeRange is used then it wil have to execute query twice

That's fine, this won't be called often.

Sorry, but your comment is unclear to me, do you mean the snipped I posted is fine or to delete the ids by first using indexFromTimeRange?

Delete the ids by first using indexFromTimeRange.

I have thinked over it and I am unable to find a good solution to where to include it outside of mainapplication, since its used in both qmltabs and qmlwindows. Do you have some specific location in mind?

For example add new class that will hold all static data, including everything else from all *Data classes, so that way there is only one static data class.
If you make it QObject then you can just parent all data to this class and don't need to explicitly set CppOwnership.

Why did you change the for loop? Unless there is a reason, don't use for(int i = 0, ....)

Missing initialization

This should not be needed.

The question still stands? If it doesn't get to the QML engine, it will leak.

Example where it may happen (I did not test it myself, so plase check) is if you instantiate Falkon.ExtensionScheme { } item in QML but don't create the requestStarted handler (eg. no onRequestStarted code).

?

Why? You can get the QmlEngine pointer with qmlEngine(QObject*) function that is called on any QObject that was created by the engine.

Why not just create QSettings with correct parent?

This seems like the same code for handling icon loading is pasted on multiple places, should be extracted to function.

init

Again, use stl iterators. This creates temporary list only to iterate over it, that's not needed at all.

QList<QObject*>

Just make it static variable in loadPlugin function as it is not used anywhere else.

Why this at all? It should just use staticData->getTab!

There should be no whitespace before/after increment, so it should be foo++.

remove

It should be something like static QmlStaticData* instance() member function and not adding staticData to global namespace.

Move the url to variable so it isn't copied thousand times.

Most (all?) these singletons should be shared with multiple engines (=multiple loaded qml extensions). I think it creates new instance for every engine right now.

Doesn't for(BookmarkItem *bookmarkitem : items) detaches the list? Should I use QListIterator<BookmarkItem *> it(items) ?

No, because items is const. As long as the Qt container is const, you can use C++11 for.

Its not designed to be initialized explicitly, but checks must be added in setIcon where it can be null eq in auto qmlEngine = qobject_cast<QmlEngine*>(m_popup->creationContext()->engine());

Sorry, that was meant for m_locations, one line above.

Its is created per engine, so shouldn't it work?

Did you try?

Its not the extension root path which is known by QmlEngine, but the path of entry point of the qml plugin. The code here gets the plugin root path given the path of entry file.

I don't understand. Please show me example how does the "extension root path" and "path of entry point" differs and why does it matter here?

Entry path and root path differs because in metadata.desktop we can specify entry file as - X-Falkon-EntryPoint=demo/main.qml, which will point to the main.qml file within the demo directory of plugin (root-dir-of-plugin/demo/main.qml) now the extensionpath set in the QmlEngine is the entry point path. Also you told previously that the paths must be relative to root irrespective of the qml file, so I added this. For example in this case -
FilePath & ExtensionPath is "/home/tarptaeya/.config/falkon/plugins/qml/TestPlugin/demo/main.qml"
m_path is "/home/tarptaeya/.config/falkon/plugins/qml/TestPlugin/"

FilePath & ExtensionPath is "/home/tarptaeya/.config/falkon/plugins/qml/TestPlugin/demo/main.qml"

This is entry point and is only needed for creating the QML component, and is never used again.

m_path is "/home/tarptaeya/.config/falkon/plugins/qml/TestPlugin/"

Still I don't see the problem. This path is what you have stored in QmlEngine (and if you don't, you should!) and only this path is needed to resolve relative paths. That means this is "basePath" of the extension which will be used for resolving all relative paths (including what you get in X-Falkon-EntryPoint). This also means that the only path that needs to be absolute is this, everything else can be stored as relative path and resolved against it.

No, the menus and action are not created by the engine, but are created (menu) in qmlplugininterface (and action in menu). But it seem to me that they leak? Should I set the parent of the qmlMenu as qmlpluginInterface or store them in a list which will be deleted in the destructor of qmlplugininterface?

You don't need to get the QmlEngine pointer in constructor, and later you call QmlEngine::newQObject on it, so after that you should get the correct pointer from qmlEngine(QObject*) I think.

Well, now it doesn't leak but it will only be released on exiting application, so it's effectively the same problem and you didn't fix anything.

Why? This shouldn't be needed.
Always keep paths without trailing slash.

What does this do?

If you know beforehand how big the resulting list will be, you should call reserve.

Please use STL iterators.

auto h = QmlStaticData::instance().windowIdHash();
for (auto it = h.constBegin(); it != h.constEnd(); ++it) {
}
/// or cbegin()/cend()

This is equivalent to

return QmlStaticData::instance().windowIdHash().value(m_window, -1);

This is not member variable so it shouldn't have m_ prefix. It shouldn't have any prefix.

You are casting twice here, data.value<QmlPluginLoader*>() already gives you QmlPluginLoader*

const

Why it isn't created on heap and parented to QmlStaticData? That way you don't need to manually specify CppOwnership. Ideally you should only specify ownership explicitly only when there is no better way.

static QmlStaticData &instance();

Then how to fix this?

If locale is an empty string, "", each part of the locale that should be modified is set according to the environment variables.

QmlEngine::newQObject as you already used in other places. But first test if it works as expected.

You already call newQObject on all events, which makes them JavaScriptOwnership, so it isn't needed here.

You can still use the QmlWebEngineUrlRequestJob pointer in signal, not QJSValue.

Why does it use QUrl?

You can just do path + / + filePath and then QDir::cleanPath.

And is that really needed?
I'd like to see explicitly choosing which language to load here.

Then it needs to make the jsvalue return of engine->newQObject as unused? something like -
QmlWebEngineUrlRequestJob *request = new QmlWebEngineUrlRequestJob(job);
auto jsval = qmlEngine(this)->newQObject(request);
emit requestStarted(request);
?

You mean something like Falkon.I18n.setLocale() on qml side?

No, just calling qmlEngine(this)->newQObject(request); should be enough.

Did you actually try it?

No, on the cpp side so when/if there is a support for switching languages it is easy to implement.

Also translations functions should be added to global namespace, so they are accessible with just i18n/i18np without Falkon namespace.

Yes I tried. But qmlEngine(this)->newQObject(request); returns QJSValue - so its need to be changed to the signal paramater

Tried how? Did you change the code so it just calls qmlEngine(this)->newQObject(request); and then checked if that object gets correctly garbage collected by engine (deleted)?

connect(m_schemeHandler, &QmlExtensionSchemeHandler::_requestStarted, this, [this](QWebEngineUrlRequestJob *job) {
        QmlWebEngineUrlRequestJob *request = new QmlWebEngineUrlRequestJob(job);
        emit requestStarted(qmlEngine(this)->newQObject(request));
    });

Or did you meant the snippet which I send in previous comment?

I meant what I told you to do, just call qmlEngine(this)->newQObject(request);

connect(m_schemeHandler, &QmlExtensionSchemeHandler::_requestStarted, this, [this](QWebEngineUrlRequestJob *job) {
    QmlWebEngineUrlRequestJob *request = new QmlWebEngineUrlRequestJob(job);
    qmlEngine(this)->newQObject(request);
    emit requestStarted(request);
});

on the cpp side so when/if there is a support for switching languages it is easy to implement.

Sorry, I didn't understood this. Can you please elaborate.

Also translations functions should be added to global namespace, so they are accessible with just i18n/i18np without Falkon namespace.

using engine contextProperty? or is there some other way?

Something like setlocale(QLocale::system().name())

Yes, contextProperty is fine if it works.

No, context property is not working. It seem like there is no way to make a method global (max is object.method ex i18n.i18n), is there some way? I couldn't found any in the documentation.

This can be similar to qsTr in Qt source, something like -
engine->globalObject().setProperty("i18n", engine->evaluate("function(string) { return i18n.i18n(string) }"));
Here i18n is the context property (QObject corresponding to QmlI18n).
But It has problems -

1. i18n as contextProperty object
2. It must be added to the engine at time of its creation i.e. remove singleton of i18n from QmlPlugins and move i18n to engine global object when the engine is created in QmlPluginLoader.

Should I change I18n so that i18n adds to global namespace or leave it now? Or is there some better solution

i18n as contextProperty object

Is that an issue?

It must be added to the engine at time of its creation i.e. remove singleton of i18n from QmlPlugins and move i18n to engine global object when the engine is created in QmlPluginLoader.

I don't see any problem with this.

If it works, please do it this way.

I asked you to test how it works, so what are your findings? I'm not myself sure if this is actually correct, as you may need to pass the QJSValue to QML (as you did) to not be garbage collected if the reference is held on QML side.
Eg. save the reference to the job on QML side and check if it is still valid after some time (with Timer).

I don't think it should call setlocale at all, that's QLocale job. Take a look how is this done in ki18n framework.

QStringLiteral