Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix specs on Qt 4.8 #376

Closed
wants to merge 2 commits into from

3 participants

@fxposter

There are some differences in capybara_webkit and selenium drivers for
now.

Also, selenium-webdriver was updated in Gemfile.lock.

jferris and others added some commits
@jferris jferris wip: Fix issues with Qt 4.8 7aa6ff7
@fxposter fxposter Fix specs on Qt 4.8
There are some differences in capybara_webkit and selenium drivers for
now
de925e5
@mhoran
Collaborator

There are some subtle differences between load and setUrl, and I don't understand enough to know whether there's any down side to use setUrl.

setUrl does have the benefit of setting the URL on the frame immediately, which fixes the failing specs. However, load also sets the URL of the frame, but elsewhere in the call stack. It seems that in the case of load, the loadFinished signal fires before the URL gets set.

Either way, upon merging this change, webkit_server segfaults for me at a seemingly random point in the spec run.

Yes, everything you say is true. And yes, it sometimes segfaults. I haven't dug into it yet.

Collaborator

Ah, I remember why setUrl is not a good solution. Check out this test: http://git.io/2DGC7Q.

Using setUrl will indeed set the URL on the QWebFrame, however when we get into WebPage::networkAccessManagerFinishedReply, currentFrame()->url() points to the requested URL, not the resolved URL. This means that the status code and headers never get set, as the test points out.

@jferris
Admin

I've started working on some of this in the jf-4-8 branch.

The segfaults are caused by a race condition; in Qt 4.8, all HTTP requests are handled in the background thread, which means any data that makes it into the HTTP layer must now be immutable or otherwise threadsafe. The segfault comes from trying to access a deleted string or deleting an already deleted string depending on which thread finishes first, but I haven't been able to track down which string is getting in there yet.

I think there may be some fixes in my branch related to the URL and UnsupportedContent issues, so that may be worth looking into.

@mhoran
Collaborator

@jferris, your fixes do seem to resolve the URL issues for me. For the record, it seems that the behavior of QWebFrame::requestedUrl() was changed from the previous behavior which included pushState/replaceState changes to instead returning only the requested URL, which makes sense.

This leaves us with the segfault issue, which I've not been able to track down, and the status code/header issue partially addressed by @fxposter.

