After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 765863 - Simplifications for the snapshot service
Simplifications for the snapshot service
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on: 763184
Blocks:
 
 
Reported: 2016-04-30 22:27 UTC by Michael Catanzaro
Modified: 2016-05-09 13:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
snapshot-service: Stop using const time_t (6.45 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
web-view-test: Fix return type of visit_url_cb (1.07 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: Clarify error message (1.43 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: Remove unused functions to retrieve pixbufs (12.45 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: Make ephy_snapshot_service_save private (12.12 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: assert that web view has loaded at some point (2.83 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: pretty up header file (4.90 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: Rename memory cache lookup function (3.22 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: Ensure snapshot freshness in a better location (4.91 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review
snapshot-service: Reuse the same SnapshotAsyncData struct (8.61 KB, patch)
2016-04-30 22:33 UTC, Michael Catanzaro
committed Details | Review

Description Michael Catanzaro 2016-04-30 22:27:38 UTC
ephy-snapshot-service.c is too long and confusing. This patches removes about 200 lines of code.
Comment 1 Michael Catanzaro 2016-04-30 22:33:08 UTC
Created attachment 327085 [details] [review]
snapshot-service: Stop using const time_t

We don't use const in C except for strings. And certainly not for
pass-by-value.
Comment 2 Michael Catanzaro 2016-04-30 22:33:11 UTC
Created attachment 327086 [details] [review]
web-view-test: Fix return type of visit_url_cb

tartan would have caught this.
Comment 3 Michael Catanzaro 2016-04-30 22:33:14 UTC
Created attachment 327087 [details] [review]
snapshot-service: Clarify error message

The snapshot service has two caches.
Comment 4 Michael Catanzaro 2016-04-30 22:33:18 UTC
Created attachment 327088 [details] [review]
snapshot-service: Remove unused functions to retrieve pixbufs

We have a bunch of complicated code to support retrieving GdkPixbufs
from the snapshot service, but Epiphany does not use these and I don't
expect it to in the future. The snapshot service is too complex right
now and it's more important to simplify than to keep dead code around.
Comment 5 Michael Catanzaro 2016-04-30 22:33:21 UTC
Created attachment 327089 [details] [review]
snapshot-service: Make ephy_snapshot_service_save private

Requires rearrangements to avoid forward declarations.
Comment 6 Michael Catanzaro 2016-04-30 22:33:25 UTC
Created attachment 327090 [details] [review]
snapshot-service: assert that web view has loaded at some point

We should never attempt to get a snapshot from a web view that has never
loaded anything ever. Instead of attempting to handle this case in a
nonsensical manner, just assert that it will never happen.
Comment 7 Michael Catanzaro 2016-04-30 22:33:29 UTC
Created attachment 327091 [details] [review]
snapshot-service: pretty up header file
Comment 8 Michael Catanzaro 2016-04-30 22:33:33 UTC
Created attachment 327092 [details] [review]
snapshot-service: Rename memory cache lookup function

To be more clear
Comment 9 Michael Catanzaro 2016-04-30 22:33:36 UTC
Created attachment 327093 [details] [review]
snapshot-service: Ensure snapshot freshness in a better location

This reduces the likelihood that we'll duplicate effort in taking a
snapshot twice, by ensuring we only ever call
ephy_snapshot_service_take_from_webview in one location.
Comment 10 Michael Catanzaro 2016-04-30 22:33:40 UTC
Created attachment 327094 [details] [review]
snapshot-service: Reuse the same SnapshotAsyncData struct

Having too many structs makes this file unnecessarily difficult to read.
Comment 11 Michael Catanzaro 2016-05-09 13:40:26 UTC
Attachment 327085 [details] pushed as 0cbd04f - snapshot-service: Stop using const time_t
Attachment 327086 [details] pushed as 050d8bb - web-view-test: Fix return type of visit_url_cb
Attachment 327087 [details] pushed as 9af9fb9 - snapshot-service: Clarify error message
Attachment 327088 [details] pushed as 3f0c4a9 - snapshot-service: Remove unused functions to retrieve pixbufs
Attachment 327089 [details] pushed as a97b8e9 - snapshot-service: Make ephy_snapshot_service_save private
Attachment 327090 [details] pushed as 3c2fc9b - snapshot-service: assert that web view has loaded at some point
Attachment 327091 [details] pushed as 8f3191c - snapshot-service: pretty up header file
Attachment 327092 [details] pushed as d89374e - snapshot-service: Rename memory cache lookup function
Attachment 327093 [details] pushed as adc6c40 - snapshot-service: Ensure snapshot freshness in a better location
Attachment 327094 [details] pushed as c6b53f5 - snapshot-service: Reuse the same SnapshotAsyncData struct