commit bdaf345ef07f30fc2f9a59967933acc0d6272ae5 Author: Michael Catanzaro Date: Sun Mar 19 16:20:29 2017 -0500 Prepare 3.22.7 NEWS | 10 ++++++++++ configure.ac | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) commit e819b36748519b3225bc596e3e0f577a28b3a560 Author: Michael Catanzaro Date: Sat Mar 4 13:28:19 2017 -0600 embed: avoid memory corruption when clearing top widgets Don't call remove_from_destroy_list_cb, which modifies the destroy list, when already iterating through the list. https://bugzilla.gnome.org/show_bug.cgi?id=779180 embed/ephy-embed.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) commit f6cd6a9a0c00471f8fce97f484c98780df2e3ca7 Author: Carlos Garcia Campos Date: Fri Mar 3 10:11:18 2017 +0100 Allocate PermissionRequestData with g_slice_new since it's freed with g_slice_free https://bugzilla.gnome.org/show_bug.cgi?id=779180 embed/ephy-web-view.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) commit e101530c3f029d0c828a93f94687821bf4c34d9d Author: Michael Catanzaro Date: Wed Mar 1 07:45:06 2017 -0600 history-service: Don't crash if database is locked Yeah this is really bad, but let's not make it fatal. I changed this in 3f76e6e5d45e4be973653f530e23c5ce2667d079 but I'm not sure if it was intentional. It doesn't look like it, because I don't like leaving unreachable code after calling g_error(). I think I was probably just considering the change and forgot to turn it back to g_warning(). lib/history/ephy-history-service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) commit cfe3911b43de2ebf3c77e3377639a0bc51f7a12b Author: Michael Catanzaro Date: Tue Feb 21 10:36:18 2017 -0600 history-service: remove longstanding transactions Instead of having a longstanding transaction open at all times and scheduling commits that open a new transaction, just create transactions around history messages, so that each message forms its own atomic transaction. This is way simpler. I considered that we might not need transactions at all, but there are performance implications to removing transactions entirely. lib/history/ephy-history-service-hosts-table.c | 1 - lib/history/ephy-history-service-private.h | 2 - lib/history/ephy-history-service-urls-table.c | 1 - lib/history/ephy-history-service-visits-table.c | 7 +- lib/history/ephy-history-service.c | 135 ++++++++---------------- 5 files changed, 47 insertions(+), 99 deletions(-) commit 0a6e90f8badde14b3bd6e112a9df2c8b68ef41bf Author: Michael Catanzaro Date: Tue Feb 21 09:54:11 2017 -0600 history-service: Remove useless allocations lib/history/ephy-history-service-hosts-table.c | 7 +++---- lib/history/ephy-history-service-urls-table.c | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) commit c636bdc739c55bb3cb9ddf4915d4137f319dbf86 Author: Michael Catanzaro Date: Mon Feb 20 20:24:03 2017 -0600 Fix theoretical race condition in ephy_history_service_add_visit_row The design of the history service feels like one big footgun. I'm really not sure why a history thread is necessary at all, or why we have longstanding transactions (defeating the entire purpose of transactions) instead of just using autocommit, which I think would be sufficient for everything we do. This commit doesn't fix any of that. That's just a rant. This commit just fixes one theoretical race condition. Prepared statements lock the database and need to be finalized BEFORE commit. The current code only works if the prepared statement is finalized on the UI thread before the scheduled commit occurs on the history thread. Which is probably always, but let's not leave it to luck. I could see this leading to a small loss of the last bit of history when closing the browser. https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service-visits-table.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) commit 429ed9b34b0cc708e8bd0b79f08486893a58b114 Author: Michael Catanzaro Date: Sun Feb 19 11:05:26 2017 -0600 history-service: Fix write to database in read-only mode Now that SQLite enforces read-only mode for us, bugs like this will be uncovered.... https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service-hosts-table.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) commit feae9a35c59bb5abd6ab6708dac82bcaf9c605ef Author: Michael Catanzaro Date: Sun Feb 19 09:45:32 2017 -0600 history-service: Fix multiple initialization race conditions This started out as a project to fix the read-only service test I just added. Initializing two history service objects in a row was racy, because I needed the first history service to be initialized before creating the second one, but there was no way to ensure that. This was only an issue for this one test, though; real Epiphany browser mode of course only creates one history service, so I assumed it was not a big problem. Fix this first issue using a condition variable to ensure the GObject initialization doesn't complete until after the history service has actually created the SQLite database. In doing this, I discovered a second bug. The use of the condition variable altered the timing slightly, and caused the history filename property to not be set in time when entering the history service thread. In fact, it's kind of amazing that the history service ever worked at all, because there is absolutely nothing here to guarantee that the filename and read-only properties have been initialized prior to starting the history service thread. So the database filename could be NULL when opening the database, which is a great way to lose all your history. Also, it could also be in read-only mode here even if it is supposed to be read/write mode, which is going to cause failures after today's commits. Fix this by adding a constructed function and starting the history thread from there, instead of doing it in init. This means that the history thread will not be started until after properties have been set. Note that, while I could not reproduce this bug on my machine until after adding the condition variable to fix the first bug, that was just due to timing and luck; it was already broken before. https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service-private.h | 2 ++ lib/history/ephy-history-service.c | 47 ++++++++++++++++++++++++------ tests/ephy-history-test.c | 5 ---- 3 files changed, 40 insertions(+), 14 deletions(-) commit 1266f3025d4fd38df1bcc1afdbe02028e7117e9a Author: Michael Catanzaro Date: Sun Feb 19 08:58:42 2017 -0600 history-service: Remove incorrect comment The code does something different, and it's not complex enough to merit a comment anyway. https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service.c | 1 - 1 file changed, 1 deletion(-) commit c4e43bf3b93a2a56070fbafea5a6ab01d5b5b6d8 Author: Michael Catanzaro Date: Sun Feb 19 08:57:04 2017 -0600 history-service: Don't schedule commit after clearing history Now that clear all is implemented by deleting the database file, there's no longer any need to schedule a commit here. https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service.c | 1 - 1 file changed, 1 deletion(-) commit 92885ae9a87bdd963eb56b378dae80db92e3c5da Author: Michael Catanzaro Date: Sat Feb 18 22:13:05 2017 -0600 history-service: Remove Yoda conditions There's no excuse for this.... https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) commit b03a62304845b1140d49be58fae72e400138e677 Author: Michael Catanzaro Date: Sat Feb 18 22:05:21 2017 -0600 history-service: Fix leak when clearing all history Closing the connection is great, but not enough. We're leaking our wrapper object. https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service.c | 3 +++ 1 file changed, 3 insertions(+) commit 6abc89cab36a5bd177f05670243227e4c50bcfad Author: Michael Catanzaro Date: Sat Feb 18 21:28:22 2017 -0600 history-service: Ensure thread member is initialized before use We have assertions to ensure that several functions are only ever called on the history thread. But the first such assertion, at the top of run_history_service_thread, sometimes fails when running the tests. It is racy. Use a mutex to fix this. These assertions are actually executed at runtime for end users, so it's surprising that nobody has ever reported a bug about this. We also need to be sure to initialize the async queue before running the history service thread. The mutex is needed as a memory barrier here, so it's not possible to remove the mutex by removing the assertions except in debug mode, which is something I considered. https://bugzilla.gnome.org/show_bug.cgi?id=778649 lib/history/ephy-history-service-private.h | 1 + lib/history/ephy-history-service.c | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) commit a6d81d79ccc31a7b262cdc1dd09ac2ea4a03bc7e Author: Michael Catanzaro Date: Sat Feb 18 20:47:50 2017 -0600 Fix search provider horribly breaking history service If the search provider is running, all database transactions will fail because the search provider will take a write lock on the database. Ouch! This is worth a good string of profanities.... Notably, this causes opening the database to fail if you searched for anything in the shell overview in the minute prior to starting Epiphany. (One minute is our search provider timeout.) Then no history will ever get saved, ever. I think. Something like that. So, although our history service has read-only mode, it's enforced at the history service level, not the SQLite connection level. SQLite actually has a read-only mode, which we are not using, and which we need to use in the search provider if we want to have any chance of reliably saving history. Accordingly, give EphySQLiteConnection a mode property, to indicate whether it is in writable mode or read-only mode. Teach all callers to set it properly. Use it, rather than a boolean, when creating the EphyHistoryService, since boolean parameters are hard to read at call sites. And actually put the underlying SQLite connection in read-only mode when set. Don't open transactions or ever attempt to rollback in read-only mode, because that doesn't make any sense. This should never have been happening due to the history service level read-only checks, but it should be enforced at the SQLite connection level now, too. Avoid initializing tables when opening the database in read-only mode. This is obviously writing to the database, and now that we really have a read-only SQLite connection it fails. As it should. SQLite connection creation will now fail in case the connection is read-only and the database does not yet exist; it will no longer be created anyway. So handle this case gracefully. It's fine for the history service to return nothing in this case. This has the small advantage that the history thread will quit immediately after it's created in this case, so it's not constantly running if there's no history in incognito mode anymore. To check for this condition, we expose the underlying SQLite error; previously, only the error message was exposed outside of EphySQLiteConnection. Exposing the error isn't really necessary or sufficient, though, since it's super generic and we have to check if the file actually exists on disk anyway. Test it. Ensure that a read/write history service functions properly if it's running at the same time as a read-only history service. Using two read/write services here fails very badly, but when one of the services is read-only it works fine. Also, remove the original read-only service test. It only ever tested that creating a read-only history service with an empty history database would succeed. And, as I just explained, that fails now. Lastly, stop running a second history service for the search provider. It needed its own once upon a time when the search provider did not run an EphyShell instance. That changed when we stopped using the default web context, because nothing works without EphyEmbedShell now, as all sorts of places need it to get the embed's web context. And since EphyEmbedShell runs its own history service, the search provider can just use that now instead of running its own separate one. https://bugzilla.gnome.org/show_bug.cgi?id=778649 embed/ephy-embed-shell.c | 10 ++++-- lib/Makefile.am | 3 +- lib/ephy-profile-migrator.c | 2 +- lib/ephy-sqlite-connection.c | 70 +++++++++++++++++++++++++++++++++----- lib/ephy-sqlite-connection.h | 13 ++++++- lib/history/ephy-history-service.c | 21 +++++++++--- lib/history/ephy-history-service.h | 4 ++- src/ephy-search-provider.c | 8 ++--- tests/ephy-history-test.c | 64 +++++++++++++++++++++------------- tests/ephy-sqlite-test.c | 2 +- 10 files changed, 149 insertions(+), 48 deletions(-) commit 37a44e922e8d873ce9d6a5616ef5fba93ee2aa88 Author: Michael Catanzaro Date: Wed Feb 15 10:28:28 2017 -0600 downloads-popover: Disconnect more signals when popover is destroyed These signals can run after the popover has been destroyed. We don't want that. Speculative fix for this critical: epiphany-Gtk-CRITICAL: gtk_widget_set_sensitive: assertion 'GTK_IS_WIDGET (widget)' failed https://bugzilla.gnome.org/show_bug.cgi?id=778659 lib/widgets/ephy-downloads-popover.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) commit 9d75f710be56c92b87fe91484d579559172aa92b Author: Michael Catanzaro Date: Tue Feb 14 22:55:37 2017 -0600 sqlite-connection: Do not ignore errors when executing commands This file is so careful to handle errors properly everywhere EXCEPT the point where actual SQLite commands are executed. The history database is pretty much totally broken right now; having error messages would be helpful thank you! lib/ephy-sqlite-connection.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) commit 305b4f8905747785f7683161cca38bb7bc449ed4 Author: Michael Catanzaro Date: Mon Feb 13 22:51:34 2017 -0600 embed-shell: fix criticals in delayed_thumbnail_update_data_free embed/ephy-embed-shell.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) commit 869d3c7d463976a558d99f1314926df59a4a85fa Author: Michael Catanzaro Date: Mon Feb 13 21:07:31 2017 -0600 snapshot-service: Fix stale snapshot replacement Looks like I broke this in a refactoring, probably adc6c404f650d51bf2709ed3d6f70475a0. Snapshots loaded for the overview are almost always available in cache before a webpage is visited, so those pages would never get updated. We need to update stale snapshots in ephy_snapshot_service_get_snapshot_path_async() to avoid this. It *could* still happen that snapshot requests are scheduled multiple times in a row, but it's unlikely and harmless. lib/ephy-snapshot-service.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) commit dfeedecc5c92980e0c6cf57f297063503bf0b013 Author: Michael Catanzaro Date: Fri Feb 3 13:27:32 2017 -0600 Prepare 3.22.6 NEWS | 8 ++++++++ configure.ac | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-)