Add command option to control enable/disable local storage

[RockLi]

Issue link: #11055
Sometimes we need the ability to control whether to enable the local storage or not, especially when we want to build a large scraping system.

[JamesMGreene]

This definitely needs to be revised as a LocalStorage config shouldn't control the IndexedDB and AppCache settings. You should add additional flags for those if need be.

[RockLi]

I can set additional flags for LocalStorage, IndexedDB, AppCache respectively. And i suggest to add a new config option to disable any disk write actions thoroughly. will it make sense?

[JamesMGreene]

Do you mean add a new config option that sets all of the various configs that control writing to disk (HTTP cache, LocalStorage, IndexedDB, and AppCache)? If so, I think that could be a handy config for those running PhantomJS on a system with limited resources.

I would also suggest that you follow the existing naming pattern for the LocalStorage config options for the other new ones (IndexedDB and AppCache):

local-storage: Enables local storage: 'true' (default) or 'false'
local-storage-path: Specifies the location for offline local storage
local-storage-quota: Sets the maximum size of the offline local storage (in KB)

I think I'm also going to file an issue to update the config names for "disk-cache" and "disk-cache-max-size" to more closely align... probably change them to "http-cache" and "http-cache-quota" (but leave the other name around for a release or two to deprecate it).

[RockLi]

I will update a proposal commit soon which including the proposal naming.
In that commit i modified some confuse naming to make it looks better, but it's not the perfect solution.
you can review it. will you going to handle updating the config names 'disk-cache' or i fix them?

[JamesMGreene]

I'll take care of disk-cache... it will need some extra handling to support its deprecated former name for a least 1 release. Thanks for offering, though!

[RockLi]

So how long will you merge my commits or take a new grace way to let we can control appcache or indexeddb respectively? I'm waiting this. :).

[JamesMGreene]

If you mean "when would my changes be available in an official binary", then the answer is March 20th (in the 1.9 release).

Till then, of course, you can easily build it yourself but hopefully you already knew that since you submitted a PR and edited the build script. :)

[JamesMGreene]

Ugh, this will need further review.... didn't realize there was some weird relationship between offlineStorage and localStorage for their disk quota (and possibly more).

@ariya @detro @vitallium Any knowledge on this WebKit oddity?

[ariya]

Please combine it into just one commit. Also, what's that echo -n change doing there?

[RockLi]

@JamesMGreene I referenced those comments in the file qwebsettings.cpp, anyone else can help review the changed part?

