Bug 14759 – QtWebKit does not track link visited

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Adam Treat]

Attached is a patch that allows clients of QtWebKit to track the visited links and display them appropriately.  It does this by introducing an interface that clients can implement.

[Adam Treat]

Created attachment 15675 [details]
QWebHistoryInterface

Proposed patch that adds an interface for client apps to implement.

[Adam Treat]

Comment on attachment 15675 [details]
QWebHistoryInterface

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 050db5c..9c1f50a 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,12 @@
[phone]  Adam Treat  <[e-mail]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        Add an interface to manage global history for clients
+
+        * WebCore.pro:
+        * platform/qt/TemporaryLinkStubs.cpp:
+
 2007-07-24  Beth Dakin  <[e-mail]>
 
         Reviewed by Darin.
diff --git a/WebCore/WebCore.pro b/WebCore/WebCore.pro
index 4bd5a3b..c78a3cd 100644
--- a/WebCore/WebCore.pro
+++ b/WebCore/WebCore.pro
@@ -744,6 +744,7 @@ qt-port:HEADERS += \
     $$PWD/../WebKitQt/Api/qwebobjectplugin.h \
     $$PWD/../WebKitQt/Api/qwebobjectplugin_p.h \
     $$PWD/../WebKitQt/Api/qwebobjectpluginconnector.h \
+    $$PWD/../WebKitQt/Api/qwebhistoryinterface.h \
     $$PWD/../WebKitQt/Api/qcookiejar.h \
     $$PWD/../WebKitQt/WebCoreSupport/FrameLoaderClientQt.h
 
@@ -815,7 +816,8 @@ qt-port:SOURCES += \
     ../WebKitQt/Api/qwebpagehistory.cpp \
     ../WebKitQt/Api/qwebsettings.cpp \
     ../WebKitQt/Api/qwebobjectplugin.cpp \