The status code and headers issues seem to be coming from WebPage::networkAccessManagerFinishedReply(). This method handles setting the status code and headers for the WebPage. The code determines whether or not to set the status code and headers based on the URL of the incoming HTTP response and the URL of the currently focused frame. Somewhere between Qt 4.7 and 4.8, the behavior of the WebPage::finished() signal was changed such that it is fired before the URL of the focused frame is set, causing this issue. I'd opened an upstream bug for this, but haven't heard back (see #315).

@fxposter

@jferris This pull request is based on your jf-4-8 branch.

@jferris
Admin

@fxposter sorry, I missed that. I agree with @mhoran on the setUrl changes. What spec were you fixing by adding the renderNonHtmlContent call there?

@mhoran
Collaborator

Using valgrind I was able to trace down at least one race condition. It looks like in some cases reset is getting processed after another command has already started. This results in the WebPage which is currently executing an invokeCapybaraFunction() trying to respond to a deleted WebPage. I was able to work around this by commenting out m_pages->first()->deleteLater() in WebPageManager::reset().

Unfortunately once I got past this isuse, I ran into another very cryptic segfault. valgrind wasn't much help this time around.

For reference, here's the backtrace before commenting out deleteLater(): https://gist.github.com/3444902. Commenting out deleteLater() reliably gets me past invokeCapybaraFunction(), but instead blows up elsewhere: https://gist.github.com/3444924.

@mhoran
Collaborator

It looks like the source of our problems is documented here: http://goo.gl/5Fs3S. Specifically,

This signal is emitted when the page finishes loading content. This signal is independant of script execution or page rendering. ok will indicate whether the load was successful or any error occurred.

Putting some debugging in place, I found that the WebPage::pageFinished() signal is fired by the WebPage before WebPage::injectJavascriptHelpers() is called. If the browser receives reset! before the JavaScript helpers have loaded, we get a segmentation fault.

Many thanks to @huuf for providing a second pair of eyes in debugging.

@jferris
Admin

@mhoran Excellent find. To make sure I understand correctly, in Qt 4.8, the following sequence can happen and results in a segfault: pageFinished is fired, a reset command is sent, the pages are deleted, and then a deleted frame gets the signal javascriptWindowObjectCleared?

Is it possible that this is happening because there's an iframe on the page that gets loaded later, and it's the iframe's window that's getting the javascriptWindowObjectCleared signal after the main frame is finished loading? Does this ever happen when there's only one frame on the page?

@mhoran
Collaborator

@jferris, that looks to be the case.

I don't think the bug is limited to iframes, unfortunately. Every WebPage has a main frame that the JavaScript helpers need to be injected into. However, from further investigation it looks like the JavaScript Window object doesn't always get cleared. I tried adding a makeshift mutex around the frame->evaluateJavaScript call in WebPage::InjectJavascriptHelpers, but this just caused the test suite to hang.

@jferris
Admin

@mhoran I think we're running into different issues.

Here's the backtrace I get pretty reliably:

Received "Visit" 
Started "Visit" 
Load started 
"Visit" started page load 
Started request to "http://127.0.0.1:53053/" 
Finished "Visit" with response "Success()" 

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x000000010c5b70fe

#0  0x00007fff8897e6c0 in malloc_error_break ()
#1  0x00007fff8897e805 in free ()
#2  0x000000010315bd3e in QString::free ()
#3  0x00000001031c2c29 in QUrlPrivate::authority ()
#4  0x00000001031ca002 in QUrlPrivate::toEncoded ()
#5  0x00000001031cb0a4 in QUrlPrivate::validate ()
#6  0x00000001031cb305 in QUrl::port ()
#7  0x0000000102faaedb in QHttpThreadDelegate::startRequest ()
#8  0x0000000103012c1b in QHttpThreadDelegate::qt_static_metacall ()
#9  0x000000010322f861 in QObject::event ()
#10 0x00000001022fbfad in QApplicationPrivate::notify_helper ()
#11 0x0000000102302fe4 in QApplication::notify ()
#12 0x000000010321b75c in QCoreApplication::notifyInternal ()
#13 0x000000010321d380 in QCoreApplicationPrivate::sendPostedEvents ()
#14 0x0000000103250832 in QEventDispatcherUNIX::processEvents ()
#15 0x000000010321a664 in QEventLoop::processEvents ()
#16 0x000000010321aa14 in QEventLoop::exec ()
#17 0x0000000103105198 in QThread::exec ()
#18 0x00000001031085aa in QThreadPrivate::start ()
#19 0x00007fff8892c8bf in _pthread_start ()
#20 0x00007fff8892fb75 in thread_start ()

It always happened directly after finishing a Visit command, and it's always in QString::free with QHttpThreadDelegate further up the trace.

Removing the m_pages->first()->deleteLater() call causes the driver to hang for me at random specs; I have no idea why.

I haven't been able to reproduce the same backtrace you've been getting. I'm going to try updating Qt (I'm on 4.8.0) to see if anything changes.

@jferris
Admin

I get a similar trace in Qt 4.8.1:


#0  0x00007fff8897e6c0 in malloc_error_break ()
#1  0x00007fff8897e805 in free ()
#2  0x0000000102d7cf68 in QString::free ()
#3  0x0000000102d7d1d9 in QString::realloc ()
#4  0x0000000102ded9e9 in operator+=<QLatin1Char, QString> ()
#5  0x0000000102de48cd in QUrlPrivate::authority ()
#6  0x0000000102dea0f2 in QUrlPrivate::toEncoded ()
#7  0x0000000102deb194 in QUrlPrivate::validate ()
#8  0x0000000102deb3f5 in QUrl::port ()
#9  0x00000001030d37db in QHttpThreadDelegate::startRequest ()
#10 0x000000010313cb2b in QHttpThreadDelegate::qt_static_metacall ()
#11 0x0000000102e528e1 in QObject::event ()
#12 0x000000010209f93d in QApplicationPrivate::notify_helper ()
#13 0x00000001020a5dc4 in QApplication::notify ()
#14 0x0000000102e3e17c in QCoreApplication::notifyInternal ()
#15 0x0000000102e3f5a0 in QCoreApplicationPrivate::sendPostedEvents ()
#16 0x0000000102e73502 in QEventDispatcherUNIX::processEvents ()
#17 0x0000000102e3d094 in QEventLoop::processEvents ()
#18 0x0000000102e3d444 in QEventLoop::exec ()
#19 0x0000000102d25f08 in QThread::exec ()
#20 0x0000000102d295ba in QThreadPrivate::start ()
#21 0x00007fff8892c8bf in _pthread_start ()
#22 0x00007fff8892fb75 in thread_start ()
@mhoran
Collaborator

