Properly implement session support for QtWebKit-NG

See #1571

qutebrowser/browser/webkit/tabhistory.py

@@ -21,7 +21,7 @@
 
 
 from PyQt5.QtCore import QByteArray, QDataStream, QIODevice, QUrl
-from PyQt5.QtWebKit import qWebKitVersion  # FIXME can we guarantee WebKit is available here?
+from PyQt5.QtWebKit import qWebKitVersion
 
 from qutebrowser.utils import qtutils
 
@@ -32,20 +32,34 @@ def _encode_url(url):
     return data.decode('ascii')
 
 
-def _serialize_item_ng(i, item):
+def _serialize_ng(items, current_idx, stream):
+    # {'currentItemIndex': 0,
+    #  'history': [{'children': [],
+    #               'documentSequenceNumber': 1485030525573123,
+    #               'documentState': [],
+    #               'formContentType': '',
+    #               'itemSequenceNumber': 1485030525573122,
+    #               'originalURLString': 'about:blank',
+    #               'pageScaleFactor': 0.0,
+    #               'referrer': '',
+    #               'scrollPosition': {'x': 0, 'y': 0},
+    #               'target': '',
+    #               'title': '',
+    #               'urlString': 'about:blank'}]}
+    data = {'currentItemIndex': current_idx, 'history': []}
+    for item in items:
+        data['history'].append(_serialize_item_ng(item))
+
+    stream.writeInt(3)  # history stream version
+    stream.writeQVariantMap(data)
+
+
+def _serialize_item_ng(item):
     data = {
-        'children': [],
+        'originalURLString': item.original_url.toString(QUrl.FullyEncoded),
-        'documentSequenceNumber': i + 1,  # FIXME what to pass here?
-        'documentState': [],
-        'formContentType': '',
-        'itemSequenceNumber': i + 1,  # FIXME what to pass here?
-        'originalURLString': item.original_url.toString(),  # FIXME encoding?
-        'pageScaleFactor': 0.0,
-        'referrer': '',
         'scrollPosition': {'x': 0, 'y': 0},
-        'target': '',
         'title': item.title,
-        'urlString': item.url.toString(),  # FIXME encoding?
+        'urlString': item.url.toString(QUrl.FullyEncoded),
     }
     try:
         data['scrollPosition']['x'] = item.user_data['scroll-pos'].x()
@@ -55,6 +69,16 @@ def _serialize_item_ng(i, item):
     return data
 
 
+def _serialize_old(items, current_idx, stream):
+    ### Source/WebKit/qt/Api/qwebhistory.cpp operator<<
+    stream.writeInt(2)  # history stream version
+    stream.writeInt(len(items))
+    stream.writeInt(current_idx)
+
+    for i, item in enumerate(items):
+        _serialize_item_old(i, item, stream)
+
+
 def _serialize_item_old(i, item, stream):
     """Serialize a single WebHistoryItem into a QDataStream.
 
@@ -121,39 +145,6 @@ def _serialize_item_old(i, item, stream):
     stream.writeBool(False)
 
 
-def _serialize_old(items, current_idx, stream):
-    ### Source/WebKit/qt/Api/qwebhistory.cpp operator<<
-    stream.writeInt(2)  # history stream version
-    stream.writeInt(len(items))
-    stream.writeInt(current_idx)
-
-    for i, item in enumerate(items):
-        _serialize_item_old(i, item, stream)
-        user_data.append(item.user_data)
-
-
-def _serialize_ng(items, current_idx, stream):
-    # {'currentItemIndex': 0,
-    #  'history': [{'children': [],
-    #               'documentSequenceNumber': 1485030525573123,
-    #               'documentState': [],
-    #               'formContentType': '',
-    #               'itemSequenceNumber': 1485030525573122,
-    #               'originalURLString': 'about:blank',
-    #               'pageScaleFactor': 0.0,
-    #               'referrer': '',
-    #               'scrollPosition': {'x': 0, 'y': 0},
-    #               'target': '',
-    #               'title': '',
-    #               'urlString': 'about:blank'}]}
-    data = {'currentItemIndex': current_idx, 'history': []}
-    for i, item in enumerate(items):
-        data['history'].append(_serialize_item_ng(i, item))
-
-    stream.writeInt(3)  # history stream version
-    stream.writeQVariantMap(data)
-
-
 def serialize(items):
     """Serialize a list of QWebHistoryItems to a data stream.
 
@@ -190,13 +181,12 @@ def serialize(items):
     else:
         current_idx = 0
 
-    if qWebKitVersion() == '538.1':  # FIXME better comparison
+    if qtutils.is_qtwebkit_ng(qWebKitVersion()):
-        _serialize_old(items, current_idx, stream)
-    else:
         _serialize_ng(items, current_idx, stream)
+    else:
+        _serialize_old(items, current_idx, stream)
 
-    for i, item in enumerate(items):  # FIXME easier way?
+    user_data += [item.user_data for item in items]
-        user_data.append(item.user_data)
 
     stream.device().reset()
     qtutils.check_qdatastream(stream)

qutebrowser/utils/qtutils.py

@@ -91,6 +91,16 @@ def version_check(version, op=operator.ge):
               pkg_resources.parse_version(version))
 
 
+def is_qtwebkit_ng(version):
+    """Check if the given version is  QtWebKit-NG.
+
+    This is typically used as is_webkit_ng(qWebKitVersion) but we don't want to
+    have QtWebKit imports in here.
+    """
+    return (pkg_resources.parse_version(version) >
+            pkg_resources.parse_version('538.1'))
+
+
 def check_overflow(arg, ctype, fatal=True):
     """Check if the given argument is in bounds for the given type.
 

scripts/testbrowser.py

@@ -23,7 +23,7 @@
 import sys
 import argparse
 
-from PyQt5.QtCore import QUrl, QDataStream, QIODevice, QByteArray
+from PyQt5.QtCore import QUrl
 from PyQt5.QtWidgets import QApplication
 
 try:
@@ -49,15 +49,6 @@ def parse_args():
     return parser.parse_args()
 
 
-def dump_history(wv):
-    ba = QByteArray()
-    ds = QDataStream(ba, QIODevice.ReadWrite)
-    ds << wv.history()
-    ds2 = QDataStream(ba)
-    assert ds2.readInt() == 3
-    print(ds2.readQVariantMap())
-
-
 if __name__ == '__main__':
     args = parse_args()
     app = QApplication(sys.argv)
@@ -82,7 +73,6 @@ if __name__ == '__main__':
     wv.loadStarted.connect(lambda: print("Loading started"))
     wv.loadProgress.connect(lambda p: print("Loading progress: {}%".format(p)))
     wv.loadFinished.connect(lambda: print("Loading finished"))
-    wv.loadFinished.connect(lambda: dump_history(wv))
 
     if args.plugins and not using_webengine:
         from PyQt5.QtWebKit import QWebSettings

tests/unit/utils/test_qtutils.py

@@ -64,6 +64,15 @@ def test_version_check(monkeypatch, qversion, version, op, expected):
     assert qtutils.version_check(version, op) == expected
 
 
+@pytest.mark.parametrize('version, ng', [
+    ('537.21', False),  # QtWebKit 5.1
+    ('538.1', False),   # Qt 5.8
+    ('602.1', True)     # QtWebKit-NG TP5
+])
+def test_is_qtwebkit_ng(version, ng):
+    assert qtutils.is_qtwebkit_ng(version) == ng
+
+
 class TestCheckOverflow:
 
     """Test check_overflow."""