From 66964ac91c62603d7af8da957fca840962ee923d Mon Sep 17 00:00:00 2001 From: hjk Date: Tue, 14 Jul 2015 16:59:46 +0200 Subject: [PATCH] Debugger: Rework QmlEngine watcher expansion Use callbacks in QmlEnginePrivate::evaluate(). This separates the four code paths through the machinery into three separate handlers and one direct access to the console. This also fixes a bug where items were put into 'debuggerCommands' but attempted to be removed from 'updateLocalsAndWatchers'. Introduce a QmlEngine::updateLocals similar to what the other engines do. Let the frame() and assignValue() paths use it. Keep track of pending changes and call notifyUpdateFinished if and only if the pending lookup queues is empty. Finally, remove some dead code. Change-Id: I173a52911d0de994b849fc6ab4f52ef7f64a8ba5 Reviewed-by: Christian Stenger --- src/plugins/debugger/qml/qmlengine.cpp | 376 +++++++++---------------- src/plugins/debugger/qml/qmlengine.h | 2 - 2 files changed, 133 insertions(+), 245 deletions(-) diff --git a/src/plugins/debugger/qml/qmlengine.cpp b/src/plugins/debugger/qml/qmlengine.cpp index c90d89ff490..aab9446c00a 100644 --- a/src/plugins/debugger/qml/qmlengine.cpp +++ b/src/plugins/debugger/qml/qmlengine.cpp @@ -120,6 +120,14 @@ struct QmlV8ObjectData typedef std::function QmlCallback; +struct LookupData +{ + QByteArray iname; + QString name; +}; + +typedef QMultiHash LookupItems; // id -> (iname, exp) + class QmlEnginePrivate : QmlDebugClient { public: @@ -136,11 +144,10 @@ public: void continueDebugging(StepAction stepAction); - void evaluate(const QString expr, bool global = false, bool disableBreak = false, - int frame = -1, bool addContext = false); - void lookup(const QList handles); + void evaluate(const QString expr, const QmlCallback &cb); + void lookup(const LookupItems &items); void backtrace(); - void frame(int number); + void updateLocals(); void scope(int number, int frameNumber = -1); void scripts(int types = 4, const QList ids = QList(), bool includeSource = false, const QVariant filter = QVariant()); @@ -151,16 +158,13 @@ public: void clearBreakpoint(int breakpoint); void setExceptionBreak(Exceptions type, bool enabled = false); - void clearCache(); - - void expandObject(const QByteArray &iname, quint64 objectId); void flushSendBuffer(); void handleBacktrace(const QVariantMap &response); void handleLookup(const QVariantMap &response); - void handleEvaluate(int sequence, bool success, const QVariant &bodyVal); + void handleExecuteDebuggerCommand(const QVariantMap &response); + void handleEvaluateWatcher(const QVariantMap &response, const QString &expr); void handleFrame(const QVariantMap &response); - void handleFrameHelper(const QVariantMap ¤tFrame); void handleScope(const QVariantMap &response); void handleVersion(const QVariantMap &response); StackFrame extractStackFrame(const QVariant &bodyVal); @@ -175,6 +179,7 @@ public: void memorizeRefs(const QVariant &refs); QmlV8ObjectData extractData(const QVariant &data) const; void insertSubItems(WatchItem *parent, const QVariantList &properties); + void checkForFinishedUpdate(); ConsoleItem *constructLogItemTree(ConsoleItem *parent, const QmlV8ObjectData &objectData); public: @@ -185,10 +190,7 @@ public: QHash breakpointsSync; QList breakpointsTemp; - QHash evaluatingExpression; - QHash localsAndWatchers; - QList updateLocalsAndWatchers; - QList debuggerCommands; + LookupItems currentlyLookingUp; // Id -> inames //Cache QList currentFrameScopes; @@ -344,11 +346,6 @@ void QmlEngine::connectionEstablished() { attemptBreakpointSynchronization(); - if (!watchHandler()->watcherNames().isEmpty()) - synchronizeWatchers(); - connect(watchModel(), &QAbstractItemModel::layoutChanged, - this, &QmlEngine::synchronizeWatchers); - if (state() == EngineRunRequested) notifyEngineRunAndInferiorRunOk(); } @@ -521,9 +518,6 @@ void QmlEngine::gotoLocation(const Location &location) void QmlEngine::closeConnection() { - disconnect(watchModel(), &QAbstractItemModel::layoutChanged, - this, &QmlEngine::synchronizeWatchers); - if (d->connectionTimer.isActive()) { d->connectionTimer.stop(); } else { @@ -755,11 +749,10 @@ void QmlEngine::activateFrame(int index) if (state() != InferiorStopOk && state() != InferiorUnrunnable) return; - if (index != stackHandler()->currentIndex()) - d->frame(d->stackIndexLookup.value(index)); - stackHandler()->setCurrentIndex(index); gotoLocation(stackHandler()->frames().value(index)); + + d->updateLocals(); } void QmlEngine::selectThread(ThreadId threadId) @@ -958,17 +951,16 @@ bool QmlEngine::canHandleToolTip(const DebuggerToolTipContext &) const } void QmlEngine::assignValueInDebugger(WatchItem *item, - const QString &expression, const QVariant &valueV) + const QString &expression, const QVariant &value) { if (!expression.isEmpty()) { if (item->isInspect() && d->inspectorAdapter.agent()) { - d->inspectorAdapter.agent()->assignValue(item, expression, valueV); + d->inspectorAdapter.agent()->assignValue(item, expression, value); } else { StackHandler *handler = stackHandler(); - QString expression = QString(_("%1 = %2;")).arg(expression).arg(valueV.toString()); + QString exp = QString(_("%1 = %2;")).arg(expression).arg(value.toString()); if (handler->isContentsValid() && handler->currentFrame().isUsable()) { - d->evaluate(expression, false, false, handler->currentIndex()); - d->updateLocalsAndWatchers.append(d->sequence); + d->evaluate(exp, [this](const QVariantMap &) { d->updateLocals(); }); } else { showMessage(QString(_("Cannot evaluate %1 in current stack frame")).arg( expression), ConsoleOutput); @@ -979,8 +971,6 @@ void QmlEngine::assignValueInDebugger(WatchItem *item, void QmlEngine::updateWatchData(const QByteArray &iname) { -// qDebug() << "UPDATE WATCH DATA" << data.toString(); - //showStatusMessage(tr("Stopped."), 5000); const WatchItem *item = watchHandler()->findItem(iname); // invalid expressions or out of scope variables if (!item) @@ -990,10 +980,12 @@ void QmlEngine::updateWatchData(const QByteArray &iname) d->inspectorAdapter.agent()->updateWatchData(*item); } else { if (!item->name.isEmpty()) { - if (item->isChildrenNeeded() && watchHandler()->isExpandedIName(item->iname)) - d->expandObject(item->iname, item->id); + if (item->isChildrenNeeded() && watchHandler()->isExpandedIName(item->iname)) { + LookupItems items; + items.insert(int(item->id), {item->iname, item->name}); + d->lookup(items); + } } - synchronizeWatchers(); } } @@ -1004,23 +996,6 @@ void QmlEngine::selectWatchData(const QByteArray &iname) d->inspectorAdapter.agent()->watchDataSelected(item->id); } -void QmlEngine::synchronizeWatchers() -{ - if (state() != InferiorStopOk) - return; - - QStringList watchers = watchHandler()->watchedExpressions(); - - // send watchers list - foreach (const QString &exp, watchers) { - StackHandler *handler = stackHandler(); - if (handler->isContentsValid() && handler->currentFrame().isUsable()) { - d->evaluate(exp, false, false, handler->currentIndex()); - d->evaluatingExpression.insert(d->sequence, exp); - } - } -} - static ConsoleItem *constructLogItemTree(ConsoleItem *parent, const QVariant &result, const QString &key = QString()) @@ -1136,8 +1111,6 @@ void QmlEngine::updateCurrentContext() context = grandParentData->name; } - synchronizeWatchers(); - if (auto consoleManager = ConsoleManagerInterface::instance()) consoleManager->setContext(tr("Context:") + QLatin1Char(' ') + context); } @@ -1149,8 +1122,7 @@ void QmlEngine::executeDebuggerCommand(const QString &command, DebuggerLanguages StackHandler *handler = stackHandler(); if (handler->isContentsValid() && handler->currentFrame().isUsable()) { - d->evaluate(command, false, false, handler->currentIndex()); - d->debuggerCommands.append(d->sequence); + d->evaluate(command, CB(d->handleExecuteDebuggerCommand)); } else { //Currently cannot evaluate if not in a javascript break d->engine->showMessage(QString(_("Cannot evaluate %1 in current stack frame")).arg( @@ -1347,8 +1319,7 @@ void QmlEnginePrivate::continueDebugging(StepAction action) previousStepAction = action; } -void QmlEnginePrivate::evaluate(const QString expr, bool global, - bool disableBreak, int frame, bool addContext) +void QmlEnginePrivate::evaluate(const QString expr, const QmlCallback &cb) { // { "seq" : , // "type" : "request", @@ -1368,36 +1339,45 @@ void QmlEnginePrivate::evaluate(const QString expr, bool global, DebuggerCommand cmd(EVALUATE); cmd.arg(EXPRESSION, expr); + cmd.arg(FRAME, engine->stackHandler()->currentIndex()); - if (frame != -1) - cmd.arg(FRAME, frame); - - if (global) - cmd.arg(GLOBAL, global); - - if (disableBreak) - cmd.arg(DISABLE_BREAK, disableBreak); - - if (addContext) { - WatchHandler *watchHandler = engine->watchHandler(); - QAbstractItemModel *watchModel = watchHandler->model(); - int rowCount = watchModel->rowCount(); - - cmd.beginList(ADDITIONAL_CONTEXT); - for (int row = 0; row < rowCount; ++row) { - QModelIndex index = watchModel->index(row, 0); // FIXME: This looks wrong. - const WatchData *data = watchHandler->watchItem(index); - cmd.beginGroup(); - cmd.arg(NAME, data->name); - cmd.arg(HANDLE, int(data->id)); - cmd.endGroup(); - } - } - - runCommand(cmd); + runCommand(cmd, cb); } -void QmlEnginePrivate::lookup(QList handles) +void QmlEnginePrivate::handleEvaluateWatcher(const QVariantMap &response, const QString &exp) +{ + // { "seq" : , + // "type" : "response", + // "request_seq" : , + // "command" : "evaluate", + // "body" : ... + // "running" : + // "success" : true + // } + + QVariant bodyVal = response.value(_(BODY)).toMap(); + QmlV8ObjectData body = extractData(bodyVal); + WatchHandler *watchHandler = engine->watchHandler(); + + QByteArray iname = watchHandler->watcherName(exp.toLatin1()); + + auto item = new WatchItem(iname, exp); + item->exp = exp.toLatin1(); + item->id = body.handle; + bool success = response.value(_("success")).toBool(); + if (success) { + item->type = body.type; + item->value = body.value.toString(); + item->wantsChildren = body.properties.count(); + } else { + //Do not set type since it is unknown + item->setError(body.value.toString()); + } + watchHandler->insertItem(item); + insertSubItems(item, body.properties); +} + +void QmlEnginePrivate::lookup(const LookupItems &items) { // { "seq" : , // "type" : "request", @@ -1409,6 +1389,12 @@ void QmlEnginePrivate::lookup(QList handles) // } // } + if (items.isEmpty()) + return; + + QList handles = items.keys(); + currentlyLookingUp += items; + DebuggerCommand cmd(LOOKUP); cmd.arg(HANDLES, handles); runCommand(cmd, CB(handleLookup)); @@ -1430,19 +1416,16 @@ void QmlEnginePrivate::backtrace() runCommand(cmd, CB(handleBacktrace)); } -void QmlEnginePrivate::frame(int number) +void QmlEnginePrivate::updateLocals() { // { "seq" : , // "type" : "request", // "command" : "frame", - // "arguments" : { "number" : - // } + // "arguments" : { "number" : } // } DebuggerCommand cmd(FRAME); - if (number != -1) - cmd.arg(NUMBER, number); - + cmd.arg(NUMBER, stackIndexLookup.value(engine->stackHandler()->currentIndex())); runCommand(cmd, CB(handleFrame)); } @@ -1696,12 +1679,6 @@ QmlV8ObjectData QmlEnginePrivate::extractData(const QVariant &data) const return objectData; } -void QmlEnginePrivate::clearCache() -{ - currentFrameScopes.clear(); - updateLocalsAndWatchers.clear(); -} - void QmlEnginePrivate::runCommand(const DebuggerCommand &command, const QmlCallback &cb) { QByteArray msg = "{\"seq\":" + QByteArray::number(++sequence) + "," @@ -1727,24 +1704,6 @@ void QmlEnginePrivate::runDirectCommand(const QByteArray &type, const QByteArray sendMessage(request); } -void QmlEnginePrivate::expandObject(const QByteArray &iname, quint64 objectId) -{ - if (objectId == 0) { - //We may have got the global object - const WatchItem *watch = engine->watchHandler()->findItem(iname); - if (watch->value == QLatin1String("global")) { - StackHandler *stackHandler = engine->stackHandler(); - if (stackHandler->isContentsValid() && stackHandler->currentFrame().isUsable()) { - evaluate(watch->name, false, false, stackHandler->currentIndex()); - evaluatingExpression.insert(sequence, QLatin1String(iname)); - } - return; - } - } - localsAndWatchers.insertMulti(objectId, iname); - lookup(QList() << objectId); -} - void QmlEnginePrivate::memorizeRefs(const QVariant &refs) { if (refs.isValid()) { @@ -1808,17 +1767,6 @@ void QmlEnginePrivate::messageReceived(const QByteArray &data) } else if (debugCommand == _(CONTINEDEBUGGING)) { //do nothing, wait for next break - } else if (debugCommand == _(EVALUATE)) { - int seq = resp.value(_("request_seq")).toInt(); - if (success) { - handleEvaluate(seq, success, resp.value(_(BODY))); - } else { - QVariantMap map; - map.insert(_(TYPE), QVariant(_("string"))); - map.insert(_(VALUE), resp.value(_("message"))); - handleEvaluate(seq, success, QVariant(map)); - } - } else if (debugCommand == _(SETBREAKPOINT)) { // { "seq" : , // "type" : "response", @@ -2062,13 +2010,13 @@ void QmlEnginePrivate::messageReceived(const QByteArray &data) //This is most probably due to a wrong eval expression. //Redirect output to console. if (eventType.isEmpty()) { - bool success = resp.value(_("success")).toBool(); - QVariantMap map; - map.insert(_(TYPE), QVariant(_("string"))); - map.insert(_(VALUE), resp.value(_("message"))); - //Since there is no sequence value, best estimate is - //last sequence value - handleEvaluate(sequence, success, QVariant(map)); + QmlV8ObjectData entry; + entry.type = "string"; + entry.value = resp.value(_("message")); + if (auto consoleManager = ConsoleManagerInterface::instance()) { + if (ConsoleItem *item = constructLogItemTree(consoleManager->rootItem(), entry)) + consoleManager->printToConsolePane(item); + } } } //EVENT @@ -2115,12 +2063,9 @@ void QmlEnginePrivate::handleBacktrace(const QVariantMap &response) i++; } stackHandler->setFrames(stackFrames); + stackHandler->setCurrentIndex(0); - //Populate locals and watchers wrt top frame - //Update all Locals visible in current scope - //Traverse the scope chain and store the local properties - //in a list and show them in the Locals Window. - handleFrameHelper(frames.value(0).toMap()); + updateLocals(); } StackFrame QmlEnginePrivate::extractStackFrame(const QVariant &bodyVal) @@ -2216,35 +2161,23 @@ void QmlEnginePrivate::handleFrame(const QVariantMap &response) // "running" : // "success" : true // } - handleFrameHelper(response.value(_(BODY)).toMap()); -} + QVariantMap body = response.value(_(BODY)).toMap(); -void QmlEnginePrivate::handleFrameHelper(const QVariantMap ¤tFrame) -{ StackHandler *stackHandler = engine->stackHandler(); WatchHandler * watchHandler = engine->watchHandler(); watchHandler->notifyUpdateStarted(); - clearCache(); const int frameIndex = stackHandler->currentIndex(); - QSet expandedInames = watchHandler->expandedINames(); - QHash handlesToLookup; - // Store handles of all expanded watch data - foreach (const QByteArray &iname, expandedInames) { - const WatchItem *item = watchHandler->findItem(iname); - if (item && item->isLocal()) - handlesToLookup.insert(item->id, iname); - } if (frameIndex < 0) return; const StackFrame frame = stackHandler->currentFrame(); if (!frame.isUsable()) return; - //Set "this" variable + // Always add a "this" variable { auto item = new WatchItem("local.this", QLatin1String("this")); - QmlV8ObjectData objectData = extractData(currentFrame.value(_("receiver"))); + QmlV8ObjectData objectData = extractData(body.value(_("receiver"))); item->id = objectData.handle; item->type = objectData.type; item->value = objectData.value.toString(); @@ -2258,7 +2191,8 @@ void QmlEnginePrivate::handleFrameHelper(const QVariantMap ¤tFrame) watchHandler->insertItem(item); } - const QVariantList scopes = currentFrame.value(_("scopes")).toList(); + currentFrameScopes.clear(); + const QVariantList scopes = body.value(_("scopes")).toList(); foreach (const QVariant &scope, scopes) { //Do not query for global types (0) //Showing global properties increases clutter. @@ -2270,11 +2204,24 @@ void QmlEnginePrivate::handleFrameHelper(const QVariantMap ¤tFrame) } engine->gotoLocation(stackHandler->currentFrame()); - // Expand watch data that were previously expanded - QHash::const_iterator itEnd = handlesToLookup.constEnd(); - for (QHash::const_iterator it = handlesToLookup.constBegin(); it != itEnd; ++it) - expandObject(it.value(), it.key()); - engine->stackFrameCompleted(); + // Send watchers list + if (stackHandler->isContentsValid() && stackHandler->currentFrame().isUsable()) { + QStringList watchers = watchHandler->watchedExpressions(); + foreach (const QString &exp, watchers) { + evaluate(exp, [this, exp](const QVariantMap &response) { + handleEvaluateWatcher(response, exp); + }); + } + } + + // Expand locals and watchers that were previously expanded + LookupItems itemsToLookup; + foreach (const QByteArray &iname, watchHandler->expandedINames()) { + const WatchItem *item = watchHandler->findItem(iname); + if (item && item->isLocal()) + itemsToLookup.insert(int(item->id), {item->iname, item->name}); + } + lookup(itemsToLookup); } void QmlEnginePrivate::handleScope(const QVariantMap &response) @@ -2309,7 +2256,7 @@ void QmlEnginePrivate::handleScope(const QVariantMap &response) QmlV8ObjectData objectData = extractData(bodyMap.value(_("object"))); - QList handlesToLookup; + LookupItems itemsToLookup; foreach (const QVariant &property, objectData.properties) { QmlV8ObjectData localData = extractData(property); auto item = new WatchItem; @@ -2320,23 +2267,24 @@ void QmlEnginePrivate::handleScope(const QVariantMap &response) item->name = QLatin1String(item->exp); item->iname = QByteArray("local.") + item->exp; + item->id = localData.handle; - int handle = localData.handle; if (localData.value.isValid()) { - item->id = handle; item->type = localData.type; item->value = localData.value.toString(); item->setHasChildren(localData.properties.count()); engine->watchHandler()->insertItem(item); } else { - handlesToLookup << handle; - localsAndWatchers.insertMulti(handle, item->exp); + itemsToLookup.insert(int(item->id), {item->iname, item->name}); } } + lookup(itemsToLookup); + checkForFinishedUpdate(); +} - if (!handlesToLookup.isEmpty()) - lookup(handlesToLookup); - else +void QmlEnginePrivate::checkForFinishedUpdate() +{ + if (currentlyLookingUp.isEmpty()) engine->watchHandler()->notifyUpdateFinished(); } @@ -2403,68 +2351,16 @@ void QmlEnginePrivate::insertSubItems(WatchItem *parent, const QVariantList &pro } } -void QmlEnginePrivate::handleEvaluate(int sequence, bool success, const QVariant &bodyVal) +void QmlEnginePrivate::handleExecuteDebuggerCommand(const QVariantMap &response) { - // { "seq" : , - // "type" : "response", - // "request_seq" : , - // "command" : "evaluate", - // "body" : ... - // "running" : - // "success" : true - // } - WatchHandler *watchHandler = engine->watchHandler(); - if (updateLocalsAndWatchers.contains(sequence)) { - updateLocalsAndWatchers.removeOne(sequence); - //Update the locals - foreach (int index, currentFrameScopes) - scope(index); - //Also update "this" - QByteArray iname("local.this"); - const WatchItem *parent = watchHandler->findItem(iname); - localsAndWatchers.insertMulti(parent->id, iname); - lookup(QList() << parent->id); - - } else if (debuggerCommands.contains(sequence)) { - updateLocalsAndWatchers.removeOne(sequence); - QmlV8ObjectData body = extractData(bodyVal); - if (auto consoleManager = ConsoleManagerInterface::instance()) { - if (ConsoleItem *item = constructLogItemTree(consoleManager->rootItem(), body)) - consoleManager->printToConsolePane(item); - } - //Update the locals - foreach (int index, currentFrameScopes) - scope(index); - - } else { - QmlV8ObjectData body = extractData(bodyVal); - if (evaluatingExpression.contains(sequence)) { - QString exp = evaluatingExpression.take(sequence); - //Do we have request to evaluate a local? - if (exp.startsWith(QLatin1String("local."))) { - WatchItem *item = watchHandler->findItem(exp.toLatin1()); - insertSubItems(item, body.properties); - } else { - QByteArray iname = watchHandler->watcherName(exp.toLatin1()); - SDEBUG(QString(iname)); - - auto item = new WatchItem(iname, exp); - item->exp = exp.toLatin1(); - item->id = body.handle; - if (success) { - item->type = body.type; - item->value = body.value.toString(); - item->wantsChildren = body.properties.count(); - } else { - //Do not set type since it is unknown - item->setError(body.value.toString()); - } - watchHandler->insertItem(item); - insertSubItems(item, body.properties); - } - //Insert the newly evaluated expression to the Watchers Window - } + QmlV8ObjectData body = extractData(response.value(_(BODY))); + if (auto consoleManager = ConsoleManagerInterface::instance()) { + if (ConsoleItem *item = constructLogItemTree(consoleManager->rootItem(), body)) + consoleManager->printToConsolePane(item); } + // Update the locals + foreach (int index, currentFrameScopes) + scope(index); } void QmlEnginePrivate::handleLookup(const QVariantMap &response) @@ -2480,33 +2376,27 @@ void QmlEnginePrivate::handleLookup(const QVariantMap &response) const QVariantMap body = response.value(_(BODY)).toMap(); QStringList handlesList = body.keys(); - WatchHandler *watchHandler = engine->watchHandler(); - foreach (const QString &handle, handlesList) { - QmlV8ObjectData bodyObjectData = extractData(body.value(handle)); - QByteArray prepend = localsAndWatchers.take(handle.toInt()); - - if (prepend.startsWith("local.") || prepend.startsWith("watch.")) { - // Data for expanded local/watch. - // Could be an object or function. - WatchItem *parent = watchHandler->findItem(prepend); - insertSubItems(parent, bodyObjectData.properties); - } else { - //rest + foreach (const QString &handleString, handlesList) { + int handle = handleString.toInt(); + QmlV8ObjectData bodyObjectData = extractData(body.value(handleString)); + QList vals = currentlyLookingUp.values(handle); + currentlyLookingUp.remove(handle); + foreach (const LookupData &res, vals) { auto item = new WatchItem; - item->exp = prepend; - item->name = QLatin1String(item->exp); - item->iname = QByteArray("local.") + item->exp; - item->id = handle.toInt(); + item->iname = res.iname; + item->name = res.name; + item->id = handle; item->type = bodyObjectData.type; item->value = bodyObjectData.value.toString(); item->setHasChildren(bodyObjectData.properties.count()); + insertSubItems(item, bodyObjectData.properties); engine->watchHandler()->insertItem(item); } } - engine->watchHandler()->notifyUpdateFinished(); + checkForFinishedUpdate(); } void QmlEnginePrivate::stateChanged(State state) diff --git a/src/plugins/debugger/qml/qmlengine.h b/src/plugins/debugger/qml/qmlengine.h index 2fe073aa498..bf51bf3ed42 100644 --- a/src/plugins/debugger/qml/qmlengine.h +++ b/src/plugins/debugger/qml/qmlengine.h @@ -77,8 +77,6 @@ private slots: void appStartupFailed(const QString &errorMessage); void appendMessage(const QString &msg, Utils::OutputFormat); - void synchronizeWatchers(); - private: void notifyEngineRemoteServerRunning(const QByteArray &, int pid); void notifyEngineRemoteSetupFinished(const RemoteSetupResult &result);