I've seen this one as well when running the entire test suite. However, when I run just ./spec/integration/driver_spec.rb under valgrind I get the injectJavascriptHelpers segfault regularly. I think valgrind slows down the app enough such that the deleteLater runs and we get the segfault. Under normal circumstances, it's even more difficult to reproduce these issues.

@mhoran
Collaborator

@jferris, the injectJavascriptHelpers segfaults seem only reproducible in valgrind, though they may be related to some of the other issues. I managed to come across three different segfaults this weekend running the malloc test outside of valgrind. The first traced back to WebPageManager::requestCreated. It seems that sometimes the QNetworkReply is NULL, so the logger blows up trying to read the url(). See the backtrace here: https://gist.github.com/3615538.

I attempted to mitigate this issue by disconnecting the signals from each page in WebPageManager::reset(), but that didn't seem to help.

I didn't have much luck analyzing the backtraces from the other segfaults: https://gist.github.com/3615583 and https://gist.github.com/3615579.

I ran these tests against Qt 4.8.2, and saw similar issues with 4.8.0.

@jferris
Admin

@mhoran cool, that's consistent with what I've been seeing. I think we're getting a call to requestCreated after the request has already been deleted. I'm not sure why yet. I'll let you know if I find anything else out.

This was referenced
@mhoran
Collaborator

We've addressed the segfaults in #403 and the test suite is passing.

@mhoran mhoran closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 12, 2012
  1. @jferris

    wip: Fix issues with Qt 4.8

    jferris authored
Commits on Aug 17, 2012
  1. @fxposter

    Fix specs on Qt 4.8

    fxposter authored
    There are some differences in capybara_webkit and selenium drivers for
    now
