[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - 43 commits - common/MessageQueue.cpp common/Session.cpp common/Session.hpp kit/Kit.cpp loleaflet/dist loleaflet/src net/Socket.cpp net/Socket.hpp test/httpwserror.cpp test/httpwstest.cpp test/run_unit.sh.in test/test.cpp test/TileQueueTests.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/FileServer.cpp wsd/FileServer.hpp wsd/LOOLWSD.cpp wsd/SenderQueue.hpp wsd/TileCache.cpp

[Michael Meeks]

 common/MessageQueue.cpp                   |    1 
 common/Session.cpp                        |   22 ++++
 common/Session.hpp                        |    2 
 kit/Kit.cpp                               |    7 -
 loleaflet/dist/loleaflet.html             |   11 +-
 loleaflet/dist/toolbar/toolbar.js         |   21 ++--
 loleaflet/src/layer/tile/CalcTileLayer.js |    6 +
 net/Socket.cpp                            |   25 +++--
 net/Socket.hpp                            |   43 +++++++--
 test/TileQueueTests.cpp                   |   28 +++++
 test/httpwserror.cpp                      |    6 -
 test/httpwstest.cpp                       |   24 +++--
 test/run_unit.sh.in                       |    8 +
 test/test.cpp                             |   18 +++
 wsd/ClientSession.cpp                     |    7 -
 wsd/ClientSession.hpp                     |    8 -
 wsd/DocumentBroker.cpp                    |   47 +++++++---
 wsd/DocumentBroker.hpp                    |   16 +--
 wsd/FileServer.cpp                        |   81 +++++++++--------
 wsd/FileServer.hpp                        |    8 +
 wsd/LOOLWSD.cpp                           |  141 ++++++++++++++----------------
 wsd/SenderQueue.hpp                       |   30 ++----
 wsd/TileCache.cpp                         |    3 
 23 files changed, 354 insertions(+), 209 deletions(-)

New commits:
commit 4b3c007ef0e260fa043e5d202ec6158d7ecdb7d8
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Tue Apr 4 13:17:23 2017 +0100

    Use heap buffers for file transfer and disable deflate for now.
    
    [This is disabled; cherry-picking just to keep the difference against master
    minimal.]
    
    Change-Id: I90ff2b0a38a4e9155397440e803c145aba47e128

diff --git a/net/Socket.cpp b/net/Socket.cpp
index 662ddb93..e4d2df4e 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -228,13 +228,13 @@ namespace HttpHelper
 
             std::ifstream file(path, std::ios::binary);
             bool flush = true;
+            std::unique_ptr<char[]> buf(new char[bufferSize]);
             do
             {
-                char buf[bufferSize];
-                file.read(buf, sizeof(buf));
+                file.read(&buf[0], bufferSize);
                 const int size = file.gcount();
                 if (size > 0)
-                    socket->send(buf, size, flush);
+                    socket->send(&buf[0], size, flush);
                 else
                     break;
                 flush = false;
@@ -252,17 +252,22 @@ namespace HttpHelper
 
             std::ifstream file(path, std::ios::binary);
             bool flush = true;
+
+            // FIXME: Should compress once ahead of time
+            // compression of bundle.js takes significant time:
+            //   200's ms for level 9 (468k), 72ms for level 1(587k)
+            //   down from 2Mb.
+            std::unique_ptr<char[]> buf(new char[st.st_size]);
             do
             {
-                static const unsigned int level = 9;
-                char buf[st.st_size]; // FIXME: Should compress in chunks.
-                file.read(buf, st.st_size);
+                static const unsigned int level = 1;
+                file.read(&buf[0], st.st_size);
                 const long unsigned int size = file.gcount();
                 long unsigned int compSize = compressBound(size);
-                char cbuf[compSize];
-                compress2((Bytef *)&cbuf, &compSize, (Bytef *)&buf, size, 
level);
+                std::unique_ptr<char[]> cbuf(new char[compSize]);
+                compress2((Bytef *)&cbuf[0], &compSize, (Bytef *)&buf[0], 
size, level);
                 if (size > 0)
-                    socket->send(cbuf, compSize, flush);
+                    socket->send(&cbuf[0], compSize, flush);
                 else
                     break;
                 flush = false;
commit ebe40346e79bbda1cf2794e978e72d722ae6d19a
Author: Jan Holesovsky <ke...@collabora.com>
Date:   Fri Apr 7 15:37:20 2017 +0200

    Write the failures we got during the test run.
    
    Change-Id: I2c05b6f2c890b3a67824f1ca612fa7f4e05d994f

diff --git a/test/run_unit.sh.in b/test/run_unit.sh.in
index 4b8e71cf..32d7ba2d 100755
--- a/test/run_unit.sh.in
+++ b/test/run_unit.sh.in
@@ -13,7 +13,7 @@ enable_debug="@ENABLE_DEBUG@"
 jails_path="@JAILS_PATH@"
 lo_path="@LO_PATH@"
 valgrind_cmd="valgrind --tool=memcheck --trace-children=no -v 
--read-var-info=yes"
-hide_stderr='2>/dev/null'
+verbose=''
 
 # Note that these options are used by commands in the Makefile that
 # Automake generates. Don't be mislead by 'git grep' not showing any
@@ -26,7 +26,7 @@ while test $# -gt 0; do
       --log-file)  tst_log=$2; shift;;
       --trs-file)  test_output=$2; shift;;
       --valgrind)  valgrind=$valgrind_cmd; shift;;
-      --verbose)   hide_stderr="";;
+      --verbose)   verbose="--verbose";;
   -*) ;; # ignore
   esac
   shift
@@ -86,7 +86,7 @@ if test "z$tst" == "z"; then
 
      oldpath=`pwd`
      cd "${abs_top_builddir}/test"
-     if eval ${valgrind} ./test ${hide_stderr}; then
+     if eval ${valgrind} ./test ${verbose}; then
         echo "Test run_test.sh passed."
         echo ":test-result: PASS run_test.sh" >> $oldpath/$test_output
         retval=0
diff --git a/test/test.cpp b/test/test.cpp
index 9ad21cd5..fc0a6dfb 100644
--- a/test/test.cpp
+++ b/test/test.cpp
@@ -56,9 +56,13 @@ bool filterTests(CPPUNIT_NS::TestRunner& runner, 
CPPUNIT_NS::Test* testRegistry,
     return haveTests;
 }
 
-int main(int /*argc*/, char** /*argv*/)
+int main(int argc, char** argv)
 {
-    Log::initialize("tst", "trace", true, false, {});
+    const char* loglevel = "error";
+    if (argc > 0 && std::string("--verbose") == argv[0])
+        loglevel = "trace";
+
+    Log::initialize("tst", loglevel, true, false, {});
 
     CPPUNIT_NS::TestResult controller;
     CPPUNIT_NS::TestResultCollector result;
@@ -92,8 +96,18 @@ int main(int /*argc*/, char** /*argv*/)
         }
     }
 
+    // redirect std::cerr temporarily
+    std::stringstream errorBuffer;
+    std::streambuf* oldCerr = std::cerr.rdbuf(errorBuffer.rdbuf());
+
     runner.run(controller);
 
+    std::cerr.rdbuf(oldCerr);
+
+    // output the errors we got during the testing
+    if (!result.wasSuccessful())
+        std::cerr << errorBuffer.str() << std::endl;
+
     CPPUNIT_NS::CompilerOutputter outputter(&result, std::cerr);
     outputter.setNoWrap();
     outputter.write();
commit d5142f84a114e33c1d312a26f7c5bdfd13cc0635
Author: Jan Holesovsky <ke...@collabora.com>
Date:   Fri Apr 7 12:29:01 2017 +0200

    Unit tests for the indicator value and page size callback merging.
    
    Change-Id: Id97fcf9bad37669eb649f73b38b4dba0b2e9a00e

diff --git a/test/TileQueueTests.cpp b/test/TileQueueTests.cpp
index 4128df3e..2bf8e7a7 100644
--- a/test/TileQueueTests.cpp
+++ b/test/TileQueueTests.cpp
@@ -50,6 +50,8 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testSenderQueueTileDeduplication);
     CPPUNIT_TEST(testInvalidateViewCursorDeduplication);
     CPPUNIT_TEST(testCallbackInvalidation);
+    CPPUNIT_TEST(testCallbackIndicatorValue);
+    CPPUNIT_TEST(testCallbackPageSize);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -62,6 +64,8 @@ class TileQueueTests : public CPPUNIT_NS::TestFixture
     void testSenderQueueTileDeduplication();
     void testInvalidateViewCursorDeduplication();
     void testCallbackInvalidation();
+    void testCallbackIndicatorValue();
+    void testCallbackPageSize();
 };
 
 void TileQueueTests::testTileQueuePriority()
@@ -444,6 +448,30 @@ void TileQueueTests::testCallbackInvalidation()
     CPPUNIT_ASSERT_EQUAL(std::string("callback all 0 EMPTY, 0"), 
payloadAsString(queue.get()));
 }
 
+void TileQueueTests::testCallbackIndicatorValue()
+{
+    TileQueue queue;
+
+    // join tiles
+    queue.put("callback all 10 25");
+    queue.put("callback all 10 50");
+
+    CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(queue._queue.size()));
+    CPPUNIT_ASSERT_EQUAL(std::string("callback all 10 50"), 
payloadAsString(queue.get()));
+}
+
+void TileQueueTests::testCallbackPageSize()
+{
+    TileQueue queue;
+
+    // join tiles
+    queue.put("callback all 13 12474, 188626");
+    queue.put("callback all 13 12474, 205748");
+
+    CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(queue._queue.size()));
+    CPPUNIT_ASSERT_EQUAL(std::string("callback all 13 12474, 205748"), 
payloadAsString(queue.get()));
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(TileQueueTests);
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
commit ccf1cc7b30bc67b853dd8e9ac60c81a48f985477
Author: Jan Holesovsky <ke...@collabora.com>
Date:   Fri Apr 7 12:13:45 2017 +0200

    Merge document size changes callbacks in the message queue.
    
    Change-Id: I1a540b17f2a72c374568db834a30b814878e9032

diff --git a/common/MessageQueue.cpp b/common/MessageQueue.cpp
index acff9b88..a53ae988 100644
--- a/common/MessageQueue.cpp
+++ b/common/MessageQueue.cpp
@@ -341,6 +341,7 @@ std::string TileQueue::removeCallbackDuplicate(const 
std::string& callbackMsg)
     else if (callbackType == "1" || // the cursor has moved
             callbackType == "5" ||  // the cursor visibility has changed
             callbackType == "10" || // setting the indicator value
+            callbackType == "13" || // setting the document size
             callbackType == "17" || // the cell cursor has moved
             callbackType == "24" || // the view cursor has moved
             callbackType == "26" || // the view cell cursor has moved
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 242dc575..6717c677 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -889,10 +889,11 @@ public:
         }
 
         // merge various callback types together if possible