@ariya the change echo -n there will let the prompt looks better(referenced other projects' prompt), no newline to accept 'y/n'. Is it make sense? If not, I will revert this change. And I will combine all these commits into one commit if there's no other problems.

[ariya]

The change echo -n is fine but generally it should not be mixed with this commit.

[RockLi]

@ariya @JamesMGreene I combined the two commits into this one 7a5d545 .

reverted the change in file build.sh.

[ariya]

Looks good, let me give it a try and test it!

[JamesMGreene]

So my main question remains: does WebKit not have a way to specify the default quota for LocalStorage, or does it also utilize the offlineStorageDefaultQuota?

If it's the latter, do we need to sum any config value the users pass for local-storage-quota (which is currently removed in this PR) with the config value passed for indexed-db-quota? i.e. pseudo-code:
setOfflineStorageDefaultQuota((indexedDbDefaultQuota || 0) + (localStorageDefaultQuota || 0));

[vitallium]

Hi James.
WebKit has a limit for Local Storage and IndexedDb in 5 Mb as described in the standard
These limits are defined here. QtWebkit sets this default value too.
We can specify the default quota for LocalStorage using QWebSettings::setOfflineStorageDefaultQuota, but not for IndexedDb.

[JamesMGreene]

@vitallium: Thanks for the info but... how can you tell that QWebSettings::setOfflineStorageDefaultQuota updates the quota for LocalStorage when the other "[Oo]fflineStorage" settings are [I assume] for IndexedDB? e.g.

• setOfflineStoragePath vs. setLocalStoragePath
• OfflineStorageDatabaseEnabled vs. LocalStroageEnabled (same link)

[vitallium]

QWebSettings::setOfflineStorageDefaultQuota doesn't update the quota for LocalStorage. Because the option to change it was moved to GroupSettings, which is not accessible from QWebSettings.
And we can't change the quota for IndexedDB, since it's there (in GroupSettings) too.

[JamesMGreene]

I believe you're saying that QWebSettings::setOfflineStorageDefaultQuota will not affect/change any actual quotas. Is that correct? If so, we'll need to drop that command line option all together and may as well remove it from our CustomPage class (/src/webpage.cpp) as well.

[vitallium]

Almost, QWebSettings::setOfflineStorageDefaultQuota will affect only the quota for offline storage (http:https://www.w3.org/TR/offline-webapps/)

[JamesMGreene]

Whoa, somehow I missed that HTML5 offers offline SQL database in addition to IndexedDB. That's... weird. o_O

Ugh, so I think this PR is all messed up now, then. The options marked as "indexed-db" would need to be updated to "offline-db" (or something like that).

So, @vitallium, does enabling LocalStorage also enable IndexedDB? Or is IndexedDB always on? Or does enabling "OfflineStorage" enable both local "offline-db" plus IndexedDB?

This is getting really confusing.

[vitallium]

Yea, HTML 5 is do damn confusing :)
Answering to your questions:

1. No, it doesn't
2. Yes, IndexedDB is always on
3. No, it doesn't
   To enable all offline storage (database, offline cache and local storage) we can use QWebSettings::enablePersistentStorage

[JamesMGreene]

Yeah, I noticed QWebSettings::enablePersistentStorage before. I was thinking that might be a good way to tackle @RockLi's suggestion of having a single switch that could enable/disable all of them, too.

[JamesMGreene]

According to @brianleroux, offline storage DBs are "essentially deprecated" from HTML5. As such, perhaps we should just remove those "offline-db_" (currently incorrectly marked as "indexed-db_") options altogether...?

[brianleroux]

well, websql is probably not moving forward and indexeddb is favored, but the browsers will continue to support for a very long time to come

[JamesMGreene]

@brianleroux: Why? 👎

Sounds like it was only ever half-baked to begin with, why keep it around?

[brianleroux]

@JamesMGreene thats how this whole browser thing works! if a feature is shipped in a browser, used by many apps/sites, and then rescinded, the internet breaks. we don't want that.

[JamesMGreene]

I understand that bit of it but it seems doubtful to me that WebSQL has prolific use on the web. Have browser vendors seen usage stats suggesting otherwise? 😕

[brianleroux]

frankly: yes.

[JamesMGreene]

Alright, then I guess PhantomJS needs to stay on the bandwagon as well.

[JamesMGreene]

There ya go, @RockLi, it's all yours if you can find the time this weekend. :)

[RockLi]

okey, I got it.

[JamesMGreene]

@RockLi Any weekend update?

[RockLi]

@JamesMGreene haven't finished yet, will continue working on that these evenings, and send out new PR ASAP.

[JamesMGreene]

Thank you, looking forward to it!

[johanlunds]

I tried building with this patch applied, but still get window.indexedDB || window.webkitIndexedDB as undefined. Anyone can confirm this or am I doing it wrong?

Gist: https://gist.github.com/johanlunds/5911808

Working in browser. Running in terminal with ./phantomjs /Users/johanlundstrom/.rbenv/versions/2.0.0-p195/lib/ruby/gems/2.0.0/gems/guard-jasmine-1.16.0/lib/guard/jasmine/phantomjs/guard-jasmine.js http:https://localhost:5000/test.html passes test for localStorage but not indexedDb. Output:

{"passed":false,"stats":{"failures":1,"specs":2,"time":0.009},"suites":[{"description":"offline storage","passed":false,"specs":[{"description":"localstorage exists","passed":true},{"description":"indexedDb exists","messages":["Expected undefined to be defined."],"passed":false,"logs":[""]}],"suites":[]}]}

[johanlunds]

Okay, I simplified the test a bit. Gist: https://gist.github.com/johanlunds/5912015. Running with ./phantomjs phantomjs_script.js http:https://localhost:5000/test.html gives output:

localstorage type:  object undefined undefined
indexedDb type:  undefined undefined undefined

[vitallium]

@johanlunds IndexedDB is disabled by default in QtWebkit.

[johanlunds]

But the patch has it on by default. 
—
Sent from my iPhone

On Tue, Jul 2, 2013 at 9:27 PM, Vitaliy Slobodin [email protected]
wrote:

@johanlunds IndexedDB is disabled by default in QtWebkit.

Reply to this email directly or view it on GitHub:
#404 (comment)

[vitallium]

@johanlunds do you mean this pull request? It's incorrect.

[johanlunds]

Do you mean the patch or my interpretation of the code is incorrect? What command should I invoke then? ./phantomjs --indexed-db=true phantomjs_script.js http:https://localhost:5000/test.html returns the same result.

I have read the comments in this PR and I think I've understood them.

[vitallium]

I'm talking about the patch. Support for IndexedDB is disabled by default. You can't enable it using this patch.

As a side note: IndexedDB disabled because it's broken in the current Webkit version.

[johanlunds]

Okay, thanks. Do you have any helpful pointers if I would like to investigate writing my own patch? To be clear: What I'm after is a fix to enable IndexedDb support in PhantomJS.

[vitallium]

@johanlunds Well, it's complicated. In the current Webkit version some features are designed as modules (http:https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/) IndexedDB is one of them (http:https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/indexeddb). To enable IndexedDB, you need to backport it. I don't think this is even possible, because you'd need to resolve a lot of references or write some code from scratch.
Or, you can enable IndexedDB to see in which state it is currently.
You can do it by adding this line to file features.pri:

!contains(DEFINES, ENABLE_INDEXED_DATABASE=.): DEFINES += ENABLE_INDEXED_DATABASE=1

[johanlunds]

That's what I started to realize after doing some quick research on QTWebKit. That's too bad. This is probably way too big of an undertaking for me (and too difficult).

[WoZ]

@RockLi has error in the branch. Nor enabling and disabling of AppCache, LocalStorage and indexedDB via cli args will not work. A small patch below.

diff --git a/src/config.cpp b/src/config.cpp
index 95a93a0..1a4600c 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -650,6 +650,9 @@ void Config::handleOption(const QString &option, const QVariant &value)
     booleanFlags << "local-to-remote-url-access";
     booleanFlags << "remote-debugger-autorun";
     booleanFlags << "web-security";
+    booleanFlags << "appcache";
+    booleanFlags << "indexed-db";
+    booleanFlags << "local-storage";
     if (booleanFlags.contains(option)) {
         if ((value != "true") && (value != "yes") && (value != "false") && (value != "no")) {
             setUnknownOption(QString("Invalid values for '%1' option.").arg(option));

Also I'm trying to change location of indexedDB (indexed-db-path arg). No luck. Databases.db file and sitename/0000*.db files are still located in ~/.qws/share/data/Ofi Labs/PhantomJS dir. Any suggestions?

[vitallium]

@WoZ IndexedDB is disabled by default. And QtWebkit doesn't provide you an API to change the IndexedDB path.
The only way to change the path is by using GroupSettings for a WebPage. But again, QtWebkit doesn't have any public API for that. Actually, that's why IndexedDB is disabled.

[WoZ]

@vitallium What does http:https://qt-project.org/doc/qt-4.8/qwebsettings.html#setOfflineStoragePath do? Does it affect indexedDB and WebSQL?

[vitallium]

@WoZ This option is only for HTML5 Local Storage and WebSQL. And it doesn't affect IndexedDB.

[johanlunds]

@WoZ @vitallium Sorry, I'm no expert but I did some research (see comments
above). QTWebKit doesn't have any IndexedDb support, so there's no way for
you to "enable it". It has WebSQL support though, and I think that's what
"offlineStorage" refers to. As far as I can see Chrome is the only Webkit
that does have IndexedDb support (using LevelDB).

Mvh,
Johan Lundström

[email protected]

On Fri, Jul 19, 2013 at 5:02 PM, Vitaliy Slobodin
[email protected]:

@WoZ https://github.com/WoZ This option is only for HTML5 Local Storage
and WebSQL. And it doesn't affect IndexedDB.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/404#issuecomment-21254898
.

[WoZ]

@vitallium My tests give me another info. setOfflineStoragePath affects only local storage, WebSQL databases still creating in ~/.qws/share/data/Ofi Labs/PhantomJS.

.
├── Databases.db
└── http_test.com_0
    └── 0000000000000001.db

[samccone]

+1

[JamesMGreene]

BUMP

[joelclimbsthings]

+1

[vitallium]

Closing this one because it's invalid and old. Will be implemented later.