-    ../WebKitQt/Api/qwebobjectpluginconnector.cpp 
+    ../WebKitQt/Api/qwebobjectpluginconnector.cpp \
+    ../WebKitQt/Api/qwebhistoryinterface.cpp
 
 gdk-port {
     HEADERS += \
diff --git a/WebCore/platform/qt/TemporaryLinkStubs.cpp b/WebCore/platform/qt/TemporaryLinkStubs.cpp
index 31f2fa2..afbfb6c 100644
--- a/WebCore/platform/qt/TemporaryLinkStubs.cpp
+++ b/WebCore/platform/qt/TemporaryLinkStubs.cpp
@@ -67,9 +67,6 @@
 
 using namespace WebCore;
 
-
-bool WebCore::historyContains(DeprecatedString const&) { return false; }
-
 void Frame::setNeedsReapplyStyles() { notImplemented(); }
 
 void FrameView::updateBorder() { notImplemented(); }
diff --git a/WebKitQt/Api/headers.pri b/WebKitQt/Api/headers.pri
index 6331384..846b793 100644
--- a/WebKitQt/Api/headers.pri
+++ b/WebKitQt/Api/headers.pri
@@ -6,4 +6,5 @@ WEBKIT_API_HEADERS = $$PWD/qcookiejar.h \
                      $$PWD/qwebobjectpluginconnector.h \
                      $$PWD/qwebpage.h \
                      $$PWD/qwebpagehistory.h \
-                     $$PWD/qwebsettings.h
+                     $$PWD/qwebsettings.h \
+                     $$PWD/qwebhistoryinterface.h
diff --git a/WebKitQt/Api/qwebhistoryinterface.cpp b/WebKitQt/Api/qwebhistoryinterface.cpp
new file mode 100644
index 0000000..47ae506
--- /dev/null
+++ b/WebKitQt/Api/qwebhistoryinterface.cpp
@@ -0,0 +1,83 @@
+/*
+  Copyright (C) 2007 Staikos Computing Services Inc.  <[e-mail]>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+  Boston, MA 02111-1307, USA.
+
+  This class provides all functionality needed tracking global history.
+*/
+
+#include "qwebhistoryinterface.h"
+
+#include <QCoreApplication>
+
+#include "KURL.h"
+
+namespace WebCore {
+
+bool historyContains(const DeprecatedString& s)
+{
+    if (!QWebHistoryInterface::defaultInterface())
+        return false;
+
+    QUrl qurl = QString(KURL(s).url());
+    return QWebHistoryInterface::defaultInterface()->historyContains(qurl);
+}
+
+} // namespace WebCore
+
+static QWebHistoryInterface *default_interface = 0;
+
+static bool gRoutineAdded = false;
+
+static void gCleanupInterface()
+{
+    delete default_interface;
+    default_interface = 0;
+}
+
+/*!
+  Sets a new default interface that will be used by all of WebKit
+  for managing history.
+*/
+void QWebHistoryInterface::setDefaultInterface(QWebHistoryInterface *defaultInterface)
+{
+    if (default_interface == defaultInterface)
+        return;
+    if (default_interface)
+        delete default_interface;
+    default_interface = defaultInterface;
+    if (!gRoutineAdded) {
+        qAddPostRoutine(gCleanupInterface);
+        gRoutineAdded = true;
+    }
+}
+
+/*!
+  Returns the default interface that will be used by WebKit. If no
+  default interface has been set, QtWebkit will not track history.
+*/
+QWebHistoryInterface *QWebHistoryInterface::defaultInterface()
+{
+    return default_interface;
+}
+
+QWebHistoryInterface::QWebHistoryInterface(QObject *parent) : QObject(parent)
+{
+}
+
+QWebHistoryInterface::~QWebHistoryInterface()
+{
+}
diff --git a/WebKitQt/Api/qwebhistoryinterface.h b/WebKitQt/Api/qwebhistoryinterface.h
new file mode 100644
index 0000000..56507f7
--- /dev/null
+++ b/WebKitQt/Api/qwebhistoryinterface.h
@@ -0,0 +1,43 @@
+/*
+  Copyright (C) 2007 Staikos Computing Services, Inc.  <[e-mail]>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+  Boston, MA 02111-1307, USA.
+
+  This class provides all functionality needed tracking global history.
+*/
+
+#ifndef QWEBHISTORYINTERFACE_H
+#define QWEBHISTORYINTERFACE_H
+
+#include <qobject.h>
+#include <qurl.h>
+
+#include <qwebkitglobal.h>
+
+class QWEBKIT_EXPORT QWebHistoryInterface : public QObject
+{
+    Q_OBJECT
+public:
+    QWebHistoryInterface(QObject *parent = 0);
+    ~QWebHistoryInterface();
+
+    static void setDefaultInterface(QWebHistoryInterface *defaultInterface);
+    static QWebHistoryInterface *defaultInterface();
+
+    virtual bool historyContains(const QUrl &url) const = 0;
+};
+
+#endif
diff --git a/WebKitQt/ChangeLog b/WebKitQt/ChangeLog
index a7883be..b7cb2d6 100644
--- a/WebKitQt/ChangeLog
+++ b/WebKitQt/ChangeLog
@@ -1,5 +1,20 @@
 2007-07-24  Adam Treat  <[e-mail]>
 
+        Reviewed by NOBODY (OOPS!).
+
+        Add an interface to manage global history for clients
+
+        * Api/headers.pri:
+        * Api/qwebhistoryinterface.cpp: Added.
+        (WebCore::historyContains):
+        (gCleanupInterface):
+        (QWebHistoryInterface::setDefaultInterface):
+        (QWebHistoryInterface::defaultInterface):
+        (QWebHistoryInterface::QWebHistoryInterface):
+        * Api/qwebhistoryinterface.h: Added.
+
[phone]  Adam Treat  <[e-mail]>
+
         Reviewed by Niko and Lars.
 
         These are no longer necessary or used.

[Adam Treat]

Comment on attachment 15675 [details]
QWebHistoryInterface

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 050db5c..9c1f50a 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,12 @@
[phone]  Adam Treat  <[e-mail]>
+
+        Reviewed by NOBODY (OOPS!).
+
+        Add an interface to manage global history for clients
+
+        * WebCore.pro:
+        * platform/qt/TemporaryLinkStubs.cpp:
+
 2007-07-24  Beth Dakin  <[e-mail]>
 
         Reviewed by Darin.
diff --git a/WebCore/WebCore.pro b/WebCore/WebCore.pro
index 4bd5a3b..c78a3cd 100644
--- a/WebCore/WebCore.pro
+++ b/WebCore/WebCore.pro
@@ -744,6 +744,7 @@ qt-port:HEADERS += \
     $$PWD/../WebKitQt/Api/qwebobjectplugin.h \
     $$PWD/../WebKitQt/Api/qwebobjectplugin_p.h \
     $$PWD/../WebKitQt/Api/qwebobjectpluginconnector.h \
+    $$PWD/../WebKitQt/Api/qwebhistoryinterface.h \
     $$PWD/../WebKitQt/Api/qcookiejar.h \
     $$PWD/../WebKitQt/WebCoreSupport/FrameLoaderClientQt.h
 
@@ -815,7 +816,8 @@ qt-port:SOURCES += \
     ../WebKitQt/Api/qwebpagehistory.cpp \
     ../WebKitQt/Api/qwebsettings.cpp \
     ../WebKitQt/Api/qwebobjectplugin.cpp \
-    ../WebKitQt/Api/qwebobjectpluginconnector.cpp 
+    ../WebKitQt/Api/qwebobjectpluginconnector.cpp \
+    ../WebKitQt/Api/qwebhistoryinterface.cpp
 
 gdk-port {
     HEADERS += \
diff --git a/WebCore/platform/qt/TemporaryLinkStubs.cpp b/WebCore/platform/qt/TemporaryLinkStubs.cpp
index 31f2fa2..afbfb6c 100644
--- a/WebCore/platform/qt/TemporaryLinkStubs.cpp
+++ b/WebCore/platform/qt/TemporaryLinkStubs.cpp
@@ -67,9 +67,6 @@
 
 using namespace WebCore;
 
-
-bool WebCore::historyContains(DeprecatedString const&) { return false; }
-
 void Frame::setNeedsReapplyStyles() { notImplemented(); }
 
 void FrameView::updateBorder() { notImplemented(); }
diff --git a/WebKitQt/Api/headers.pri b/WebKitQt/Api/headers.pri
index 6331384..846b793 100644
--- a/WebKitQt/Api/headers.pri
+++ b/WebKitQt/Api/headers.pri
@@ -6,4 +6,5 @@ WEBKIT_API_HEADERS = $$PWD/qcookiejar.h \
                      $$PWD/qwebobjectpluginconnector.h \
                      $$PWD/qwebpage.h \
                      $$PWD/qwebpagehistory.h \
-                     $$PWD/qwebsettings.h
+                     $$PWD/qwebsettings.h \
+                     $$PWD/qwebhistoryinterface.h
diff --git a/WebKitQt/Api/qwebhistoryinterface.cpp b/WebKitQt/Api/qwebhistoryinterface.cpp
new file mode 100644
index 0000000..47ae506
--- /dev/null
+++ b/WebKitQt/Api/qwebhistoryinterface.cpp
@@ -0,0 +1,83 @@
+/*
+  Copyright (C) 2007 Staikos Computing Services Inc.  <[e-mail]>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+  Boston, MA 02111-1307, USA.
+
+  This class provides all functionality needed tracking global history.
+*/
+
+#include "qwebhistoryinterface.h"
+
+#include <QCoreApplication>
+
+#include "KURL.h"
+
+namespace WebCore {
+
+bool historyContains(const DeprecatedString& s)
+{
+    if (!QWebHistoryInterface::defaultInterface())
+        return false;
+
+    QUrl qurl = QString(KURL(s).url());
+    return QWebHistoryInterface::defaultInterface()->historyContains(qurl);
+}
+
+} // namespace WebCore
+
+static QWebHistoryInterface *default_interface = 0;
+
+static bool gRoutineAdded = false;
+
+static void gCleanupInterface()
+{
+    delete default_interface;
+    default_interface = 0;
+}
+
+/*!
+  Sets a new default interface that will be used by all of WebKit
+  for managing history.
+*/
+void QWebHistoryInterface::setDefaultInterface(QWebHistoryInterface *defaultInterface)
+{
+    if (default_interface == defaultInterface)
+        return;
+    if (default_interface)
+        delete default_interface;
+    default_interface = defaultInterface;
+    if (!gRoutineAdded) {
+        qAddPostRoutine(gCleanupInterface);
+        gRoutineAdded = true;
+    }
+}
+
+/*!
+  Returns the default interface that will be used by WebKit. If no
+  default interface has been set, QtWebkit will not track history.
+*/
+QWebHistoryInterface *QWebHistoryInterface::defaultInterface()
+{
+    return default_interface;
+}
+
+QWebHistoryInterface::QWebHistoryInterface(QObject *parent) : QObject(parent)
+{
+}
+
+QWebHistoryInterface::~QWebHistoryInterface()
+{
+}
diff --git a/WebKitQt/Api/qwebhistoryinterface.h b/WebKitQt/Api/qwebhistoryinterface.h
new file mode 100644
index 0000000..56507f7
--- /dev/null
+++ b/WebKitQt/Api/qwebhistoryinterface.h
@@ -0,0 +1,43 @@
+/*
+  Copyright (C) 2007 Staikos Computing Services, Inc.  <[e-mail]>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+  Boston, MA 02111-1307, USA.
+
+  This class provides all functionality needed tracking global history.
+*/
+
+#ifndef QWEBHISTORYINTERFACE_H
+#define QWEBHISTORYINTERFACE_H
+
+#include <qobject.h>
+#include <qurl.h>
+
+#include <qwebkitglobal.h>
+
+class QWEBKIT_EXPORT QWebHistoryInterface : public QObject
+{
+    Q_OBJECT
+public:
+    QWebHistoryInterface(QObject *parent = 0);
+    ~QWebHistoryInterface();
+
+    static void setDefaultInterface(QWebHistoryInterface *defaultInterface);
+    static QWebHistoryInterface *defaultInterface();
+
+    virtual bool historyContains(const QUrl &url) const = 0;
+};
+
+#endif
diff --git a/WebKitQt/ChangeLog b/WebKitQt/ChangeLog
index a7883be..b7cb2d6 100644
--- a/WebKitQt/ChangeLog
+++ b/WebKitQt/ChangeLog
@@ -1,5 +1,20 @@
 2007-07-24  Adam Treat  <[e-mail]>
 
+        Reviewed by NOBODY (OOPS!).
+
+        Add an interface to manage global history for clients
+
+        * Api/headers.pri:
+        * Api/qwebhistoryinterface.cpp: Added.
+        (WebCore::historyContains):
+        (gCleanupInterface):
+        (QWebHistoryInterface::setDefaultInterface):
+        (QWebHistoryInterface::defaultInterface):
+        (QWebHistoryInterface::QWebHistoryInterface):
+        * Api/qwebhistoryinterface.h: Added.
+
[phone]  Adam Treat  <[e-mail]>
+
         Reviewed by Niko and Lars.
 
         These are no longer necessary or used.

[Adam Treat]

Created attachment 15676 [details]
QWebHistoryInterface

Correct comments.

[Simon Hausmann]

I have two suggestions:

+static QWebHistoryInterface *default_interface = 0;
+
+static bool gRoutineAdded = false;

The explicit initializations will make the variables end up in the data section instead of bss. I suggest to remove the intialization, they will still be initialized to zero (3.6.2).

+bool historyContains(const DeprecatedString& s)
+{
+    if (!QWebHistoryInterface::defaultInterface())
+        return false;
+
+    QUrl qurl = QString(KURL(s).url());
+    return QWebHistoryInterface::defaultInterface()->historyContains(qurl);
+}

I recall that in KDE we used to have a similar interface (HistoryProvider) and passing a url turned out to be a noticeable performance bottleneck as this function is called very often and it parses a URL every time. We changed it to use a plain QString as a result of that. Even though KURL does not have this problem anymore AFAIK we still have to convert it to a QUrl and it'll be parsed again. I think a plain const QString & is a safer choice here.


P.S.: Nice to see more git usage :)

[Adam Treat]

(In reply to comment #5)
> The explicit initializations will make the variables end up in the data section
> instead of bss. I suggest to remove the intialization, they will still be
> initialized to zero (3.6.2).

I can make the other changes, but I'm not sure what you mean here.  This follows from what qwebhistoryinterface does to create a singleton.

[Adam Treat]

Created attachment 15800 [details]
Revised patch after suggestions

Revised comments reflecting comments