-        if (type == LOK_CALLBACK_INVALIDATE_TILES)
+        if (type == LOK_CALLBACK_INVALIDATE_TILES ||
+            type == LOK_CALLBACK_DOCUMENT_SIZE_CHANGED)
         {
-            // no point in handling invalidations per-view, all views have to
-            // be in sync
+            // no point in handling invalidations or page resizes per-view,
+            // all views have to be in sync
             tileQueue->put("callback all " + std::to_string(type) + ' ' + 
payload);
         }
         else if (type == LOK_CALLBACK_INVALIDATE_VIEW_CURSOR ||
commit de2bc17c04af088d9c7e18a97216b174494e1a9c
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Fri Apr 7 12:16:51 2017 +0530

    wsd: Fileserver cleanup
    
    Remove unnecessary checks
    
    Rename preprocessFile -> preprocessAndSendLoleafletHtml and
    Rename isAdminLoggedIn -> tryAdminLogin
    so that their name matches the actual reality of what these
    function really does.
    
    Change-Id: I549eae31f8ab0a320bb3ff8ecd17a282b8f91e1a

diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 29be66f6..70abae4a 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -44,8 +44,8 @@ using Poco::Net::HTTPBasicCredentials;
 using Poco::StreamCopier;
 using Poco::Util::Application;
 
-bool FileServerRequestHandler::isAdminLoggedIn(const HTTPRequest& request,
-                                               HTTPResponse &response)
+bool FileServerRequestHandler::tryAdminLogin(const HTTPRequest& request,
+                                             HTTPResponse &response)
 {
     const auto& config = Application::instance().config();
     const auto sslKeyPath = config.getString("ssl.key_file_path", "");
@@ -109,53 +109,40 @@ void FileServerRequestHandler::handleRequest(const 
HTTPRequest& request, Poco::M
     {
         bool noCache = false;
         Poco::Net::HTTPResponse response;
-        Poco::URI requestUri(request.getURI());
-        LOG_TRC("Fileserver request: " << requestUri.toString());
-        requestUri.normalize(); // avoid .'s and ..'s
-
-        std::vector<std::string> requestSegments;
-        requestUri.getPathSegments(requestSegments);
-        if (requestSegments.size() < 1)
+        const auto requestPathname = Poco::Path(getRequestPathname(request));
+        const auto filePath = Poco::Path(LOOLWSD::FileServerRoot, 
requestPathname);
+        const auto file = Poco::File(filePath);
+        LOG_TRC("Fileserver request: " << requestPathname.toString() << ", " <<
+                "Resolved file path: " << filePath.toString());
+
+        if (!file.exists() ||
+            requestPathname[0] != "loleaflet" ||
+            requestPathname[1] != "dist")
         {
-            throw Poco::FileNotFoundException("Invalid URI request: [" + 
requestUri.toString() + "].");
+            throw Poco::FileNotFoundException("Invalid URI request: [" + 
filePath.toString() + "].");
         }
 
         const auto& config = Application::instance().config();
         const std::string loleafletHtml = config.getString("loleaflet_html", 
"loleaflet.html");
-        const std::string endPoint = requestSegments[requestSegments.size() - 
1];
-        if (endPoint == loleafletHtml)
+        if (filePath.getFileName() == loleafletHtml)
         {
-            preprocessFile(request, message, socket);
+            preprocessAndSendLoleafletHtml(request, message, socket);
             return;
         }
 
         if (request.getMethod() == HTTPRequest::HTTP_GET)
         {
-            if (endPoint == "admin.html" ||
-                endPoint == "adminSettings.html" ||
-                endPoint == "adminAnalytics.html")
+            if (filePath.getFileName() == "admin.html" ||
+                filePath.getFileName() == "adminSettings.html" ||
+                filePath.getFileName() == "adminAnalytics.html")
             {
                 noCache = true;
 
-                if (!FileServerRequestHandler::isAdminLoggedIn(request, 
response))
+                if (!FileServerRequestHandler::tryAdminLogin(request, 
response))
                     throw Poco::Net::NotAuthenticatedException("Invalid admin 
login");
             }
 
-            const auto path = Poco::Path(LOOLWSD::FileServerRoot, 
getRequestPathname(request));
-            const auto filepath = path.absolute().toString();
-            if (filepath.find(LOOLWSD::FileServerRoot) != 0)
-            {
-                // Accessing unauthorized path.
-                throw Poco::FileAccessDeniedException("Invalid or forbidden 
file path: [" + filepath + "].");
-            }
-
-            const std::size_t extPoint = endPoint.find_last_of('.');
-            if (extPoint == std::string::npos)
-            {
-                throw Poco::FileNotFoundException("Invalid file.");
-            }
-
-            const std::string fileType = endPoint.substr(extPoint + 1);
+            const std::string fileType = filePath.getExtension();
             std::string mimeType;
             if (fileType == "js")
                 mimeType = "application/javascript";
@@ -195,7 +182,7 @@ void FileServerRequestHandler::handleRequest(const 
HTTPRequest& request, Poco::M
 
             response.setContentType(mimeType);
             bool deflate = request.hasToken("Accept-Encoding", "deflate");
-            HttpHelper::sendFile(socket, filepath, response, noCache, deflate);
+            HttpHelper::sendFile(socket, filePath.toString(), response, 
noCache, deflate);
         }
     }
     catch (const Poco::Net::NotAuthenticatedException& exc)
@@ -248,13 +235,17 @@ std::string 
FileServerRequestHandler::getRequestPathname(const HTTPRequest& requ
 
     std::string path(requestUri.getPath());
 
-    // Convert version back to a real file name. Remove first foreslash as the 
root ends in one.
-    Poco::replaceInPlace(path, std::string("/loleaflet/" LOOLWSD_VERSION_HASH 
"/"), std::string("loleaflet/dist/"));
+    // Remove first foreslash as the root ends in one.
+    if (path[0] == '/')
+        path = path.substr(1);
+
+    // Convert version back to a real file name.
+    Poco::replaceInPlace(path, std::string("loleaflet/" LOOLWSD_VERSION_HASH 
"/"), std::string("loleaflet/dist/"));
 
     return path;
 }
 
-void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, 
Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket)
+void FileServerRequestHandler::preprocessAndSendLoleafletHtml(const 
HTTPRequest& request, Poco::MemoryInputStream& message, const 
std::shared_ptr<StreamSocket>& socket)
 {
     const auto host = ((LOOLWSD::isSSLEnabled() || 
LOOLWSD::isSSLTermination()) ? "wss://" : "ws://") + 
(LOOLWSD::ServerName.empty() ? request.getHost() : LOOLWSD::ServerName);
     const auto params = Poco::URI(request.getURI()).getQueryParameters();
diff --git a/wsd/FileServer.hpp b/wsd/FileServer.hpp
index b27526f3..3ba6803f 100644
--- a/wsd/FileServer.hpp
+++ b/wsd/FileServer.hpp
@@ -20,11 +20,13 @@ class FileServerRequestHandler
 {
     static std::string getRequestPathname(const Poco::Net::HTTPRequest& 
request);
 
-    static void preprocessFile(const Poco::Net::HTTPRequest& request, 
Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket);
+    static void preprocessAndSendLoleafletHtml(const Poco::Net::HTTPRequest& 
request, Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& 
socket);
 
 public:
-    /// Evaluate if the cookie exists, and if not, ask for the credentials.
-    static bool isAdminLoggedIn(const Poco::Net::HTTPRequest& request, 
Poco::Net::HTTPResponse& response);
+    /// If valid cookies exists in request, log the admin in (returns true)
+    /// If no cookie exist check the credentials, set the cookie and log the 
admin in
+    /// In case no valid cookie exists or invalid or no credentials exist, 
return false
+    static bool tryAdminLogin(const Poco::Net::HTTPRequest& request, 
Poco::Net::HTTPResponse& response);
 
     static void handleRequest(const Poco::Net::HTTPRequest& request, 
Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket);
 };
commit 9095294e6adf9e6041de3cd1adcf91842d65ad91
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Fri Apr 7 11:58:27 2017 +0530

    security: Mention X-Frame-Options too for ie/edge
    
    ie/edge ignores frame-ancestor directive of CSP (yet). Mention 
X-Frame-Options
    for them. Similary, X-Frame-Options allow-from attribute is not
    supported by Chrome:
    (see https://bugs.chromium.org/p/chromium/issues/detail?id=511521)
    In that case, we already have frame-ancestor CSP directive for it.
    
    Change-Id: Ide00c4db88c438de5e9c679360b3da6f4eb4a1be

diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index cedd8259..29be66f6 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -341,7 +341,8 @@ void FileServerRequestHandler::preprocessFile(const 
HTTPRequest& request, Poco::
         << "Cache-Control:max-age=11059200\r\n"
         << "ETag: \"" LOOLWSD_VERSION_HASH "\"\r\n"
         << "Content-Length: " << preprocess.size() << "\r\n"
-        << "Content-Type: " << mimeType << "\r\n";
+        << "Content-Type: " << mimeType << "\r\n"
+        << "X-Frame-Options: allow-from " << wopiDomain << "\r\n";
 
     if (!wopiDomain.empty())
         oss << "Content-Security-Policy: frame-ancestors " << wopiDomain << 
"\r\n";
commit a2b8bb83f87220719414da3d70c7d1f38b481d74
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Fri Apr 7 11:53:43 2017 +0530

    security: CSP: Add frame-ancestor directive
    
    Block embedding LibreOffice Online is frames of different origin.
    
    Change-Id: If3e04a0704e42853dc757b4be1f30fc22b8b33e4

diff --git a/wsd/FileServer.cpp b/wsd/FileServer.cpp
index 0f987579..cedd8259 100644
--- a/wsd/FileServer.cpp
+++ b/wsd/FileServer.cpp
@@ -257,6 +257,17 @@ std::string 
FileServerRequestHandler::getRequestPathname(const HTTPRequest& requ
 void FileServerRequestHandler::preprocessFile(const HTTPRequest& request, 
Poco::MemoryInputStream& message, const std::shared_ptr<StreamSocket>& socket)
 {
     const auto host = ((LOOLWSD::isSSLEnabled() || 
LOOLWSD::isSSLTermination()) ? "wss://" : "ws://") + 
(LOOLWSD::ServerName.empty() ? request.getHost() : LOOLWSD::ServerName);
+    const auto params = Poco::URI(request.getURI()).getQueryParameters();
+    std::string wopiDomain;
+    for (const auto& param : params)
+    {
+        if (param.first == "WOPISrc")
+        {
+            std::string wopiHost;
+            Poco::URI::decode(param.second, wopiHost);
+            wopiDomain = Poco::URI(wopiHost).getScheme() + "://" + 
Poco::URI(wopiHost).getHost();
+        }
+    }
     const auto path = Poco::Path(LOOLWSD::FileServerRoot, 
getRequestPathname(request));
     LOG_DBG("Preprocessing file: " << path.toString());
 
@@ -330,8 +341,12 @@ void FileServerRequestHandler::preprocessFile(const 
HTTPRequest& request, Poco::
         << "Cache-Control:max-age=11059200\r\n"
         << "ETag: \"" LOOLWSD_VERSION_HASH "\"\r\n"
         << "Content-Length: " << preprocess.size() << "\r\n"
-        << "Content-Type: " << mimeType << "\r\n"
-        << "\r\n"
+        << "Content-Type: " << mimeType << "\r\n";
+
+    if (!wopiDomain.empty())
+        oss << "Content-Security-Policy: frame-ancestors " << wopiDomain << 
"\r\n";
+
+    oss << "\r\n"
         << preprocess;
 
     socket->send(oss.str());
commit 4050c7786cc833a1cad7023ef19296fb36fd529a
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Fri Apr 7 00:49:31 2017 +0530

    Stop using inline event handlers wherever possible
    
    Unfortunately, our dependencies (various plugins etc.) still make heavy
    use of inline event handlers, so not possible get rid of all of them in
    our bundled js. This is the reason we still have to use 'unsafe-inline'
    in our CSP.
    
    Change-Id: I519dec0834606ab3c56e090c882a93160ddcb52c

diff --git a/loleaflet/dist/loleaflet.html b/loleaflet/dist/loleaflet.html
index cb0f73c5..05a49e07 100644
--- a/loleaflet/dist/loleaflet.html
+++ b/loleaflet/dist/loleaflet.html
@@ -57,7 +57,7 @@
        <div id="formulabar"></div>
        <div id="toolbar-up-more"></div>
       </div>
-      <input id="insertgraphic" type="file" onchange="onInsertFile()" 
style="position: fixed; top: -100em">
+      <input id="insertgraphic" type="file" style="position: fixed; top: 
-100em">
     </div>
 
     <div id="spreadsheet-row-column-frame">
diff --git a/loleaflet/dist/toolbar/toolbar.js 
b/loleaflet/dist/toolbar/toolbar.js
index d779235c..63e3fd82 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -616,11 +616,15 @@ $(function () {
                        {type: 'button',  id: 'function',  img: 'equal', hint: 
_('Function')},
                        {type: 'button', hidden: true, id: 'cancelformula',  
img: 'cancel', hint: _('Cancel')},
                        {type: 'button', hidden: true, id: 'acceptformula',  
img: 'accepttrackedchanges', hint: _('Accept')},
-                       {type: 'html', id: 'formula', html: '<input 
id="formulaInput" onkeyup="onFormulaInput(event)"' +
-                        'onblur="onFormulaBarBlur()" 
onfocus="onFormulaBarFocus()" type=text>'}
+                       {type: 'html', id: 'formula', html: '<input 
id="formulaInput" type="text">'}
                ],
                onClick: function (e) {
                        onClick(e.target);
+               },
+               onRefresh: function(e) {
+                       $('#formulaInput').off('keyup', 
onFormulaInput).on('keyup', onFormulaInput);
+                       $('#formulaInput').off('blur', 
onFormulaBarBlur).on('blur', onFormulaBarBlur);
+                       $('#formulaInput').off('focus', 
onFormulaBarFocus).on('focus', onFormulaBarFocus);
                }
        });
        $('#spreadsheet-toolbar').w2toolbar({
@@ -657,7 +661,7 @@ $(function () {
                        {type: 'html',  id: 'search',
                         html: '<div style="padding: 3px 10px;" 
class="loleaflet-font">' +
                         ' ' + _('Search:') +
-                        '    <input size="10" id="search-input" 
oninput="onSearch(event)" onkeypress="onSearchKeyPress(event)" ' +
+                        '    <input size="10" id="search-input"' +
                         'style="padding: 3px; border-radius: 2px; border: 1px 
solid silver"/>' +
                         '</div>'
                        },
@@ -686,6 +690,8 @@ $(function () {
                },
                onRefresh: function(e) {
                        $('#tb_toolbar-down_item_userlist 
.w2ui-tb-caption').addClass('loleaflet-font');
+                       $('#search-input').off('input', onSearch).on('input', 
onSearch);
+                       $('#search-input').off('keypress', 
onSearchKeyPress).on('keypress', onSearchKeyPress);
                }
        });
 });
@@ -1617,4 +1623,7 @@ $(document).ready(function() {
        if (closebutton) {
                toolbar.show('close');
        }
+
+       // Attach insert file action
+       $('#insertgraphic').on('change', onInsertFile);
 });
commit b8605ec4c93737b47f1e28e13282ea738ef931a4
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Fri Apr 7 00:38:02 2017 +0530

    Bin unused function
    
    Change-Id: I57bc98cd382081e776e1ed58da095a404834b431

diff --git a/loleaflet/dist/toolbar/toolbar.js 
b/loleaflet/dist/toolbar/toolbar.js
index 0650ec16..d779235c 100644
--- a/loleaflet/dist/toolbar/toolbar.js
+++ b/loleaflet/dist/toolbar/toolbar.js
@@ -758,12 +758,6 @@ function onSearchKeyPress(e) {
        }
 }
 
-function onSaveAs(e) {
-       if (e !== false) {
-               map.saveAs(e.url, e.format, e.options);
-       }
-}
-
 function sortFontSizes() {
        var oldVal = $('.fontsizes-select').val();
        var selectList = $('.fontsizes-select option');
commit c80b6914d2bf7aa1b9fbbd4536ab45a0f73771eb
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Thu Apr 6 23:52:32 2017 +0530

    Bin this unused inline style
    
    Change-Id: Ib4ae0cde13acea0e04526cda925f3d4528bb6605

diff --git a/loleaflet/dist/loleaflet.html b/loleaflet/dist/loleaflet.html
index 0772bee1..cb0f73c5 100644
--- a/loleaflet/dist/loleaflet.html
+++ b/loleaflet/dist/loleaflet.html
@@ -30,7 +30,6 @@
 <link rel="localizations" 
href="/loleaflet/%VERSION%/l10n/styles-localizations.json" 
type="application/vnd.oftn.l10n+json" />
 <link rel="localizations" 
href="/loleaflet/%VERSION%/l10n/uno-localizations.json" 
type="application/vnd.oftn.l10n+json" />
 <link rel="localizations" 
href="/loleaflet/%VERSION%/l10n/help-localizations.json" 
type="application/vnd.oftn.l10n+json"/>
-<style type="text/css"></style></head>
   <body>
     <!--The "controls" div holds map controls such as the Zoom button and
         it's separated from the map in order to have the controls on the top
commit 79e9141ad9d6f74bb4bf868ad93b07488b3e5d15
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Thu Apr 6 23:49:56 2017 +0530

    loleaflet: Add Content Security Policy
    
    Change-Id: I450e0c9fb24d114af35ba9c503d3940ab30a4f4e

diff --git a/loleaflet/dist/loleaflet.html b/loleaflet/dist/loleaflet.html
index d8de0bb3..0772bee1 100644
--- a/loleaflet/dist/loleaflet.html
+++ b/loleaflet/dist/loleaflet.html
@@ -3,7 +3,13 @@
 <html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
 <title>Online Editor</title>
 <meta charset="utf-8">
-
+<meta http-equiv="Content-Security-Policy" content="default-src 'none';
+                                                   frame-src blob:;
+                                                   connect-src 'self' %HOST%;
+                                                   script-src 'self' 
'unsafe-inline';
+                                                   style-src 'self' 
'unsafe-inline';
+                                                   font-src 'self' data:;
+                                                   img-src 'self' data:;">
 <meta name="viewport" content="width=device-width, initial-scale=1.0">
 
 <script>
commit 6e85a2a142e12dd97dc789f33f042da6c58b3249
Author: Miklos Vajna <vmik...@collabora.co.uk>
Date:   Fri Apr 7 09:39:52 2017 +0200

    net: Socket::assertCorrectThread() can be non-virtual
    
    None of the subclasses override it, and if they would, it would be
    problematic, since e.g. the StreamSocket dtor calls it (and virtual
    calls during dtors are a problem).
    
    Change-Id: Ie0891349808a81539078fd1f2d95a55a4ce5107a

diff --git a/net/Socket.hpp b/net/Socket.hpp
index c726f337..3b44e803 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -193,7 +193,7 @@ public:
     }
 
     /// Asserts in the debug builds, otherwise just logs.
-    virtual void assertCorrectThread()
+    void assertCorrectThread()
     {
         if (InhibitThreadChecks)
             return;
commit 48cbf2afb6393d758e1ee6a46b3fdde06274c508
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Fri Apr 7 01:54:53 2017 -0400

    wsd: be more flexible with the svg export test
    
    Change-Id: I4ff645605b911bb8a872894bec9eeed0eff1ae3c
    Reviewed-on: https://gerrit.libreoffice.org/36246
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 33a13533..033c5bd1 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -1181,9 +1181,9 @@ void HTTPWSTest::testSlideShow()
         (void)rs;
         // Some setups render differently; recognize these two valid output 
sizes for now.
         // Seems LO generates different svg content, even though visually 
identical.
-        // Current known sizes: 434748, 451329, 467345.
-        CPPUNIT_ASSERT(responseSVG.getContentLength() >= 
std::streamsize(434748) &&
-                       responseSVG.getContentLength() <= 
std::streamsize(467345));
+        // Current known sizes: 434748, 451329, 467345, 468653.
+        CPPUNIT_ASSERT(responseSVG.getContentLength() >= 
std::streamsize(430000) &&
+                       responseSVG.getContentLength() <= 
std::streamsize(470000));
     }
     catch (const Poco::Exception& exc)
     {
commit 89bb368185b935f26ada0962c562447100c3bfcb
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Fri Apr 7 01:29:39 2017 -0400

    wsd: re-enable some passing tests
    
    Change-Id: Ifbbc77d6603e378ab18cb1b92bbca71c76df477d
    Reviewed-on: https://gerrit.libreoffice.org/36245
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 0a629ceb..33a13533 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -58,7 +58,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST_SUITE(HTTPWSTest);
 
     CPPUNIT_TEST(testBadRequest);
-    // FIXME CPPUNIT_TEST(testHandshake);
+    CPPUNIT_TEST(testHandshake);
     CPPUNIT_TEST(testCloseAfterClose);
     CPPUNIT_TEST(testConnectNoLoad);
     CPPUNIT_TEST(testLoadSimple);
@@ -99,7 +99,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     // FIXME CPPUNIT_TEST(testOptimalResize);
     CPPUNIT_TEST(testInvalidateViewCursor);
     CPPUNIT_TEST(testViewCursorVisible);
-    // FIXME CPPUNIT_TEST(testCellViewCursor);
+    CPPUNIT_TEST(testCellViewCursor);
     CPPUNIT_TEST(testGraphicViewSelectionWriter);
     CPPUNIT_TEST(testGraphicViewSelectionCalc);
     CPPUNIT_TEST(testGraphicViewSelectionImpress);
commit a4f9cfec0afbadf3eaa17498eb1262d1fe65302f
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Fri Apr 7 00:32:01 2017 -0400

    wsd: fix testSlideShow
    
    Change-Id: I2acf7f0ee509f193b0be46af6ba4363b8aecb98f
    Reviewed-on: https://gerrit.libreoffice.org/36244
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 5b364fd2..0a629ceb 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -31,6 +31,7 @@
 #include <Poco/Net/Socket.h>
 #include <Poco/Path.h>
 #include <Poco/RegularExpression.h>
+#include <Poco/StreamCopier.h>
 #include <Poco/StringTokenizer.h>
 #include <Poco/URI.h>
 
@@ -82,7 +83,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testPasswordProtectedOOXMLDocument);
     CPPUNIT_TEST(testPasswordProtectedBinaryMSOfficeDocument);
     CPPUNIT_TEST(testInsertDelete);
-    // FIXME CPPUNIT_TEST(testSlideShow);
+    CPPUNIT_TEST(testSlideShow);
     CPPUNIT_TEST(testInactiveClient);
     CPPUNIT_TEST(testMaxColumn);
     CPPUNIT_TEST(testMaxRow);
@@ -1170,12 +1171,19 @@ void HTTPWSTest::testSlideShow()
         session->sendRequest(requestSVG);
 
         Poco::Net::HTTPResponse responseSVG;
-        session->receiveResponse(responseSVG);
+        std::istream& rs = session->receiveResponse(responseSVG);
         CPPUNIT_ASSERT_EQUAL(Poco::Net::HTTPResponse::HTTP_OK, 
responseSVG.getStatus());
         CPPUNIT_ASSERT_EQUAL(std::string("image/svg+xml"), 
responseSVG.getContentType());
+        std::cerr << "SVG file size: " << responseSVG.getContentLength() << 
std::endl;
+        // std::ofstream ofs("/tmp/slide.svg");
+        // Poco::StreamCopier::copyStream(rs, ofs);
+        // ofs.close();
+        (void)rs;
         // Some setups render differently; recognize these two valid output 
sizes for now.
-        CPPUNIT_ASSERT(responseSVG.getContentLength() == 
std::streamsize(451329) ||
-                       responseSVG.getContentLength() == 
std::streamsize(467345));
+        // Seems LO generates different svg content, even though visually 
identical.
+        // Current known sizes: 434748, 451329, 467345.
+        CPPUNIT_ASSERT(responseSVG.getContentLength() >= 
std::streamsize(434748) &&
+                       responseSVG.getContentLength() <= 
std::streamsize(467345));
     }
     catch (const Poco::Exception& exc)
     {
commit c2be44aee0b8612276a15cd8db21a4736e4ff92c
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 23:38:54 2017 -0400

    wsd: merge DocumentBroker poll exit conditions
    
    These conditions must be checked together. Otherwise we might
    set _stop prematurely.
    
    Change-Id: I3de0d2b3833959593315669ad245f94c1243f7f7
    Reviewed-on: https://gerrit.libreoffice.org/36242
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index eb53f195..f7bbd9f1 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -234,19 +234,14 @@ void DocumentBroker::pollThread()
             last30SecCheckTime = std::chrono::steady_clock::now();
         }
 
-        // If all sessions have been removed, no reason to linger.
-        if (_sessions.empty() && 
std::chrono::duration_cast<std::chrono::milliseconds>
-            (std::chrono::steady_clock::now() - _lastSaveRequestTime).count() 
> COMMAND_TIMEOUT_MS)
-        {
-            LOG_INF("No more sessions in doc [" << _docKey << "]. 
Terminating.");
-            _stop = true;
-        }
+        const bool notSaving = 
(std::chrono::duration_cast<std::chrono::milliseconds>
+                                (std::chrono::steady_clock::now() - 
_lastSaveRequestTime).count() > COMMAND_TIMEOUT_MS);
 
         // Remove idle documents after 1 hour.
-        const bool idle = getIdleTimeSecs() >= 3600;
+        const bool idle = (getIdleTimeSecs() >= 3600);
 
-        // Cleanup used and dead entries.
-        if ((isLoaded() || _markToDestroy) &&
+        // If all sessions have been removed, no reason to linger.
+        if ((isLoaded() || _markToDestroy) && notSaving &&
             (_sessions.empty() || !isAlive() || idle))
         {
             LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
commit 2456e4ffe8a60acb0e186d1b258c342a434dad28
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 21:50:29 2017 -0400

    wsd: lower the max number of test docs and connections
    
    Tests should have sensible limits so they don't
    go overboard and fail needlessly causing noise.
    
    Change-Id: Idd556c348cc0e97e38c710fdbf76fe20c76d8f9b
    Reviewed-on: https://gerrit.libreoffice.org/36241
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/test/httpwserror.cpp b/test/httpwserror.cpp
index 254b2982..ff417f25 100644
--- a/test/httpwserror.cpp
+++ b/test/httpwserror.cpp
@@ -117,7 +117,7 @@ void HTTPWSError::testMaxDocuments()
     static_assert(MAX_DOCUMENTS >= 2, "MAX_DOCUMENTS must be at least 2");
     const auto testname = "maxDocuments ";
 
-    if (MAX_DOCUMENTS > 200)
+    if (MAX_DOCUMENTS > 20)
     {
         std::cerr << "Skipping " << testname << "test since MAX_DOCUMENTS (" 
<< MAX_DOCUMENTS
                   << ") is too high to test. Set to a more sensible number, 
ideally a dozen or so." << std::endl;
@@ -168,7 +168,7 @@ void HTTPWSError::testMaxConnections()
     static_assert(MAX_CONNECTIONS >= 3, "MAX_CONNECTIONS must be at least 3");
     const auto testname = "maxConnections ";
 
-    if (MAX_CONNECTIONS > 300)
+    if (MAX_CONNECTIONS > 40)
     {
         std::cerr << "Skipping " << testname << "test since MAX_CONNECTION (" 
<< MAX_CONNECTIONS
                   << ") is too high to test. Set to a more sensible number, 
ideally a dozen or so." << std::endl;
@@ -223,7 +223,7 @@ void HTTPWSError::testMaxViews()
     static_assert(MAX_CONNECTIONS >= 3, "MAX_CONNECTIONS must be at least 3");
     const auto testname = "maxViews ";
 
-    if (MAX_CONNECTIONS > 200)
+    if (MAX_CONNECTIONS > 40)
     {
         std::cerr << "Skipping " << testname << "test since MAX_CONNECTION (" 
<< MAX_CONNECTIONS
                   << ") is too high to test. Set to a more sensible number, 
ideally a dozen or so." << std::endl;
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 27b698d7..5b364fd2 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -2201,7 +2201,7 @@ void HTTPWSTest::testEachView(const std::string& doc, 
const std::string& type,
         // Connect and load 0..N Views, where N<=limit
         std::vector<std::shared_ptr<LOOLWebSocket>> views;
         static_assert(MAX_DOCUMENTS >= 2, "MAX_DOCUMENTS must be at least 2");
-        const auto limit = std::max(2, MAX_DOCUMENTS - 1); // +1 connection 
above
+        const auto limit = std::min(4, MAX_DOCUMENTS - 1); // +1 connection 
above
         for (itView = 0; itView < limit; ++itView)
         {
             views.emplace_back(loadDocAndGetSocket(_uri, documentURL, 
Poco::format(view, itView)));
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 439ab67e..0fe28328 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -747,7 +747,8 @@ void LOOLWSD::initialize(Application& self)
 
     // Log the connection and document limits.
     static_assert(MAX_CONNECTIONS >= 3, "MAX_CONNECTIONS must be at least 3");
-    static_assert(MAX_DOCUMENTS > 0 && MAX_DOCUMENTS <= MAX_CONNECTIONS, 
"MAX_DOCUMENTS must be positive and no more than MAX_CONNECTIONS");
+    static_assert(MAX_DOCUMENTS > 0 && MAX_DOCUMENTS <= MAX_CONNECTIONS,
+                  "max_documents must be positive and no more than 
max_connections");
     LOG_INF("Maximum concurrent open Documents limit: " << MAX_DOCUMENTS);
     LOG_INF("Maximum concurrent client Connections limit: " << 
MAX_CONNECTIONS);
 
commit 03979385637a43de929d03f3d024270fc85b21b8
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 13:49:44 2017 -0400

    wsd: don't take reference to session member being destroyed
    
    Change-Id: I0074f4557018feb47a7a2a95a3fca238407a0023
    Reviewed-on: https://gerrit.libreoffice.org/36227
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 9550dfd2..eb53f195 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -516,7 +516,7 @@ bool DocumentBroker::load(const 
std::shared_ptr<ClientSession>& session, const s
     return true;
 }
 
-bool DocumentBroker::saveToStorage(const std::string& sessionId,
+bool DocumentBroker::saveToStorage(const std::string sessionId,
                                    bool success, const std::string& result)
 {
     assertCorrectThread();
@@ -817,7 +817,7 @@ size_t DocumentBroker::addSession(const 
std::shared_ptr<ClientSession>& session)
     return count;
 }
 
-size_t DocumentBroker::removeSession(const std::string& id, bool destroyIfLast)
+size_t DocumentBroker::removeSession(const std::string id, bool destroyIfLast)
 {
     assertCorrectThread();
 
@@ -840,7 +840,7 @@ size_t DocumentBroker::removeSession(const std::string& id, 
bool destroyIfLast)
     return _sessions.size();
 }
 
-size_t DocumentBroker::removeSessionInternal(const std::string& id)
+size_t DocumentBroker::removeSessionInternal(const std::string id)
 {
     assertCorrectThread();
     try
@@ -853,6 +853,10 @@ size_t DocumentBroker::removeSessionInternal(const 
std::string& id)
             LOOLWSD::dumpEndSessionTrace(getJailId(), id, _uriOrig);
 
             const auto readonly = (it->second ? it->second->isReadOnly() : 
false);
+
+            //FIXME: We might be called from the session we are removing,
+            //FIXME: and if this is the last/only reference, we destroy it.
+            //FIXME: Should flag and remove from the poll thread.
             _sessions.erase(it);
 
             const auto count = _sessions.size();
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index c8758ba9..88fa2b6e 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -233,7 +233,7 @@ public:
     void setLoaded();
 
     /// Save the document to Storage if it needs persisting.
-    bool saveToStorage(const std::string& sesionId, bool success, const 
std::string& result = "");
+    bool saveToStorage(const std::string sesionId, bool success, const 
std::string& result = "");
     bool isModified() const { return _isModified; }
     void setModified(const bool value);
 
@@ -265,7 +265,7 @@ public:
     size_t addSession(const std::shared_ptr<ClientSession>& session);
 
     /// Removes a session by ID. Returns the new number of sessions.
-    size_t removeSession(const std::string& id, bool destroyIfLast = false);
+    size_t removeSession(const std::string id, bool destroyIfLast = false);
 
     /// Add a callback to be invoked in our polling thread.
     void addCallback(SocketPoll::CallbackFn fn);
@@ -342,7 +342,7 @@ private:
     bool saveToStorageInternal(const std::string& sesionId, bool success, 
const std::string& result = "");
 
     /// Removes a session by ID. Returns the new number of sessions.
-    size_t removeSessionInternal(const std::string& id);
+    size_t removeSessionInternal(const std::string id);
 
     /// Forward a message from child session to its respective client session.
     bool forwardToClient(const std::shared_ptr<Message>& payload);
commit 4222ae73bca7ce4d985c42fce3564f37079575ba
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 13:49:22 2017 -0400

    wsd: kill DocumentBroker::getSessionsCount
    
    Change-Id: Icd3229fe9b7d2f17a0e8a8f955c41ead8bca98c7
    Reviewed-on: https://gerrit.libreoffice.org/36226
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0ee99738..9550dfd2 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -247,7 +247,7 @@ void DocumentBroker::pollThread()
 
         // Cleanup used and dead entries.
         if ((isLoaded() || _markToDestroy) &&
-            (getSessionsCount() == 0 || !isAlive() || idle))
+            (_sessions.empty() || !isAlive() || idle))
         {
             LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
                     " DocumentBroker for docKey [" << getDocKey() << "].");
@@ -1369,7 +1369,7 @@ void DocumentBroker::dumpState(std::ostream& os)
     os << "\n  jailed uri: " << _uriJailed.toString();
     os << "\n  doc key: " << _docKey;
     os << "\n  doc id: " << _docId;
-    os << "\n  num sessions: " << getSessionsCount();
+    os << "\n  num sessions: " << _sessions.size();
     os << "\n  last editable?: " << _lastEditableSession;
     std::time_t t = std::chrono::system_clock::to_time_t(
         std::chrono::system_clock::now()
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index fd8d20e8..c8758ba9 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -251,11 +251,6 @@ public:
     const std::string& getFilename() const { return _filename; };
     TileCache& tileCache() { return *_tileCache; }
     bool isAlive() const;
-    size_t getSessionsCount() const
-    {
-        Util::assertIsLocked(_mutex);
-        return _sessions.size();
-    }
 
     /// Are we running in either shutdown, or the polling thread.
     /// Asserts in the debug builds, otherwise just logs.
commit 47f2f6993a3e9d66c4ccb7562f32d4741b746e57
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Thu Apr 6 17:58:41 2017 +0100

    Let the DocBroker thread clean itself up and expire.
    
    (cherry picked from commit 2e372b70b32d4e052458547daa229c537442774f)
    
    Change-Id: I5835c83f44ef770fa6ccd2418fc6ca73e17694e4
    Reviewed-on: https://gerrit.libreoffice.org/36225
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a5a995df..0ee99738 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -241,6 +241,18 @@ void DocumentBroker::pollThread()
             LOG_INF("No more sessions in doc [" << _docKey << "]. 
Terminating.");
             _stop = true;
         }
+
+        // Remove idle documents after 1 hour.
+        const bool idle = getIdleTimeSecs() >= 3600;
+
+        // Cleanup used and dead entries.
+        if ((isLoaded() || _markToDestroy) &&
+            (getSessionsCount() == 0 || !isAlive() || idle))
+        {
+            LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
+                    " DocumentBroker for docKey [" << getDocKey() << "].");
+            _stop = true;
+        }
     }
 
     LOG_INF("Finished polling doc [" << _docKey << "]. stop: " << _stop << ", 
continuePolling: " <<
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index d05437e9..fd8d20e8 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -304,7 +304,7 @@ public:
     void handleTileCombinedResponse(const std::vector<char>& payload);
 
     void destroyIfLastEditor(const std::string& id);
-    bool isMarkedToDestroy() const { return _markToDestroy; }
+    bool isMarkedToDestroy() const { return _markToDestroy || _stop; }
 
     bool handleInput(const std::vector<char>& payload);
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index e5d4cb68..439ab67e 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -235,33 +235,15 @@ void cleanupDocBrokers()
     {
         auto docBroker = it->second;
 
-        // If document busy at the moment, cleanup later.
-        auto lock = docBroker->getDeferredLock();
-        if (lock.try_lock())
+        // Remove only when not alive.
+        if (!docBroker->isAlive())
         {
-            // Remove idle documents after 1 hour.
-            const bool idle = (docBroker->getIdleTimeSecs() >= 3600);
-
-            // Cleanup used and dead entries.
-            if ((docBroker->isLoaded() || docBroker->isMarkedToDestroy()) &&
-                (docBroker->getSessionsCount() == 0 || !docBroker->isAlive() 
|| idle))
-            {
-                LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
-                        " DocumentBroker for docKey [" << it->first << "].");
-                docBroker->stop();
-
-                // Remove only when not alive.
-                if (!docBroker->isAlive())
-                {
-                    LOG_INF("Removing " << (idle ? "idle" : "dead") <<
-                            " DocumentBroker for docKey [" << it->first << 
"].");
-                    it = DocBrokers.erase(it);
-                    continue;
-                }
-            }
+            LOG_INF("Removing DocumentBroker for docKey [" << it->first << 
"].");
+            it = DocBrokers.erase(it);
+            continue;
+        } else {
+            ++it;
         }
-
-        ++it;
     }
 
     if (count != DocBrokers.size())
commit 6e18c66f5e0acf737bdc91f61df3a23651c20555
Author: Pranav Kant <pran...@collabora.co.uk>
Date:   Thu Apr 6 16:59:44 2017 +0530

    loleaflet: Exit early for invalid commandvalues
    
    LO core doesn't output any change tracking information for spreadsheets
    which results in sending an empty 'commandvalues: ' message to
    loleaflet. While the actual fix for it would go in LO core, lets handle
    this empty case for now in loleaflet, so that we don't throw any runtime
    JS exceptions.
    
    Change-Id: Id2b66b8e7f1c87b074d3e72168e0ca3c3c0d345d

diff --git a/loleaflet/src/layer/tile/CalcTileLayer.js 
b/loleaflet/src/layer/tile/CalcTileLayer.js
index 1fa97c45..9fba31fc 100644
--- a/loleaflet/src/layer/tile/CalcTileLayer.js
+++ b/loleaflet/src/layer/tile/CalcTileLayer.js
@@ -417,7 +417,11 @@ L.CalcTileLayer = L.TileLayer.extend({
        },
 
        _onCommandValuesMsg: function (textMsg) {
-               var values = 
JSON.parse(textMsg.substring(textMsg.indexOf('{')));
+               var jsonIdx = textMsg.indexOf('{');
+               if (jsonIdx === -1)
+                       return;
+
+               var values = JSON.parse(textMsg.substring(jsonIdx));
                if (!values) {
                        return;
                }
commit 33c268abfbca025ff738ba98124ce8af8733f98d
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 03:32:21 2017 -0400

    wsd: don't invoke poll handler when invalidated
    
    Only during shutdown do we expect a callback
    to invalidate the pollSockets, but if it happens
    we shouldn't invoke the poll handlers.
    
    Change-Id: I2f56da19aec2f04cc871bd4eae1f93da110e0eb9
    Reviewed-on: https://gerrit.libreoffice.org/36189
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/net/Socket.hpp b/net/Socket.hpp
index bb08dca1..c726f337 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -390,6 +390,10 @@ public:
             }
         }
 
+        // This should only happen when we're stopping.
+        if (_pollSockets.size() != size)
+            return;
+
         // Fire the poll callbacks and remove dead fds.
         std::chrono::steady_clock::time_point newNow =
             std::chrono::steady_clock::now();
commit d7152cb3b52f859014c13ed0b49e7f221b973ce5
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 02:56:21 2017 -0400

    wsd: accomodate accept_poll shutdown
    
    When shutting down accept_poll from
    main, we can't remove sockets or cleanup.
    That work needs to be done fro within accept_poll's
    thread. This is different from when DocBroker's
    poll needs to cleanup its own sockets before
    it exists.
    
    So we split the stop and removeSockets so they
    can each be called in the proper way.
    
    For accept_poll and others that joinThread
    we queue a callback to cleanup before stopping.
    
    Change-Id: If780d6a97ac0fc6da6897f895d5b4dda443f9e73
    Reviewed-on: https://gerrit.libreoffice.org/36186
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/net/Socket.cpp b/net/Socket.cpp
index d42f0730..662ddb93 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -96,6 +96,7 @@ void SocketPoll::startThread()
 
 void SocketPoll::joinThread()
 {
+    addCallback([this](){ removeSockets(); });
     stop();
     if (_threadStarted && _thread.joinable())
     {
diff --git a/net/Socket.hpp b/net/Socket.hpp
index cd1ea6cc..bb08dca1 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -264,9 +264,14 @@ public:
     /// Stop the polling thread.
     void stop()
     {
-        LOG_DBG("Stopping " << _name << " and removing all sockets.");
+        LOG_DBG("Stopping " << _name << ".");
         _stop = true;
+        wakeup();
+    }
 
+    void removeSockets()
+    {
+        LOG_DBG("Removing all sockets from " << _name << ".");
         assert(socket);
         assertCorrectThread();
 
@@ -280,8 +285,6 @@ public:
 
             _pollSockets.pop_back();
         }
-
-        wakeup();
     }
 
     bool isAlive() const { return _threadStarted && !_threadFinished; }
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 74f730b6..a5a995df 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -266,6 +266,7 @@ void DocumentBroker::pollThread()
 
     // Stop to mark it done and cleanup.
     _poll->stop();
+    _poll->removeSockets();
 
     // Async cleanup.
     LOOLWSD::doHousekeeping();
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index ac721294..e5d4cb68 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2474,9 +2474,6 @@ int LOOLWSD::innerMain()
     LOG_INF("Stopping server socket listening. ShutdownFlag: " <<
             ShutdownRequestFlag << ", TerminationFlag: " << TerminationFlag);
 
-    // Disable thread checking - we'll now cleanup lots of things if we can
-    Socket::InhibitThreadChecks = true;
-
     // Wait until documents are saved and sessions closed.
     srv.stop();
 
@@ -2485,6 +2482,9 @@ int LOOLWSD::innerMain()
     for (auto& docBrokerIt : DocBrokers)
         docBrokerIt.second->joinThread();
 
+    // Disable thread checking - we'll now cleanup lots of things if we can
+    Socket::InhibitThreadChecks = true;
+
     DocBrokers.clear();
 
 #ifndef KIT_IN_PROCESS
commit 0de8557120721d6d80f39af5264c8f442e7b9313
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 02:29:25 2017 -0400

    wsd: inhibit thread checks sooner when shutting down
    
    LOOLWSDServer::stop() now removes the accept_poll
    socket, which will assertCorrectThread. So we need
    to disable checks before it.
    
    Change-Id: I3445610c1c48c2b4c23bcfcbc87e236b36d18c0b
    Reviewed-on: https://gerrit.libreoffice.org/36185
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index e5d4cb68..ac721294 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2474,6 +2474,9 @@ int LOOLWSD::innerMain()
     LOG_INF("Stopping server socket listening. ShutdownFlag: " <<
             ShutdownRequestFlag << ", TerminationFlag: " << TerminationFlag);
 
+    // Disable thread checking - we'll now cleanup lots of things if we can
+    Socket::InhibitThreadChecks = true;
+
     // Wait until documents are saved and sessions closed.
     srv.stop();
 
@@ -2482,9 +2485,6 @@ int LOOLWSD::innerMain()
     for (auto& docBrokerIt : DocBrokers)
         docBrokerIt.second->joinThread();
 
-    // Disable thread checking - we'll now cleanup lots of things if we can
-    Socket::InhibitThreadChecks = true;
-
     DocBrokers.clear();
 
 #ifndef KIT_IN_PROCESS
commit 9d2e2aae114e228596d5420f717ea5e387d531e9
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 01:59:20 2017 -0400

    wsd: leave the poll running so DocBroker can flush the sockets
    
    By stopping the poll we fail to notify the clients
    of the shutdown. Let the DocBroker poll thread
    take care of the poll stopping when it's ready.
    
    Change-Id: I2cb4c76da2722ce41a60fc1983b10dc8b18b4cab
    Reviewed-on: https://gerrit.libreoffice.org/36184
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0b7d9a6e..74f730b6 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1319,8 +1319,6 @@ void DocumentBroker::terminateChild(const std::string& 
closeReason, const bool r
         _childProcess->close(rude);
     }
 
-    // Stop the polling thread.
-    _poll->stop();
     _stop = true;
 }
 
commit 816b9933a8cf9731645b1274bba327c49109eb7a
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Thu Apr 6 01:32:39 2017 -0400

    wsd: remove sockets when stopping poll thread
    
    And assume correct thread if poll thread is
    not running (i.e. no race).
    
    Change-Id: I17958e682aba434ebb47fe0de199b9f530b54dee
    Reviewed-on: https://gerrit.libreoffice.org/36183
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/net/Socket.hpp b/net/Socket.hpp
index ad59bbfb..cd1ea6cc 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -264,7 +264,23 @@ public:
     /// Stop the polling thread.
     void stop()
     {
+        LOG_DBG("Stopping " << _name << " and removing all sockets.");
         _stop = true;
+
+        assert(socket);
+        assertCorrectThread();
+
+        while (!_pollSockets.empty())
+        {
+            const std::shared_ptr<Socket>& socket = _pollSockets.back();
+
+            LOG_DBG("Removing socket #" << socket->getFD() << " from " << 
_name);
+            socket->assertCorrectThread();
+            socket->setThreadOwner(std::thread::id(0));
+
+            _pollSockets.pop_back();
+        }
+
         wakeup();
     }
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 20fb8461..0b7d9a6e 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -264,6 +264,9 @@ void DocumentBroker::pollThread()
         _poll->poll(std::min(flushTimeoutMs - elapsedMs, POLL_TIMEOUT_MS / 5));
     }
 
+    // Stop to mark it done and cleanup.
+    _poll->stop();
+
     // Async cleanup.
     LOOLWSD::doHousekeeping();
 
commit e96ac2ece6cfc4cc3d806ee1d8c92b1bcaca415d
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 21:31:15 2017 +0100

    Always cleanup DocBrokers in the PrisonerPoll thread.
    
    This simplifies things, and keeps process management in one thread.
    Also - wakeup the DocumentBroker when we want to stop it.
    
    Change-Id: I597ba4b34719fc072a4b4ad3697442b5eebe5784
    Reviewed-on: https://gerrit.libreoffice.org/36182
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index d7c52de5..20fb8461 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -264,7 +264,7 @@ void DocumentBroker::pollThread()
         _poll->poll(std::min(flushTimeoutMs - elapsedMs, POLL_TIMEOUT_MS / 5));
     }
 
-    // Cleanup.
+    // Async cleanup.
     LOOLWSD::doHousekeeping();
 
     LOG_INF("Finished docBroker polling thread for docKey [" << _docKey << 
"].");
@@ -306,6 +306,12 @@ void DocumentBroker::joinThread()
     _poll->joinThread();
 }
 
+void DocumentBroker::stop()
+{
+    _stop = true;
+    _poll->wakeup();
+}
+
 bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const 
std::string& jailId)
 {
     assertCorrectThread();
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 0af441ff..d05437e9 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -222,8 +222,7 @@ public:
     void startThread();
 
     /// Flag for termination.
-    //TODO: Take reason to broadcast to clients.
-    void stop() { _stop = true; }
+    void stop();
 
     /// Thread safe termination of this broker if it has a lingering thread
     void joinThread();
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 08a20ce7..e5d4cb68 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -226,7 +226,7 @@ void alertAllUsersInternal(const std::string& msg)
 /// Remove dead and idle DocBrokers.
 /// The client of idle document should've greyed-out long ago.
 /// Returns true if at least one is removed.
-bool cleanupDocBrokers()
+void cleanupDocBrokers()
 {
     Util::assertIsLocked(DocBrokersMutex);
 
@@ -277,11 +277,7 @@ bool cleanupDocBrokers()
 
             LOG_END(logger);
         }
-
-        return true;
     }
-
-    return false;
 }
 
 /// Forks as many children as requested.
@@ -582,7 +578,7 @@ class PrisonerPoll : public TerminatingPoll {
 public:
     PrisonerPoll() : TerminatingPoll("prisoner_poll") {}
 
-    /// Check prisoners are still alive and balaned.
+    /// Check prisoners are still alive and balanced.
     void wakeupHook() override;
 };
 
@@ -1099,12 +1095,13 @@ bool LOOLWSD::checkAndRestoreForKit()
 #endif
 }
 
-void PrisonerPoll::wakeupHook()
+void LOOLWSD::doHousekeeping()
 {
-    LOOLWSD::doHousekeeping();
+    PrisonerPoll.wakeup();
 }
 
-void LOOLWSD::doHousekeeping()
+/// Really do the house-keeping
+void PrisonerPoll::wakeupHook()
 {
     if (!LOOLWSD::checkAndRestoreForKit())
     {
commit eb3572a8be51e03b8c52829045b5f0b1d8f0263b
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Wed Apr 5 22:35:22 2017 -0400

    wsd: move prisoner socket in the poll thread
    
    Change-Id: I4097da97d4485d98618604c039a4570efe52bc19
    Reviewed-on: https://gerrit.libreoffice.org/36181
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/net/Socket.hpp b/net/Socket.hpp
index d05c1099..ad59bbfb 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -459,8 +459,8 @@ public:
         assert(it != _pollSockets.end());
 
         _pollSockets.erase(it);
-        LOG_TRC("Release socket #" << socket->getFD() << " from " << _name <<
-                " leaving " << _pollSockets.size());
+        LOG_DBG("Removing socket #" << socket->getFD() << " (of " <<
+                _pollSockets.size() << ") from " << _name);
     }
 
     size_t getSocketCount() const
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index d2506f24..08a20ce7 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1458,6 +1458,8 @@ private:
                 return SocketHandlerInterface::SocketOwnership::UNCHANGED;
             }
 
+            in.clear();
+
             LOG_INF("New child [" << pid << "].");
 
             UnitWSD::get().newChild(*this);
@@ -1466,15 +1468,13 @@ private:
             _childProcess = child; // weak
             addNewChild(child);
 
-            // We no longer own this thread: FIXME.
+            // We no longer own this socket.
             socket->setThreadOwner(std::thread::id(0));
 
             // Remove from prisoner poll since there is no activity
             // until we attach the childProcess (with this socket)
             // to a docBroker, which will do the polling.
-            PrisonerPoll.releaseSocket(socket);
-
-            in.clear();
+            return SocketHandlerInterface::SocketOwnership::MOVED;
         }
         catch (const std::exception& exc)
         {
@@ -1848,7 +1848,6 @@ private:
 
                     cleanupDocBrokers();
 
-                    // FIXME: What if the same document is already open? Need 
a fake dockey here?
                     LOG_DBG("New DocumentBroker for docKey [" << docKey << 
"].");
                     DocBrokers.emplace(docKey, docBroker);
                     LOG_TRC("Have " << DocBrokers.size() << " DocBrokers after 
inserting [" << docKey << "].");
@@ -1865,7 +1864,7 @@ private:
                         // Make sure the thread is running before adding 
callback.
                         docBroker->startThread();
 
-                        // We no longer own this thread: FIXME.
+                        // We no longer own this socket.
                         socket->setThreadOwner(std::thread::id(0));
 
                         docBroker->addCallback([docBroker, socket, 
clientSession, format]()
@@ -2094,7 +2093,7 @@ private:
                 // Make sure the thread is running before adding callback.
                 docBroker->startThread();
 
-                // We no longer own this thread: FIXME.
+                // We no longer own this socket.
                 socket->setThreadOwner(std::thread::id(0));
 
                 docBroker->addCallback([docBroker, socket, clientSession]()
commit 1096caa1e6876f06ec5b14e31ec9975a4e4cb06e
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 21:48:35 2017 +0100

    Give up on doing thread checks during late shutdown.
    
    Change-Id: Icb600e4d734e075bec6c2cf6adbb2afd58c0d98b
    Reviewed-on: https://gerrit.libreoffice.org/36180
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 62879c1d..d2506f24 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2485,6 +2485,10 @@ int LOOLWSD::innerMain()
     LOG_INF("Cleaning up lingering documents.");
     for (auto& docBrokerIt : DocBrokers)
         docBrokerIt.second->joinThread();
+
+    // Disable thread checking - we'll now cleanup lots of things if we can
+    Socket::InhibitThreadChecks = true;
+
     DocBrokers.clear();
 
 #ifndef KIT_IN_PROCESS
commit 9c5ed63b1f4a84dba6f7b0db25c9798dc17d12e9
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 21:01:48 2017 +0100

    Set thread owner to zero earlier to avoid race.
    
    Stop-gap fix, while we come up with a nice API for transferring
    sockets between SocketPolls, and/or detaching them generally.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 300186a7..d05c1099 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -392,7 +392,6 @@ public:
             {
                 LOG_DBG("Removing socket #" << _pollFds[i].fd << " (of " <<
                         _pollSockets.size() << ") from " << _name);
-                _pollSockets[i]->setThreadOwner(std::thread::id(0));
                 _pollSockets.erase(_pollSockets.begin() + i);
             }
         }
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 408d49d4..62879c1d 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1466,6 +1466,9 @@ private:
             _childProcess = child; // weak
             addNewChild(child);
 
+            // We no longer own this thread: FIXME.
+            socket->setThreadOwner(std::thread::id(0));
+
             // Remove from prisoner poll since there is no activity
             // until we attach the childProcess (with this socket)
             // to a docBroker, which will do the polling.
@@ -1862,6 +1865,9 @@ private:
                         // Make sure the thread is running before adding 
callback.
                         docBroker->startThread();
 
+                        // We no longer own this thread: FIXME.
+                        socket->setThreadOwner(std::thread::id(0));
+
                         docBroker->addCallback([docBroker, socket, 
clientSession, format]()
                         {
                             clientSession->setSaveAsSocket(socket);
@@ -2088,6 +2094,9 @@ private:
                 // Make sure the thread is running before adding callback.
                 docBroker->startThread();
 
+                // We no longer own this thread: FIXME.
+                socket->setThreadOwner(std::thread::id(0));
+
                 docBroker->addCallback([docBroker, socket, clientSession]()
                 {
                     // Set the ClientSession to handle Socket events.
commit 5cd3f1119aab84e48565d3dfa3a9377e0f5f4f2a
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 18:06:58 2017 +0100

    Remove redundant structure, include, and _stop members.

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index d70d18c8..7ad8df31 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -37,8 +37,7 @@ ClientSession::ClientSession(const std::string& id,
     _docBroker(docBroker),
     _uriPublic(uriPublic),
     _isDocumentOwner(false),
-    _isAttached(false),
-    _stop(false)
+    _isAttached(false)
 {
     const size_t curConnections = ++LOOLWSD::NumConnections;
     LOG_INF("ClientSession ctor [" << getName() << "], current number of 
connections: " << curConnections);
@@ -48,8 +47,6 @@ ClientSession::~ClientSession()
 {
     const size_t curConnections = --LOOLWSD::NumConnections;
     LOG_INF("~ClientSession dtor [" << getName() << "], current number of 
connections: " << curConnections);
-
-    stop();
 }
 
 SocketHandlerInterface::SocketOwnership ClientSession::handleIncomingMessage()
@@ -790,7 +787,6 @@ void ClientSession::dumpState(std::ostream& os)
     os << "\t\tisReadOnly: " << isReadOnly()
        << "\n\t\tisDocumentOwner: " << _isDocumentOwner
        << "\n\t\tisAttached: " << _isAttached
-       << "\n\t\tstop: " <<_stop
        << "\n";
     _senderQueue.dumpState(os);
 }
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 3181b0c8..9cd67533 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -75,13 +75,6 @@ public:
         _senderQueue.enqueue(data);
     }
 
-    bool stopping() const { return _stop || _senderQueue.stopping(); }
-    void stop()
-    {
-        _stop = true;
-        _senderQueue.stop();
-    }
-
     /// Set the save-as socket which is used to send convert-to results.
     void setSaveAsSocket(const std::shared_ptr<StreamSocket>& socket)
     {
@@ -152,7 +145,6 @@ private:
     std::unique_ptr<WopiStorage::WOPIFileInfo> _wopiFileInfo;
 
     SenderQueue<std::shared_ptr<Message>> _senderQueue;
-    std::atomic<bool> _stop;
 };
 
 #endif
diff --git a/wsd/SenderQueue.hpp b/wsd/SenderQueue.hpp
index af8199f1..41445ac9 100644
--- a/wsd/SenderQueue.hpp
+++ b/wsd/SenderQueue.hpp
@@ -26,31 +26,17 @@
 #include "Log.hpp"
 #include "TileDesc.hpp"
 
-#include "Socket.hpp" // FIXME: hack for wakeup-world ...
-
-struct SendItem
-{
-    std::shared_ptr<Message> Data;
-    std::string Meta;
-    std::chrono::steady_clock::time_point BirthTime;
-};
-
 /// A queue of data to send to certain Session's WS.
 template <typename Item>
 class SenderQueue final
 {
 public:
 
-    SenderQueue() :
-        _stop(false)
+    SenderQueue()
     {
     }
 
-    bool stopping() const { return _stop || TerminationFlag; }
-    void stop()
-    {
-        _stop = true;
-    }
+    bool stopping() const { return TerminationFlag; }
 
     size_t enqueue(const Item& item)
     {
@@ -173,7 +159,6 @@ private:
     mutable std::mutex _mutex;
     std::deque<Item> _queue;
     typedef typename std::deque<Item>::value_type queue_item_t;
-    std::atomic<bool> _stop;
 };
 
 #endif
commit 58bee0bff037911d770a2aa636f50d8b312c4e3b
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 17:59:29 2017 +0100

    Dump ClientSession and MessageQueue state too.

diff --git a/common/Session.cpp b/common/Session.cpp
index 9dcb5329..69696fb3 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -198,4 +198,26 @@ void Session::handleMessage(bool /*fin*/, WSOpCode 
/*code*/, std::vector<char> &
     }
 }
 
+void Session::dumpState(std::ostream& os)
+{
+    WebSocketHandler::dumpState(os);
+
+    os <<   "\t\tid: " << _id
+       << "\n\t\tname: " << _name
+       << "\n\t\tdisconnected: " << _disconnected
+       << "\n\t\tisActive: " << _isActive
+       << "\n\t\tisCloseFrame: " << _isCloseFrame
+       << "\n\t\tisReadOnly: " << _isReadOnly
+       << "\n\t\tdocURL: " << _docURL
+       << "\n\t\tjailedFilePath: " << _jailedFilePath
+       << "\n\t\tdocPwd: " << _docPassword
+       << "\n\t\thaveDocPwd: " << _haveDocPassword
+       << "\n\t\tisDocPwdProtected: " << _isDocPasswordProtected
+       << "\n\t\tDocOptions: " << _docOptions
+       << "\n\t\tuserId: " << _userId
+       << "\n\t\tuserName: " << _userName
+       << "\n\t\tlang: " << _lang
+       << "\n";
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/common/Session.hpp b/common/Session.hpp
index 2b460d43..b67466e3 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -100,6 +100,8 @@ protected:
         return std::unique_lock<std::mutex>(_mutex);
     }
 
+    void dumpState(std::ostream& os) override;
+
 private:
     virtual bool _handleInput(const char* buffer, int length) = 0;
 
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 8eca9d80..d70d18c8 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -792,6 +792,7 @@ void ClientSession::dumpState(std::ostream& os)
        << "\n\t\tisAttached: " << _isAttached
        << "\n\t\tstop: " <<_stop
        << "\n";
+    _senderQueue.dumpState(os);
 }
 
 
diff --git a/wsd/SenderQueue.hpp b/wsd/SenderQueue.hpp
index 77f09770..af8199f1 100644
--- a/wsd/SenderQueue.hpp
+++ b/wsd/SenderQueue.hpp
@@ -87,6 +87,17 @@ public:
         return _queue.size();
     }
 
+    void dumpState(std::ostream& os)
+    {
+        os << "\n\t\tqueue size " << _queue.size() << "\n";
+        std::lock_guard<std::mutex> lock(_mutex);
+        for (const Item &item : _queue)
+        {
+            os << "\t\t\ttype: " << (item->isBinary() ? "binary" : "text") << 
"\n";
+            os << "\t\t\t" << item->abbr() << "\n";
+        }
+    }
+
 private:
     /// Deduplicate messages based on the new one.
     /// Returns true if the new message should be
commit 1b29185af25db14b0f1f785d5278623af11e2e1f
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 17:58:52 2017 +0100

    Inhibit thread checks for SIGUSR1 handling.
    
    USR1 handling is not thread-safe; we walk the structures and hope.

diff --git a/net/Socket.cpp b/net/Socket.cpp
index cb71ff7b..d42f0730 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -24,6 +24,7 @@
 #include "WebSocketHandler.hpp"
 
 int SocketPoll::DefaultPollTimeoutMs = 5000;
+std::atomic<bool> Socket::InhibitThreadChecks(false);
 
 // help with initialization order
 namespace {
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 64c1b864..300186a7 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -44,6 +44,7 @@ class Socket
 public:
     static const int DefaultSendBufferSize = 16 * 1024;
     static const int MaximumSendBufferSize = 128 * 1024;
+    static std::atomic<bool> InhibitThreadChecks;
 
     Socket() :
         _fd(socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0)),
@@ -194,6 +195,8 @@ public:
     /// Asserts in the debug builds, otherwise just logs.
     virtual void assertCorrectThread()
     {
+        if (InhibitThreadChecks)
+            return;
         // 0 owner means detached and can be invoked by any thread.
         const bool sameThread = (_owner == std::thread::id(0) || 
std::this_thread::get_id() == _owner);
         if (!sameThread)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 196a1c5d..408d49d4 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2192,8 +2192,11 @@ public:
 
     void dumpState(std::ostream& os)
     {
+        // FIXME: add some stop-world magic before doing the dump(?)
+        Socket::InhibitThreadChecks = true;
+
         os << "LOOLWSDServer:\n"
-           << "   Ports: server " << ClientPortNumber
+           << "  Ports: server " << ClientPortNumber
            <<          " prisoner " << MasterPortNumber << "\n"
            << "  TerminationFlag: " << TerminationFlag << "\n"
            << "  isShuttingDown: " << ShutdownRequestFlag << "\n"
@@ -2217,6 +2220,8 @@ public:
                   << "[ " << DocBrokers.size() << " ]:\n";
         for (auto &i : DocBrokers)
             i.second->dumpState(os);
+
+        Socket::InhibitThreadChecks = false;
     }
 
 private:
commit 1f68eddfee88fcff6043866144f84edcd94f6e7a
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Wed Apr 5 11:57:11 2017 +0100

    Remove un-used _stop member, and cleanup redundant code.

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 824a1c3a..196a1c5d 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2156,7 +2156,6 @@ class LOOLWSDServer
     const LOOLWSDServer& operator=(LOOLWSDServer&& other) = delete;
 public:
     LOOLWSDServer() :
-        _stop(false),
         _acceptPoll("accept_poll")
     {
     }
@@ -2174,7 +2173,6 @@ public:
 
     void stopPrisoners()
     {
-        PrisonerPoll.stop();
         PrisonerPoll.joinThread();
     }
 
@@ -2188,10 +2186,6 @@ public:
 
     void stop()
     {
-        _stop = true;
-        _acceptPoll.stop();
-        WebServerPoll.stop();
-        SocketPoll::wakeupWorld(); //TODO: Why?
         _acceptPoll.joinThread();
         WebServerPoll.joinThread();
     }
@@ -2201,7 +2195,6 @@ public:
         os << "LOOLWSDServer:\n"
            << "   Ports: server " << ClientPortNumber
            <<          " prisoner " << MasterPortNumber << "\n"
-           << "  stop: " << _stop << "\n"
            << "  TerminationFlag: " << TerminationFlag << "\n"
            << "  isShuttingDown: " << ShutdownRequestFlag << "\n"
            << "  NewChildren: " << NewChildren.size() << "\n"
@@ -2227,8 +2220,6 @@ public:
     }
 
 private:
-    std::atomic<bool> _stop;
-
     class AcceptPoll : public TerminatingPoll {
     public:
         AcceptPoll(const std::string &threadName) :
commit f9735cf079c8ceb6c11fb60ba090bc09f29432d6
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Wed Apr 5 00:36:33 2017 -0400

    wsd: allow for slow startup of LOK
    
    Change-Id: Idf821f2a3638e76e1a8b169d5672a2059b00491c
    Reviewed-on: https://gerrit.libreoffice.org/36118
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/test/run_unit.sh.in b/test/run_unit.sh.in
index dc1d4021..4b8e71cf 100755
--- a/test/run_unit.sh.in
+++ b/test/run_unit.sh.in
@@ -75,13 +75,14 @@ if test "z$tst" == "z"; then
                                  --o:child_root_path="$jails_path" \
                                  --o:storage.filesystem[@allow]=true \
                                  --o:logging.level=trace \
-                                --o:logging.file[@enable]=false \
+                                 --o:logging.file[@enable]=false \
                                  
--o:ssl.key_file_path="${abs_top_builddir}/etc/key.pem" \
                                  
--o:ssl.cert_file_path="${abs_top_builddir}/etc/cert.pem" \
                                  
--o:ssl.ca_file_path="${abs_top_builddir}/etc/ca-chain.cert.pem" \
                                  --o:admin_console.username=admin 
--o:admin_console.password=admin \
                                  > "$tst_log" 2>&1 &
      echo "  executing test"
+     sleep 5 # allow for wsd to startup
 
      oldpath=`pwd`
      cd "${abs_top_builddir}/test"
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index c50f1a7d..824a1c3a 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2410,7 +2410,7 @@ int LOOLWSD::innerMain()
     {
         std::unique_lock<std::mutex> lock(NewChildrenMutex);
 
-        const auto timeoutMs = CHILD_TIMEOUT_MS * (LOOLWSD::NoCapsForKit ? 150 
: 3);
+        const auto timeoutMs = CHILD_TIMEOUT_MS * (LOOLWSD::NoCapsForKit ? 150 
: 10);
         const auto timeout = std::chrono::milliseconds(timeoutMs);
         // Make sure we have at least one before moving forward.
         LOG_TRC("Waiting for a new child for a max of " << timeoutMs << " 
ms.");
commit 88434550eb933f7e9852cdc6c15702135052654d
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Wed Apr 5 00:26:31 2017 -0400

    wsd: mark detached sockets to have no owner
    
    The DocBroker might not get a chance to
    take ownership of a socket (which is done
    via callbacks that are invoked in the poll loop)
    if it (or WSD) is flagged for termination.
    
    In that case, DocBroker doesn't take ownership
    but ultimately needs to disconnect the socket.
    
    By detaching ownership we signal that any thread
    can rightly take ownership and thus avoid spurious
    warning or assertions.
    
    Change-Id: Idb192bfaac05c5c86809cb21876f3926a080b775
    Reviewed-on: https://gerrit.libreoffice.org/36117
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 9d7c3fd8..64c1b864 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -194,7 +194,8 @@ public:
     /// Asserts in the debug builds, otherwise just logs.
     virtual void assertCorrectThread()
     {
-        const bool sameThread = std::this_thread::get_id() == _owner;
+        // 0 owner means detached and can be invoked by any thread.
+        const bool sameThread = (_owner == std::thread::id(0) || 
std::this_thread::get_id() == _owner);
         if (!sameThread)
             LOG_ERR("#" << _fd << " Invoked from foreign thread. Expected: 0x" 
<< std::hex <<
                     _owner << " but called from 0x" << 
std::this_thread::get_id() << " (" <<
@@ -288,7 +289,8 @@ public:
     /// Asserts in the debug builds, otherwise just logs.
     void assertCorrectThread() const
     {
-        const bool sameThread = (std::this_thread::get_id() == _owner);
+        // 0 owner means detached and can be invoked by any thread.
+        const bool sameThread = (!isAlive() || _owner == std::thread::id(0) || 
std::this_thread::get_id() == _owner);
         if (!sameThread)
             LOG_ERR("Incorrect thread affinity for " << _name << ". Expected: 
0x" << std::hex <<
                     _owner << " (" << std::dec << Util::getThreadId() << ") 
but called from 0x" <<
@@ -387,6 +389,7 @@ public:
             {
                 LOG_DBG("Removing socket #" << _pollFds[i].fd << " (of " <<
                         _pollSockets.size() << ") from " << _name);
+                _pollSockets[i]->setThreadOwner(std::thread::id(0));
                 _pollSockets.erase(_pollSockets.begin() + i);
             }
         }
commit 703e08f786f99d0f222481995384480957ddcb34
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Wed Apr 5 00:26:09 2017 -0400

    wsd: some informative logging
    
    Change-Id: I4338f5bd8056d1d66da01efaa1a1fe54f8717793
    Reviewed-on: https://gerrit.libreoffice.org/36116
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0f896200..d7c52de5 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -243,6 +243,10 @@ void DocumentBroker::pollThread()
         }
     }
 
+    LOG_INF("Finished polling doc [" << _docKey << "]. stop: " << _stop << ", 
continuePolling: " <<
+            _poll->continuePolling() << ", TerminationFlag: " << 
TerminationFlag <<
+            ", ShutdownRequestFlag: " << ShutdownRequestFlag << ".");
+
     // Terminate properly while we can.
     //TODO: pass some sensible reason.
     terminateChild("", false);
@@ -833,6 +837,10 @@ size_t DocumentBroker::removeSessionInternal(const 
std::string& id)
             LOG_TRC("Removed " << (readonly ? "readonly" : "non-readonly") <<
                     " session [" << id << "] from docKey [" <<
                     _docKey << "] to have " << count << " sessions.");
+            for (const auto& pair : _sessions)
+            {
+                LOG_TRC("Session: " << pair.second->getName());
+            }
 
             // Let the child know the client has disconnected.
             const std::string msg("child-" + id + " disconnect");
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index 4c1d7572..b744eded 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -53,7 +53,8 @@ TileCache::TileCache(const std::string& docURL,
     _cacheDir(cacheDir)
 {
     LOG_INF("TileCache ctor for uri [" << _docURL <<
-            "] modifiedTime=" << (modifiedTime.raw()/1000000) <<
+            "], cacheDir: [" << _cacheDir <<
+            "], modifiedTime=" << (modifiedTime.raw()/1000000) <<
             " getLastModified()=" << (getLastModified().raw()/1000000));
     File directory(_cacheDir);
     std::string unsaved;
commit b82a2cc16afcae67572a46babb4e47989c4eefa9
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Wed Apr 5 00:25:15 2017 -0400

    wsd: start DocBroker thread before adding callbacks
    
    And move more into the callback to ensure
    thread affinity.
    
    Change-Id: I1d6985716d0d36aa488b65263ecb41f444f77255
    Reviewed-on: https://gerrit.libreoffice.org/36115
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 62598dee..c50f1a7d 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1856,15 +1856,19 @@ private:
                     auto clientSession = createNewClientSession(nullptr, _id, 
uriPublic, docBroker, isReadOnly);
                     if (clientSession)
                     {
-                        clientSession->setSaveAsSocket(socket);
-
                         // Transfer the client socket to the DocumentBroker.
-                        // Move the socket into DocBroker.
-                        docBroker->addSocketToPoll(socket);
                         socketOwnership = 
SocketHandlerInterface::SocketOwnership::MOVED;
 
-                        docBroker->addCallback([&, clientSession]()
+                        // Make sure the thread is running before adding 
callback.
+                        docBroker->startThread();
+
+                        docBroker->addCallback([docBroker, socket, 
clientSession, format]()
                         {
+                            clientSession->setSaveAsSocket(socket);
+
+                            // Move the socket into DocBroker.
+                            docBroker->addSocketToPoll(socket);
+
                             // First add and load the session.
                             docBroker->addSession(clientSession);
 
@@ -1888,8 +1892,6 @@ private:
                             clientSession->handleMessage(true, 
WebSocketHandler::WSOpCode::Text, saveasRequest);
                         });
 
-                        docBroker->startThread();
-
                         sent = true;
                     }
                     else
@@ -2083,6 +2085,9 @@ private:
                 // Remove from current poll as we're moving ownership.
                 socketOwnership = 
SocketHandlerInterface::SocketOwnership::MOVED;
 
+                // Make sure the thread is running before adding callback.
+                docBroker->startThread();
+
                 docBroker->addCallback([docBroker, socket, clientSession]()
                 {
                     // Set the ClientSession to handle Socket events.
@@ -2095,8 +2100,6 @@ private:
                     // Add and load the session.
                     docBroker->addSession(clientSession);
                 });
-
-                docBroker->startThread();
             }
             else
             {
commit e319b75fa3b18aba2cd95bfebe78cdc1e3d0f405
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Wed Apr 5 00:18:16 2017 -0400

    wsd: simplify career span timing
    
    Change-Id: I0bfb3bca99f3f20ca9244e580c80801e89890fc2
    Reviewed-on: https://gerrit.libreoffice.org/36113
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 2c8b33e1..62598dee 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -182,7 +182,7 @@ static std::mutex DocBrokersMutex;
 extern "C" { void dump_state(void); /* easy for gdb */ }
 
 #if ENABLE_DEBUG
-static int careerSpanSeconds = 0;
+static int careerSpanMs = 0;
 #endif
 
 namespace
@@ -985,7 +985,7 @@ void LOOLWSD::handleOption(const std::string& optionName,
         NoCapsForKit = true;
 #endif
     else if (optionName == "careerspan")
-        careerSpanSeconds = std::stoi(value);
+        careerSpanMs = std::stoi(value) * 1000; // Convert second to ms
 
     static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT");
     if (clientPort)
@@ -2431,14 +2431,11 @@ int LOOLWSD::innerMain()
     // Start the server.
     srv.start(ClientPortNumber);
 
-#if ENABLE_DEBUG
-    time_t startTimeSpan = time(nullptr);
-#endif
-
-    auto startStamp = std::chrono::steady_clock::now();
-
     /// The main-poll does next to nothing:
     SocketPoll mainWait("main");
+
+    const auto startStamp = std::chrono::steady_clock::now();
+
     while (!TerminationFlag && !ShutdownRequestFlag)
     {
         UnitWSD::get().invokeTest();
@@ -2452,16 +2449,17 @@ int LOOLWSD::innerMain()
         // Wake the prisoner poll to spawn some children, if necessary.
         PrisonerPoll.wakeup();
 
+        const auto timeSinceStartMs = 
std::chrono::duration_cast<std::chrono::milliseconds>(
+                                            std::chrono::steady_clock::now() - 
startStamp).count();
+
         // Unit test timeout
-        if (std::chrono::duration_cast<std::chrono::milliseconds>(
-                std::chrono::steady_clock::now() - startStamp).count() >
-            UnitWSD::get().getTimeoutMilliSeconds())
+        if (timeSinceStartMs > UnitWSD::get().getTimeoutMilliSeconds())
             UnitWSD::get().timeout();
 
 #if ENABLE_DEBUG
-        if (careerSpanSeconds > 0 && time(nullptr) > startTimeSpan + 
careerSpanSeconds)
+        if (careerSpanMs > 0 && timeSinceStartMs > careerSpanMs)
         {
-            LOG_INF((time(nullptr) - startTimeSpan) << " seconds gone, 
finishing as requested.");
+            LOG_INF(timeSinceStartMs << " milliseconds gone, finishing as 
requested.");
             break;
         }
 #endif
commit 160c23fb214c9583a3388781ddf31f9004793888
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Tue Apr 4 21:28:55 2017 -0400

    wsd: stop poll threads before joining
    
    Also add symmetric stopPrisoners to
    match startPrisoners to LOOLWSDServer.
    
    Change-Id: I78d76d86a8e7efc0964cd06df2340658c1b6c4ba
    Reviewed-on: https://gerrit.libreoffice.org/36111
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index ac6313dc..2c8b33e1 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2169,6 +2169,12 @@ public:
         PrisonerPoll.insertNewSocket(findPrisonerServerPort(port));
     }
 
+    void stopPrisoners()
+    {
+        PrisonerPoll.stop();
+        PrisonerPoll.joinThread();
+    }
+
     void start(const int port)
     {
         _acceptPoll.startThread();
@@ -2180,7 +2186,9 @@ public:
     void stop()
     {
         _stop = true;
-        SocketPoll::wakeupWorld();
+        _acceptPoll.stop();
+        WebServerPoll.stop();
+        SocketPoll::wakeupWorld(); //TODO: Why?
         _acceptPoll.joinThread();
         WebServerPoll.joinThread();
     }
@@ -2479,7 +2487,7 @@ int LOOLWSD::innerMain()
     SigUtil::killChild(ForKitProcId);
 #endif
 
-    PrisonerPoll.joinThread();
+    srv.stopPrisoners();
 
     // Terminate child processes
     LOG_INF("Requesting child processes to terminate.");
commit d29973fe51d5a0ab490abe4962bc380f80261159
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Tue Apr 4 21:04:48 2017 -0400

    wsd: warn when waking dead poll
    
    And insert sockets after starting the
    thread so we poll the socket immediately.
    
    Change-Id: Id336e1838f2f624ebfe59c4c2caf33eaa1a638c9
    Reviewed-on: https://gerrit.libreoffice.org/36110
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 11f2c91b..9d7c3fd8 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -402,12 +402,15 @@ public:
         } while (rc == -1 && errno == EINTR);
 
         if (rc == -1 && errno != EAGAIN && errno != EWOULDBLOCK)
-            LOG_WRN("wakeup socket #" << fd << " is closd at wakeup? error: " 
<< errno);
+            LOG_SYS("wakeup socket #" << fd << " is closd at wakeup?");
     }
 
     /// Wakeup the main polling loop in another thread
     void wakeup()
     {
+        if (!isAlive())
+            LOG_WRN("Waking up dead poll thread [" << _name << "]");
+
         wakeup(_wakeup[1]);
     }
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index a1d7ec61..ac6313dc 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2165,14 +2165,14 @@ public:
 
     void startPrisoners(int& port)
     {
-        PrisonerPoll.insertNewSocket(findPrisonerServerPort(port));
         PrisonerPoll.startThread();
+        PrisonerPoll.insertNewSocket(findPrisonerServerPort(port));
     }
 
     void start(const int port)
     {
-        _acceptPoll.insertNewSocket(findServerPort(port));
         _acceptPoll.startThread();
+        _acceptPoll.insertNewSocket(findServerPort(port));
         WebServerPoll.startThread();
         Admin::instance().start();
     }
commit d031108659dee794606c22cb8100a8967ee4ecc9
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Tue Apr 4 08:28:25 2017 -0400

    wsd: accept alternative slideshow export
    
    Apparently some setups produce a slightly different
    output than the expected. Here we tolerate for those
    outputs as well.
    
    Change-Id: Ia4beeb653ff6182e1403a59fbd05c6a46b9277ac
    Reviewed-on: https://gerrit.libreoffice.org/36080
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index a848e656..27b698d7 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -59,7 +59,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testBadRequest);
     // FIXME CPPUNIT_TEST(testHandshake);
     CPPUNIT_TEST(testCloseAfterClose);
-    CPPUNIT_TEST(testConnectNoLoad); // This fails most of the times but 
occasionally succeeds
+    CPPUNIT_TEST(testConnectNoLoad);
     CPPUNIT_TEST(testLoadSimple);
     CPPUNIT_TEST(testLoadTortureODT);
     CPPUNIT_TEST(testLoadTortureODS);
@@ -1173,7 +1173,9 @@ void HTTPWSTest::testSlideShow()
         session->receiveResponse(responseSVG);
         CPPUNIT_ASSERT_EQUAL(Poco::Net::HTTPResponse::HTTP_OK, 
responseSVG.getStatus());
         CPPUNIT_ASSERT_EQUAL(std::string("image/svg+xml"), 
responseSVG.getContentType());
-        CPPUNIT_ASSERT_EQUAL(std::streamsize(451329), 
responseSVG.getContentLength());
+        // Some setups render differently; recognize these two valid output 
sizes for now.
+        CPPUNIT_ASSERT(responseSVG.getContentLength() == 
std::streamsize(451329) ||
+                       responseSVG.getContentLength() == 
std::streamsize(467345));
     }
     catch (const Poco::Exception& exc)
     {
commit 449148d341f3bdc033ed59a82da8848fa59b1242
Author: Michael Meeks <michael.me...@collabora.com>
Date:   Mon Apr 3 21:50:09 2017 +0100

    old style unit tests: re-direct output to log file & add --verbose.

diff --git a/test/run_unit.sh.in b/test/run_unit.sh.in
index df6fb155..dc1d4021 100755
--- a/test/run_unit.sh.in
+++ b/test/run_unit.sh.in
@@ -13,6 +13,7 @@ enable_debug="@ENABLE_DEBUG@"
 jails_path="@JAILS_PATH@"
 lo_path="@LO_PATH@"
 valgrind_cmd="valgrind --tool=memcheck --trace-children=no -v 
--read-var-info=yes"
+hide_stderr='2>/dev/null'
 
 # Note that these options are used by commands in the Makefile that
 # Automake generates. Don't be mislead by 'git grep' not showing any
@@ -25,13 +26,14 @@ while test $# -gt 0; do
       --log-file)  tst_log=$2; shift;;
       --trs-file)  test_output=$2; shift;;
       --valgrind)  valgrind=$valgrind_cmd; shift;;
+      --verbose)   hide_stderr="";;
   -*) ;; # ignore
   esac
   shift
 done
 
 echo
-echo "Running $tst"
+echo "Running ${tst}"
 echo " $cmd_line"
 
 # drop .la suffix
@@ -73,6 +75,7 @@ if test "z$tst" == "z"; then
                                  --o:child_root_path="$jails_path" \
                                  --o:storage.filesystem[@allow]=true \
                                  --o:logging.level=trace \
+                                --o:logging.file[@enable]=false \
                                  
--o:ssl.key_file_path="${abs_top_builddir}/etc/key.pem" \
                                  
--o:ssl.cert_file_path="${abs_top_builddir}/etc/cert.pem" \
                                  
--o:ssl.ca_file_path="${abs_top_builddir}/etc/ca-chain.cert.pem" \
@@ -82,7 +85,7 @@ if test "z$tst" == "z"; then
 
      oldpath=`pwd`
      cd "${abs_top_builddir}/test"
-     if ${valgrind} ./test; then
+     if eval ${valgrind} ./test ${hide_stderr}; then
         echo "Test run_test.sh passed."
         echo ":test-result: PASS run_test.sh" >> $oldpath/$test_output
         retval=0
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits