Skip to content
This repository
Browse code

Fix memory leak of response.

Turns Response into a QObject and sets parent to the
command that emits it.

Each Command is also a child of the decorator commands,
Timeout and PageLoading commnds, so that deleting the
top level command will delete all the children.

See discussion in #430.
  • Loading branch information...
commit 77811ca9bae7754c11e11a00e8894f43d2e0f366 1 parent 1cc1428
Sean Geoghegan seangeo authored mhoran committed

Showing 44 changed files with 83 additions and 59 deletions. Show diff stats Hide diff stats

  1. +1 1  src/Authenticate.cpp
  2. +1 1  src/ClearCookies.cpp
  3. +1 1  src/ClearPromptText.cpp
  4. +9 0 src/Command.cpp
  5. +5 0 src/Command.h
  6. +4 3 src/Connection.cpp
  7. +1 1  src/ConsoleMessages.cpp
  8. +1 1  src/CurrentUrl.cpp
  9. +1 1  src/EnableLogging.cpp
  10. +1 1  src/Evaluate.cpp
  11. +2 2 src/Execute.cpp
  12. +2 2 src/Find.cpp
  13. +3 3 src/FrameFocus.cpp
  14. +1 1  src/GetCookies.cpp
  15. +1 1  src/GetTimeout.cpp
  16. +1 1  src/GetWindowHandle.cpp
  17. +1 1  src/GetWindowHandles.cpp
  18. +1 1  src/Header.cpp
  19. +1 1  src/Headers.cpp
  20. +1 1  src/IgnoreSslErrors.cpp
  21. +1 1  src/JavascriptAlertMessages.cpp
  22. +1 1  src/JavascriptConfirmMessages.cpp
  23. +1 1  src/JavascriptPromptMessages.cpp
  24. +1 1  src/Node.cpp
  25. +1 1  src/NullCommand.cpp
  26. +3 2 src/PageLoadingCommand.cpp
  27. +1 1  src/Render.cpp
  28. +1 1  src/Reset.cpp
  29. +1 1  src/ResizeWindow.cpp
  30. +3 3 src/Response.cpp
  31. +13 4 src/Response.h
  32. +1 1  src/SetConfirmAction.cpp
  33. +1 1  src/SetCookie.cpp
  34. +1 1  src/SetPromptAction.cpp
  35. +1 1  src/SetPromptText.cpp
  36. +1 1  src/SetProxy.cpp
  37. +1 1  src/SetSkipImageLoading.cpp
  38. +2 2 src/SetTimeout.cpp
  39. +1 1  src/SetUrlBlacklist.cpp
  40. +1 1  src/Status.cpp
  41. +3 4 src/TimeoutCommand.cpp
  42. +1 1  src/Visit.cpp
  43. +2 2 src/WindowFocus.cpp
  44. +1 1  src/body.cpp
