From 9e802e3af1e21ecacc607daf9716f690fe0a73bf Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@digia.com>
Date: Thu, 3 Apr 2014 11:15:56 +0200
Subject: [PATCH] QmlProfiler: Reduce the amount of useless signals on item
 selection

By passing signals with identification information of varying accurary
between the different views the event being selected could be changed
while it was selected. By checking the current selection against the
information given in the signal and not reselecting when it matches the
situation is improved.

Also, the selection methods are given more appropriate names. We hardly
ever want to select the "next" event, but rather the "best fitting" one.

Task-number: QTCREATORBUG-11945
Change-Id: I659b4929cb88f4c931a0893aa95a3bc92da5a23b
Reviewed-by: Kai Koehne <kai.koehne@digia.com>
---
 src/plugins/qmlprofiler/qml/MainView.qml      | 12 +++++-------
 .../qmlprofiler/qmlprofilereventview.cpp      | 19 +++++++++++++------
 .../qmlprofiler/qmlprofilereventview.h        |  1 +
 .../qmlprofiler/qmlprofilertraceview.cpp      | 11 +++++------
 .../qmlprofiler/qmlprofilertraceview.h        |  4 ++--
 .../qmlprofiler/qmlprofilerviewmanager.cpp    |  6 +++---
 src/plugins/qmlprofiler/timelinerenderer.cpp  |  6 +++++-
 7 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/src/plugins/qmlprofiler/qml/MainView.qml b/src/plugins/qmlprofiler/qml/MainView.qml
index 3455e262a56..ad4f364c664 100644
--- a/src/plugins/qmlprofiler/qml/MainView.qml
+++ b/src/plugins/qmlprofiler/qml/MainView.qml
@@ -162,14 +162,12 @@ Rectangle {
         rangeDetails.isBindingLoop = false;
     }
 