This page is out of date. Refresh to see the latest.
View
20 Gemfile.lock
@@ -8,6 +8,7 @@ PATH
GEM
remote: http://rubygems.org/
specs:
+ addressable (2.3.2)
appraisal (0.4.0)
bundler
rake
@@ -18,16 +19,19 @@ GEM
rack-test (>= 0.5.4)
selenium-webdriver (~> 2.0)
xpath (~> 0.1.4)
- childprocess (0.3.1)
- ffi (~> 1.0.6)
+ childprocess (0.3.5)
+ ffi (~> 1.0, >= 1.0.6)
diff-lcs (1.1.2)
- ffi (1.0.11)
+ ffi (1.1.5)
json (1.7.1)
+ libwebsocket (0.1.5)
+ addressable
mime-types (1.18)
mini_magick (3.2.1)
subexec (~> 0.0.4)
- multi_json (1.0.4)
+ multi_json (1.3.6)
nokogiri (1.5.0)
+ nokogiri (1.5.0-x86-mingw32)
rack (1.3.2)
rack-test (0.6.1)
rack (>= 1.0)
@@ -40,11 +44,11 @@ GEM
rspec-expectations (2.6.0)
diff-lcs (~> 1.1.2)
rspec-mocks (2.6.0)
- rubyzip (0.9.7)
- selenium-webdriver (2.19.0)
+ rubyzip (0.9.9)
+ selenium-webdriver (2.25.0)
childprocess (>= 0.2.5)
- ffi (~> 1.0.9)
- multi_json (~> 1.0.4)
+ libwebsocket (~> 0.1.3)
+ multi_json (~> 1.0)
rubyzip
sinatra (1.1.2)
rack (~> 1.1)
View
2  spec/browser_spec.rb
@@ -236,7 +236,7 @@
it 'uses URLs changed by javascript' do
browser.execute_script "window.history.pushState('', '', '/blah')"
- browser.requested_url.should == 'http://example.org/blah'
+ browser.current_url.should == 'http://example.org/blah'
end
it 'is possible to disable proxy again' do
View
8 spec/driver_spec.rb
@@ -239,7 +239,7 @@ def in_iframe_request?
it "has a blank location after reseting" do
driver.reset!
- driver.current_url.should == ""
+ driver.current_url.should == "about:blank"
end
it "raises an error for an invalid xpath query" do
@@ -395,7 +395,11 @@ def in_iframe_request?
before { driver.visit("/") }
it "collects messages logged to the console" do
- driver.console_messages.first.should include :source, :message => "hello", :line_number => 6
+ url = driver_url(driver, "/")
+ message = driver.console_messages.first
+ message.should include :source => url, :message => "hello"
+ # QtWebKit returns different line numbers depending on the version
+ [5, 6].should include(message[:line_number])
driver.console_messages.length.should eq 3
end
View
64 src/CurrentUrl.cpp
@@ -5,65 +5,9 @@
CurrentUrl::CurrentUrl(WebPageManager *manager, QStringList &arguments, QObject *parent) : SocketCommand(manager, arguments, parent) {
}
-/*
- * This CurrentUrl command attempts to produce a current_url value consistent
- * with that returned by the Selenium WebDriver Capybara driver.
- *
- * It does not currently return the correct value in the case of an iframe whose
- * source URL results in a redirect because the loading of the iframe does not
- * generate a history item. This is most likely a rare case and is consistent
- * with the current behavior of the capybara-webkit driver.
- *
- * The following two values are *not* affected by Javascript pushState.
- *
- * QWebFrame->url()
- * QWebHistoryItem.originalUrl()
- *
- * The following two values *are* affected by Javascript pushState.
- *
- * QWebFrame->requestedUrl()
- * QWebHistoryItem.url()
- *
- * In the cases that we have access to both the QWebFrame values and the
- * correct history item for that frame, we can compare the values and determine
- * if a redirect occurred and if pushState was used. The table below describes
- * the various combinations of URL values that are possible.
- *
- * O -> originally requested URL
- * R -> URL after redirection
- * P -> URL set by pushState
- * * -> denotes the desired URL value from the frame
- *
- * frame history
- * case url requestedUrl url originalUrl
- * -----------------------------------------------------------------
- * regular load O O* O O
- *
- * redirect w/o R* O R O
- * pushState
- *
- * pushState O P* P O
- * only
- *
- * redirect w/ R P* P O
- * pushState
- *
- * Based on the above information, we only need to check for the case of a
- * redirect w/o pushState, in which case QWebFrame->url() will have the correct
- * current_url value. In all other cases QWebFrame->requestedUrl() is correct.
- */
void CurrentUrl::start() {
- QUrl humanUrl = wasRedirectedAndNotModifiedByJavascript() ?
- page()->currentFrame()->url() : page()->currentFrame()->requestedUrl();
- QByteArray encodedBytes = humanUrl.toEncoded();
- emit finished(new Response(true, encodedBytes));
+ QStringList arguments;
+ QVariant result = page()->invokeCapybaraFunction("currentUrl", arguments);
+ QString url = result.toString();
+ emit finished(new Response(true, url));
}
-
-bool CurrentUrl::wasRegularLoad() {
- return page()->currentFrame()->url() == page()->currentFrame()->requestedUrl();
-}
-
-bool CurrentUrl::wasRedirectedAndNotModifiedByJavascript() {
- return !wasRegularLoad() && page()->currentFrame()->url() == page()->history()->currentItem().url();
-}
-
View
4 src/CurrentUrl.h
@@ -6,9 +6,5 @@ class CurrentUrl : public SocketCommand {
public:
CurrentUrl(WebPageManager *, QStringList &arguments, QObject *parent = 0);
virtual void start();
-
- private:
- bool wasRegularLoad();
- bool wasRedirectedAndNotModifiedByJavascript();
};
View
2  src/Source.cpp
@@ -7,7 +7,7 @@ Source::Source(WebPageManager *manager, QStringList &arguments, QObject *parent)
void Source::start() {
QNetworkAccessManager* accessManager = page()->networkAccessManager();
- QNetworkRequest request(page()->currentFrame()->url());
+ QNetworkRequest request(page()->currentFrame()->requestedUrl());
reply = accessManager->get(request);
connect(reply, SIGNAL(finished()), this, SLOT(sourceLoaded()));
View
23 src/UnsupportedContentHandler.cpp
@@ -5,23 +5,22 @@
UnsupportedContentHandler::UnsupportedContentHandler(WebPage *page, QNetworkReply *reply, QObject *parent) : QObject(parent) {
m_page = page;
m_reply = reply;
- connect(m_reply, SIGNAL(finished()), this, SLOT(handleUnsupportedContent()));
- disconnect(m_page, SIGNAL(loadFinished(bool)), m_page, SLOT(loadFinished(bool)));
}
-void UnsupportedContentHandler::handleUnsupportedContent() {
- this->renderNonHtmlContent();
- this->finish();
+void UnsupportedContentHandler::renderNonHtmlContent() {
+ QByteArray text = m_reply->readAll();
+ m_page->mainFrame()->setContent(text, QString("text/plain"), m_reply->url());
+ m_page->networkAccessManagerFinishedReply(m_reply);
+ m_page->loadFinished(true);
this->deleteLater();
}
-void UnsupportedContentHandler::renderNonHtmlContent() {
- QByteArray text = m_reply->readAll();
- m_page->mainFrame()->setContent(text, QString("text/plain"), m_reply->url());
+void UnsupportedContentHandler::waitForReplyToFinish() {
+ connect(m_reply, SIGNAL(finished()), this, SLOT(replyFinished()));
+ disconnect(m_page, SIGNAL(loadFinished(bool)), m_page, SLOT(loadFinished(bool)));
}
-void UnsupportedContentHandler::finish() {
- connect(m_page, SIGNAL(loadFinished(bool)), m_page, SLOT(loadFinished(bool)));
- m_page->networkAccessManagerFinishedReply(m_reply);
- m_page->loadFinished(true);
+void UnsupportedContentHandler::replyFinished() {
+ connect(m_page, SIGNAL(loadFinished(bool)), m_page, SLOT(loadFinished(bool)));
+ renderNonHtmlContent();
}
View
8 src/UnsupportedContentHandler.h
@@ -1,18 +1,20 @@
#include <QObject>
+
class WebPage;
class QNetworkReply;
+
class UnsupportedContentHandler : public QObject {
Q_OBJECT
public:
UnsupportedContentHandler(WebPage *page, QNetworkReply *reply, QObject *parent = 0);
+ void waitForReplyToFinish();
+ void renderNonHtmlContent();
public slots:
- void handleUnsupportedContent();
+ void replyFinished();
private:
WebPage *m_page;
QNetworkReply *m_reply;
- void renderNonHtmlContent();
- void finish();
};
View
2  src/Visit.cpp
@@ -8,6 +8,6 @@ Visit::Visit(WebPageManager *manager, QStringList &arguments, QObject *parent) :
void Visit::start() {
QUrl requestedUrl = QUrl::fromEncoded(arguments()[0].toUtf8(), QUrl::StrictMode);
- page()->currentFrame()->load(QUrl(requestedUrl));
+ page()->currentFrame()->setUrl(QUrl(requestedUrl));
emit finished(new Response(true));
}
View
5 src/WebPage.cpp
@@ -256,7 +256,10 @@ void WebPage::handleUnsupportedContent(QNetworkReply *reply) {
if(!contentMimeType.isNull()) {
triggerAction(QWebPage::Stop);
UnsupportedContentHandler *handler = new UnsupportedContentHandler(this, reply);
- Q_UNUSED(handler);
+ if (reply->isFinished())
+ handler->renderNonHtmlContent();
+ else
+ handler->waitForReplyToFinish();
}
}
View
4 src/capybara.js
@@ -11,6 +11,10 @@ Capybara = {
return this.findRelativeTo(document, xpath);
},
+ currentUrl: function () {
+ return window.location.toString();
+ },
+
findWithin: function (index, xpath) {
return this.findRelativeTo(this.nodes[index], xpath);
},
Something went wrong with that request. Please try again.