2  src/Authenticate.cpp
@@ -13,6 +13,6 @@ void Authenticate::start() {
13 13 networkAccessManager->setUserName(username);
14 14 networkAccessManager->setPassword(password);
15 15
16   - emit finished(new Response(true));
  16 + emitFinished(true);
17 17 }
18 18
2  src/ClearCookies.cpp
@@ -9,5 +9,5 @@ ClearCookies::ClearCookies(WebPageManager *manager, QStringList &arguments, QObj
9 9 void ClearCookies::start()
10 10 {
11 11 manager()->cookieJar()->clearCookies();
12   - emit finished(new Response(true));
  12 + emitFinished(true);
13 13 }
2  src/ClearPromptText.cpp
@@ -7,5 +7,5 @@ ClearPromptText::ClearPromptText(WebPageManager *manager, QStringList &arguments
7 7 void ClearPromptText::start()
8 8 {
9 9 page()->setPromptText(QString());
10   - emit finished(new Response(true));
  10 + emitFinished(true);
11 11 }
9 src/Command.cpp
@@ -6,3 +6,12 @@ Command::Command(QObject *parent) : QObject(parent) {
6 6 QString Command::toString() const {
7 7 return metaObject()->className();
8 8 }
  9 +
  10 +void Command::emitFinished(bool success) {
  11 + emit finished(new Response(success, this));
  12 +}
  13 +
  14 +void Command::emitFinished(bool success, QString message) {
  15 + emit finished(new Response(success, message, this));
  16 +}
  17 +
5 src/Command.h
@@ -3,6 +3,7 @@
3 3
4 4 #include <QObject>
5 5 #include "Response.h"
  6 +#include <QString>
6 7
7 8 class Command : public QObject {
8 9 Q_OBJECT
@@ -12,6 +13,10 @@ class Command : public QObject {
12 13 virtual void start() = 0;
13 14 virtual QString toString() const;
14 15
  16 + protected:
  17 + void emitFinished(bool success);
  18 + void emitFinished(bool success, QString message);
  19 +
15 20 signals:
16 21 void finished(Response *response);
17 22 };
7 src/Connection.cpp
@@ -43,13 +43,15 @@ void Connection::pendingLoadFinished(bool success) {
43 43 void Connection::writePageLoadFailure() {
44 44 m_pageSuccess = true;
45 45 QString message = currentPage()->failureString();
46   - writeResponse(new Response(false, message));
  46 + Response *response = new Response(false, message, this);
  47 + writeResponse(response);
  48 + delete response;
47 49 }
48 50
49 51 void Connection::finishCommand(Response *response) {
50 52 m_pageSuccess = true;
51   - sender()->deleteLater();
52 53 writeResponse(response);
  54 + sender()->deleteLater();
53 55 }
54 56
55 57 void Connection::writeResponse(Response *response) {
@@ -64,7 +66,6 @@ void Connection::writeResponse(Response *response) {
64 66 QString messageLength = QString::number(messageUtf8.size()) + "\n";
65 67 m_socket->write(messageLength.toAscii());
66 68 m_socket->write(messageUtf8);
67   - delete response;
68 69 }
69 70
70 71 WebPage *Connection::currentPage() {
2  src/ConsoleMessages.cpp
@@ -6,6 +6,6 @@ ConsoleMessages::ConsoleMessages(WebPageManager *manager, QStringList &arguments
6 6 }
7 7
8 8 void ConsoleMessages::start() {
9   - emit finished(new Response(true, page()->consoleMessages()));
  9 + emitFinished(true, page()->consoleMessages());
10 10 }
11 11
2  src/CurrentUrl.cpp
@@ -9,6 +9,6 @@ void CurrentUrl::start() {
9 9 QStringList arguments;
10 10 QVariant result = page()->invokeCapybaraFunction("currentUrl", arguments);
11 11 QString url = result.toString();
12   - emit finished(new Response(true, url));
  12 + emitFinished(true, url);
13 13 }
14 14
2  src/EnableLogging.cpp
@@ -6,5 +6,5 @@ EnableLogging::EnableLogging(WebPageManager *manager, QStringList &arguments, QO
6 6
7 7 void EnableLogging::start() {
8 8 manager()->enableLogging();
9   - emit finished(new Response(true));
  9 + emitFinished(true);
10 10 }
2  src/Evaluate.cpp
@@ -10,7 +10,7 @@ Evaluate::Evaluate(WebPageManager *manager, QStringList &arguments, QObject *par
10 10 void Evaluate::start() {
11 11 QVariant result = page()->currentFrame()->evaluateJavaScript(arguments()[0]);
12 12 addVariant(result);
13   - emit finished(new Response(true, m_buffer));
  13 + emitFinished(true, m_buffer);
14 14 }
15 15
16 16 void Evaluate::addVariant(QVariant &object) {
4 src/Execute.cpp
@@ -9,9 +9,9 @@ void Execute::start() {
9 9 QString script = arguments()[0] + QString("; 'success'");
10 10 QVariant result = page()->currentFrame()->evaluateJavaScript(script);
11 11 if (result.isValid()) {
12   - emit finished(new Response(true));
  12 + emitFinished(true);
13 13 } else {
14   - emit finished(new Response(false, QString("Javascript failed to execute")));
  14 + emitFinished(false, QString("Javascript failed to execute"));
15 15 }
16 16 }
17 17
4 src/Find.cpp
@@ -12,9 +12,9 @@ void Find::start() {
12 12
13 13 if (result.isValid()) {
14 14 message = result.toString();
15   - emit finished(new Response(true, message));
  15 + emitFinished(true, message);
16 16 } else {
17   - emit finished(new Response(false, QString("Invalid XPath expression")));
  17 + emitFinished(false, QString("Invalid XPath expression"));
18 18 }
19 19 }
20 20
6 src/FrameFocus.cpp
@@ -51,7 +51,7 @@ void FrameFocus::focusId(QString name) {
51 51
52 52 void FrameFocus::focusParent() {
53 53 if (page()->currentFrame()->parentFrame() == 0) {
54   - emit finished(new Response(false, QString("Already at parent frame.")));
  54 + emitFinished(false, QString("Already at parent frame."));
55 55 } else {
56 56 page()->currentFrame()->parentFrame()->setFocus();
57 57 success();
@@ -59,9 +59,9 @@ void FrameFocus::focusParent() {
59 59 }
60 60
61 61 void FrameFocus::frameNotFound() {
62   - emit finished(new Response(false, QString("Unable to locate frame. ")));
  62 + emitFinished(false, QString("Unable to locate frame. "));
63 63 }
64 64
65 65 void FrameFocus::success() {
66   - emit finished(new Response(true));
  66 + emitFinished(true);
67 67 }
2  src/GetCookies.cpp
@@ -15,5 +15,5 @@ void GetCookies::start()
15 15 m_buffer.append(cookie.toRawForm());
16 16 m_buffer.append("\n");
17 17 }
18   - emit finished(new Response(true, m_buffer));
  18 + emitFinished(true, m_buffer);
19 19 }
2  src/GetTimeout.cpp
@@ -5,5 +5,5 @@ GetTimeout::GetTimeout(WebPageManager *manager, QStringList &arguments, QObject
5 5 }
6 6
7 7 void GetTimeout::start() {
8   - emit finished(new Response(true, QString::number(manager()->getTimeout())));
  8 + emitFinished(true, QString::number(manager()->getTimeout()));
9 9 }
2  src/GetWindowHandle.cpp
@@ -7,5 +7,5 @@ GetWindowHandle::GetWindowHandle(WebPageManager *manager, QStringList &arguments
7 7 }
8 8
9 9 void GetWindowHandle::start() {
10   - emit finished(new Response(true, page()->uuid()));
  10 + emitFinished(true, page()->uuid());
11 11 }
2  src/GetWindowHandles.cpp
@@ -16,5 +16,5 @@ void GetWindowHandles::start() {
16 16
17 17 handles += stringList.join(",") + "]";
18 18
19   - emit finished(new Response(true, handles));
  19 + emitFinished(true, handles);
20 20 }
2  src/Header.cpp
@@ -15,5 +15,5 @@ void Header::start() {
15 15 } else {
16 16 networkAccessManager->addHeader(key, value);
17 17 }
18   - emit finished(new Response(true));
  18 + emitFinished(true);
19 19 }
2  src/Headers.cpp
@@ -11,6 +11,6 @@ void Headers::start() {
11 11 foreach(QNetworkReply::RawHeaderPair header, page()->pageHeaders())
12 12 headers << header.first+": "+header.second;
13 13
14   - emit finished(new Response(true, headers.join("\n")));
  14 + emitFinished(true, headers.join("\n"));
15 15 }
16 16
2  src/IgnoreSslErrors.cpp
@@ -8,6 +8,6 @@ IgnoreSslErrors::IgnoreSslErrors(WebPageManager *manager, QStringList &arguments
8 8
9 9 void IgnoreSslErrors::start() {
10 10 manager()->setIgnoreSslErrors(true);
11   - emit finished(new Response(true));
  11 + emitFinished(true);
12 12 }
13 13
2  src/JavascriptAlertMessages.cpp
@@ -6,5 +6,5 @@ JavascriptAlertMessages::JavascriptAlertMessages(WebPageManager *manager, QStrin
6 6
7 7 void JavascriptAlertMessages::start()
8 8 {
9   - emit finished(new Response(true, page()->alertMessages()));
  9 + emitFinished(true, page()->alertMessages());
10 10 }
2  src/JavascriptConfirmMessages.cpp
@@ -6,5 +6,5 @@ JavascriptConfirmMessages::JavascriptConfirmMessages(WebPageManager *manager, QS
6 6
7 7 void JavascriptConfirmMessages::start()
8 8 {
9   - emit finished(new Response(true, page()->confirmMessages()));
  9 + emitFinished(true, page()->confirmMessages());
10 10 }
2  src/JavascriptPromptMessages.cpp
@@ -6,5 +6,5 @@ JavascriptPromptMessages::JavascriptPromptMessages(WebPageManager *manager, QStr
6 6
7 7 void JavascriptPromptMessages::start()
8 8 {
9   - emit finished(new Response(true, page()->promptMessages()));
  9 + emitFinished(true, page()->promptMessages());
10 10 }
2  src/Node.cpp
@@ -10,7 +10,7 @@ void Node::start() {
10 10 QString functionName = functionArguments.takeFirst();
11 11 QVariant result = page()->invokeCapybaraFunction(functionName, functionArguments);
12 12 QString attributeValue = result.toString();
13   - emit finished(new Response(true, attributeValue));
  13 + emitFinished(true, attributeValue);
14 14 }
15 15
16 16 QString Node::toString() const {
2  src/NullCommand.cpp
@@ -8,6 +8,6 @@ NullCommand::NullCommand(QString name, QObject *parent) : Command(parent) {
8 8
9 9 void NullCommand::start() {
10 10 QString failure = QString("[Capybara WebKit] Unknown command: ") + m_name + "\n";
11   - emit finished(new Response(false, failure));
  11 + emitFinished(false, failure);
12 12 }
13 13
5 src/PageLoadingCommand.cpp
@@ -9,6 +9,7 @@ PageLoadingCommand::PageLoadingCommand(Command *command, WebPageManager *manager
9 9 m_pageLoadingFromCommand = false;
10 10 m_pageSuccess = true;
11 11 m_pendingResponse = NULL;
  12 + m_command->setParent(this);
12 13 }
13 14
14 15 void PageLoadingCommand::start() {
@@ -29,7 +30,7 @@ void PageLoadingCommand::pendingLoadFinished(bool success) {
29 30 emit finished(m_pendingResponse);
30 31 } else {
31 32 QString message = m_manager->currentPage()->failureString();
32   - emit finished(new Response(false, message));
  33 + emitFinished(false, message);
33 34 }
34 35 }
35 36 }
@@ -43,7 +44,7 @@ void PageLoadingCommand::pageLoadingFromCommand() {
43 44 void PageLoadingCommand::commandFinished(Response *response) {
44 45 disconnect(m_manager, SIGNAL(loadStarted()), this, SLOT(pageLoadingFromCommand()));
45 46 m_manager->logger() << "Finished" << m_command->toString() << "with response" << response->toString();
46   - m_command->deleteLater();
  47 +
47 48 if (m_pageLoadingFromCommand)
48 49 m_pendingResponse = response;
49 50 else
2  src/Render.cpp
@@ -15,5 +15,5 @@ void Render::start() {
15 15
16 16 bool result = page()->render( imagePath );
17 17
18   - emit finished(new Response(result));
  18 + emitFinished(result);
19 19 }
2  src/Reset.cpp
@@ -10,6 +10,6 @@ void Reset::start() {
10 10
11 11 manager()->reset();
12 12
13   - emit finished(new Response(true));
  13 + emitFinished(true);
14 14 }
15 15
2  src/ResizeWindow.cpp
@@ -12,6 +12,6 @@ void ResizeWindow::start() {
12 12 QSize size(width, height);
13 13 page()->setViewportSize(size);
14 14
15   - emit finished(new Response(true));
  15 + emitFinished(true);
16 16 }
17 17
6 src/Response.cpp
... ... @@ -1,17 +1,17 @@
1 1 #include "Response.h"
2 2 #include <iostream>
3 3
4   -Response::Response(bool success, QString message) {
  4 +Response::Response(bool success, QString message, QObject *parent) : QObject(parent) {
5 5 m_success = success;
6 6 m_message = message.toUtf8();
7 7 }
8 8
9   -Response::Response(bool success, QByteArray message) {
  9 +Response::Response(bool success, QByteArray message, QObject *parent) : QObject(parent) {
10 10 m_success = success;
11 11 m_message = message;
12 12 }
13 13
14   -Response::Response(bool success) {
  14 +Response::Response(bool success, QObject *parent) : QObject(parent) {
15 15 m_success = success;
16 16 }
17 17
17 src/Response.h
... ... @@ -1,11 +1,17 @@
  1 +#ifndef RESPONSE_H
  2 +#define RESPONSE_H
  3 +
  4 +#include <QObject>
1 5 #include <QString>
2 6 #include <QByteArray>
3 7
4   -class Response {
  8 +class Response : public QObject {
  9 + Q_OBJECT
  10 +
5 11 public:
6   - Response(bool success, QString message);
7   - Response(bool success, QByteArray message);
8   - Response(bool success);
  12 + Response(bool success, QString message, QObject *parent);
  13 + Response(bool success, QByteArray message, QObject *parent);
  14 + Response(bool success, QObject *parent);
9 15 bool isSuccess() const;
10 16 QByteArray message() const;
11 17 QString toString() const;
@@ -14,3 +20,6 @@ class Response {
14 20 bool m_success;
15 21 QByteArray m_message;
16 22 };
  23 +
  24 +#endif
  25 +
2  src/SetConfirmAction.cpp
@@ -7,5 +7,5 @@ SetConfirmAction::SetConfirmAction(WebPageManager *manager, QStringList &argumen
7 7 void SetConfirmAction::start()
8 8 {
9 9 page()->setConfirmAction(arguments()[0]);
10   - emit finished(new Response(true));
  10 + emitFinished(true);
11 11 }
2  src/SetCookie.cpp
@@ -11,5 +11,5 @@ void SetCookie::start()
11 11 QList<QNetworkCookie> cookies = QNetworkCookie::parseCookies(arguments()[0].toAscii());
12 12 NetworkCookieJar *jar = manager()->cookieJar();
13 13 jar->overwriteCookies(cookies);
14   - emit finished(new Response(true));
  14 + emitFinished(true);
15 15 }
2  src/SetPromptAction.cpp
@@ -7,5 +7,5 @@ SetPromptAction::SetPromptAction(WebPageManager *manager, QStringList &arguments
7 7 void SetPromptAction::start()
8 8 {
9 9 page()->setPromptAction(arguments()[0]);
10   - emit finished(new Response(true));
  10 + emitFinished(true);
11 11 }
2  src/SetPromptText.cpp
@@ -7,5 +7,5 @@ SetPromptText::SetPromptText(WebPageManager *manager, QStringList &arguments, QO
7 7 void SetPromptText::start()
8 8 {
9 9 page()->setPromptText(arguments()[0]);
10   - emit finished(new Response(true));
  10 + emitFinished(true);
11 11 }
2  src/SetProxy.cpp
@@ -19,5 +19,5 @@ void SetProxy::start()
19 19 arguments()[3]);
20 20
21 21 page()->networkAccessManager()->setProxy(proxy);
22   - emit finished(new Response(true));
  22 + emitFinished(true);
23 23 }
2  src/SetSkipImageLoading.cpp
@@ -8,5 +8,5 @@ SetSkipImageLoading::SetSkipImageLoading(WebPageManager *manager, QStringList &a
8 8
9 9 void SetSkipImageLoading::start() {
10 10 page()->setSkipImageLoading(arguments().contains("true"));
11   - emit finished(new Response(true));
  11 + emitFinished(true);
12 12 }
4 src/SetTimeout.cpp
@@ -11,9 +11,9 @@ void SetTimeout::start() {
11 11
12 12 if (ok) {
13 13 manager()->setTimeout(timeout);
14   - emit finished(new Response(true));
  14 + emitFinished(true);
15 15 } else {
16   - emit finished(new Response(false, QString("Invalid value for timeout")));
  16 + emitFinished(false, QString("Invalid value for timeout"));
17 17 }
18 18 }
19 19
2  src/SetUrlBlacklist.cpp
@@ -10,6 +10,6 @@ SetUrlBlacklist::SetUrlBlacklist(WebPageManager *manager, QStringList &arguments
10 10 void SetUrlBlacklist::start() {
11 11 NetworkAccessManager* networkAccessManager = page()->networkAccessManager();
12 12 networkAccessManager->setUrlBlacklist(arguments());
13   - emit finished(new Response(true));
  13 + emitFinished(true);
14 14 }
15 15
2  src/Status.cpp
@@ -8,6 +8,6 @@ Status::Status(WebPageManager *manager, QStringList &arguments, QObject *parent)
8 8
9 9 void Status::start() {
10 10 int status = page()->getLastStatus();
11   - emit finished(new Response(true, QString::number(status)));
  11 + emitFinished(true, QString::number(status));
12 12 }
13 13
7 src/TimeoutCommand.cpp
@@ -10,6 +10,7 @@ TimeoutCommand::TimeoutCommand(Command *command, WebPageManager *manager, QObjec
10 10 m_manager = manager;
11 11 m_timer = new QTimer(this);
12 12 m_timer->setSingleShot(true);
  13 + m_command->setParent(this);
13 14 connect(m_timer, SIGNAL(timeout()), this, SLOT(commandTimeout()));
14 15 connect(m_manager, SIGNAL(loadStarted()), this, SLOT(pageLoadingFromCommand()));
15 16 }
@@ -44,7 +45,7 @@ void TimeoutCommand::pendingLoadFinished(bool success) {
44 45 disconnect(m_timer, SIGNAL(timeout()), this, SLOT(commandTimeout()));
45 46 disconnect(m_manager, SIGNAL(loadStarted()), this, SLOT(pageLoadingFromCommand()));
46 47 disconnect(m_manager, SIGNAL(pageFinished(bool)), this, SLOT(pendingLoadFinished(bool)));
47   - emit finished(new Response(false, m_manager->currentPage()->failureString()));
  48 + emitFinished(false, m_manager->currentPage()->failureString());
48 49 }
49 50 }
50 51
@@ -57,15 +58,13 @@ void TimeoutCommand::commandTimeout() {
57 58 disconnect(m_manager, SIGNAL(pageFinished(bool)), this, SLOT(pendingLoadFinished(bool)));
58 59 disconnect(m_command, SIGNAL(finished(Response *)), this, SLOT(commandFinished(Response *)));
59 60 m_manager->currentPage()->triggerAction(QWebPage::Stop);
60   - m_command->deleteLater();
61   - emit finished(new Response(false, QString("timeout")));
  61 + emit finished(new Response(false, QString("timeout"), this));
62 62 }
63 63
64 64 void TimeoutCommand::commandFinished(Response *response) {
65 65 disconnect(m_timer, SIGNAL(timeout()), this, SLOT(commandTimeout()));
66 66 disconnect(m_manager, SIGNAL(loadStarted()), this, SLOT(pageLoadingFromCommand()));
67 67 disconnect(m_manager, SIGNAL(pageFinished(bool)), this, SLOT(pendingLoadFinished(bool)));
68   - m_command->deleteLater();
69 68 emit finished(response);
70 69 }
71 70
2  src/Visit.cpp
@@ -9,5 +9,5 @@ Visit::Visit(WebPageManager *manager, QStringList &arguments, QObject *parent) :
9 9 void Visit::start() {
10 10 QUrl requestedUrl = QUrl::fromEncoded(arguments()[0].toUtf8(), QUrl::StrictMode);
11 11 page()->currentFrame()->load(QUrl(requestedUrl));
12   - emit finished(new Response(true));
  12 + emitFinished(true);
13 13 }
4 src/WindowFocus.cpp
@@ -12,12 +12,12 @@ void WindowFocus::start() {
12 12 }
13 13
14 14 void WindowFocus::windowNotFound() {
15   - emit finished(new Response(false, QString("Unable to locate window. ")));
  15 + emitFinished(false, QString("Unable to locate window. "));
16 16 }
17 17
18 18 void WindowFocus::success(WebPage *page) {
19 19 page->setFocus();
20   - emit finished(new Response(true));
  20 + emitFinished(true);
21 21 }
22 22
23 23 void WindowFocus::focusWindow(QString selector) {
2  src/body.cpp
@@ -12,5 +12,5 @@ void Body::start() {
12 12 else
13 13 result = page()->currentFrame()->toHtml();
14 14
15   - emit finished(new Response(true, result));
  15 + emitFinished(true, result);
16 16 }

0 comments on commit 77811ca

Please sign in to comment.
Something went wrong with that request. Please try again.