-    function selectNextByHash(hash) {
-        var eventId = qmlProfilerModelProxy.getEventIdForHash(hash);
-        if (eventId !== -1)
-            selectNextById(eventId);
-    }
-
-    function selectNextById(eventId)
+    function selectById(eventId)
     {
+        if (eventId === -1 ||
+                eventId === qmlProfilerModelProxy.getEventId(view.selectedModel, view.selectedItem))
+            return;
+
         // this is a slot responding to events from the other pane
         // which tracks only events from the basic model
         if (!lockItemSelection) {
diff --git a/src/plugins/qmlprofiler/qmlprofilereventview.cpp b/src/plugins/qmlprofiler/qmlprofilereventview.cpp
index 7fdbc614584..ddf2365e031 100644
--- a/src/plugins/qmlprofiler/qmlprofilereventview.cpp
+++ b/src/plugins/qmlprofiler/qmlprofilereventview.cpp
@@ -699,13 +699,22 @@ void QmlProfilerEventsMainView::jumpToItem(const QModelIndex &index)
     d->m_preventSelectBounce = false;
 }
 
+void QmlProfilerEventsMainView::selectItem(const QStandardItem *item)
+{
+    // If the same item is already selected, don't reselect it.
+    QModelIndex index = d->m_model->indexFromItem(item);
+    if (index != currentIndex()) {
+        setCurrentIndex(index);
+        jumpToItem(index);
+    }
+}
+
 void QmlProfilerEventsMainView::selectEvent(const QString &eventHash)
 {
     for (int i=0; i<d->m_model->rowCount(); i++) {
         QStandardItem *infoItem = d->m_model->item(i, 0);
         if (infoItem->data(EventHashStrRole).toString() == eventHash) {
-            setCurrentIndex(d->m_model->indexFromItem(infoItem));
-            jumpToItem(currentIndex());
+            selectItem(infoItem);
             return;
         }
     }
@@ -718,13 +727,11 @@ void QmlProfilerEventsMainView::selectEventByLocation(const QString &filename, i
 
     for (int i=0; i<d->m_model->rowCount(); i++) {
         QStandardItem *infoItem = d->m_model->item(i, 0);
-        if (currentIndex() != d->m_model->indexFromItem(infoItem) &&
-                infoItem->data(FilenameRole).toString() == filename &&
+        if (infoItem->data(FilenameRole).toString() == filename &&
                 infoItem->data(LineRole).toInt() == line &&
                 (column == -1 ||
                 infoItem->data(ColumnRole).toInt() == column)) {
-            setCurrentIndex(d->m_model->indexFromItem(infoItem));
-            jumpToItem(currentIndex());
+            selectItem(infoItem);
             return;
         }
     }
diff --git a/src/plugins/qmlprofiler/qmlprofilereventview.h b/src/plugins/qmlprofiler/qmlprofilereventview.h
index e7f9eb5097c..492fdbf4f4e 100644
--- a/src/plugins/qmlprofiler/qmlprofilereventview.h
+++ b/src/plugins/qmlprofiler/qmlprofilereventview.h
@@ -143,6 +143,7 @@ private slots:
     void profilerDataModelStateChanged();
 
 private:
+    void selectItem(const QStandardItem *item);
     void setHeaderLabels();
     void parseModelProxy();
 
diff --git a/src/plugins/qmlprofiler/qmlprofilertraceview.cpp b/src/plugins/qmlprofiler/qmlprofilertraceview.cpp
index 6d7fa6a5944..91428fe7e90 100644
--- a/src/plugins/qmlprofiler/qmlprofilertraceview.cpp
+++ b/src/plugins/qmlprofiler/qmlprofilertraceview.cpp
@@ -371,24 +371,23 @@ void QmlProfilerTraceView::clear()
     QMetaObject::invokeMethod(d->m_timebar->rootObject(), "clear");
 }
 
-void QmlProfilerTraceView::selectNextEventByHash(const QString &hash)
+void QmlProfilerTraceView::selectByHash(const QString &hash)
 {
     QQuickItem *rootObject = d->m_mainView->rootObject();
 
     if (rootObject)
-        QMetaObject::invokeMethod(rootObject, "selectNextByHash",
-                                  Q_ARG(QVariant,QVariant(hash)));
+        QMetaObject::invokeMethod(rootObject, "selectById",
+                Q_ARG(QVariant,QVariant(d->m_modelProxy->getEventIdForHash(hash))));
 }
 
-void QmlProfilerTraceView::selectNextEventByLocation(const QString &filename, const int line, const int column)
+void QmlProfilerTraceView::selectBySourceLocation(const QString &filename, int line, int column)
 {
     int eventId = d->m_modelProxy->getEventIdForLocation(filename, line, column);
 
     if (eventId != -1) {
         QQuickItem *rootObject = d->m_mainView->rootObject();
         if (rootObject)
-            QMetaObject::invokeMethod(rootObject, "selectNextById",
-                                      Q_ARG(QVariant,QVariant(eventId)));
+            QMetaObject::invokeMethod(rootObject, "selectById", Q_ARG(QVariant,QVariant(eventId)));
     }
 }
 
diff --git a/src/plugins/qmlprofiler/qmlprofilertraceview.h b/src/plugins/qmlprofiler/qmlprofilertraceview.h
index 2203e643474..e5991020798 100644
--- a/src/plugins/qmlprofiler/qmlprofilertraceview.h
+++ b/src/plugins/qmlprofiler/qmlprofilertraceview.h
@@ -100,8 +100,8 @@ public:
 
 public slots:
     void clear();
-    void selectNextEventByHash(const QString &eventHash);
-    void selectNextEventByLocation(const QString &filename, const int line, const int column);
+    void selectByHash(const QString &eventHash);
+    void selectBySourceLocation(const QString &filename, int line, int column);
 
 private slots:
     void updateCursorPosition();
diff --git a/src/plugins/qmlprofiler/qmlprofilerviewmanager.cpp b/src/plugins/qmlprofiler/qmlprofilerviewmanager.cpp
index 0f937b76911..a81528747b7 100644
--- a/src/plugins/qmlprofiler/qmlprofilerviewmanager.cpp
+++ b/src/plugins/qmlprofiler/qmlprofilerviewmanager.cpp
@@ -105,8 +105,8 @@ void QmlProfilerViewManager::createViews()
                                                 d->profilerModelManager);
     connect(d->eventsView, SIGNAL(gotoSourceLocation(QString,int,int)), this,
             SIGNAL(gotoSourceLocation(QString,int,int)));
-    connect(d->eventsView, SIGNAL(eventSelectedByHash(QString)), d->traceView,
-            SLOT(selectNextEventByHash(QString)));
+    connect(d->eventsView, SIGNAL(eventSelectedByHash(QString)),
+            d->traceView, SLOT(selectByHash(QString)));
     connect(d->traceView, SIGNAL(gotoSourceLocation(QString,int,int)),
             d->eventsView, SLOT(selectBySourceLocation(QString,int,int)));
 
@@ -117,7 +117,7 @@ void QmlProfilerViewManager::createViews()
     connect(d->traceView, SIGNAL(gotoSourceLocation(QString,int,int)),
             d->v8profilerView, SLOT(selectBySourceLocation(QString,int,int)));
     connect(d->v8profilerView, SIGNAL(gotoSourceLocation(QString,int,int)),
-            d->traceView, SLOT(selectNextEventByLocation(QString,int,int)));
+            d->traceView, SLOT(selectBySourceLocation(QString,int,int)));
     connect(d->v8profilerView, SIGNAL(gotoSourceLocation(QString,int,int)),
             d->eventsView, SLOT(selectBySourceLocation(QString,int,int)));
     connect(d->eventsView, SIGNAL(gotoSourceLocation(QString,int,int)),
diff --git a/src/plugins/qmlprofiler/timelinerenderer.cpp b/src/plugins/qmlprofiler/timelinerenderer.cpp
index 4b797ed8ade..356761a3c2e 100644
--- a/src/plugins/qmlprofiler/timelinerenderer.cpp
+++ b/src/plugins/qmlprofiler/timelinerenderer.cpp
@@ -309,11 +309,15 @@ void TimelineRenderer::manageClicked()
             setSelectionLocked(!m_selectionLocked);
         else
             setSelectionLocked(true);
+
+        // itemPressed() will trigger an update of the events and JavaScript views. Make sure the
+        // correct event is already selected when that happens, to prevent confusion.
+        selectFromId(m_currentSelection.modelIndex, m_currentSelection.eventIndex);
         emit itemPressed(m_currentSelection.modelIndex, m_currentSelection.eventIndex);
     } else {
         setSelectionLocked(false);
+        selectFromId(m_currentSelection.modelIndex, m_currentSelection.eventIndex);
     }
-    selectFromId(m_currentSelection.modelIndex, m_currentSelection.eventIndex);
 }
 
 void TimelineRenderer::manageHovered(int mouseX, int mouseY)
-- 
GitLab