Implement --unlimited-storage switch for crosswalk #3958

alxndrsn · 2016-11-17T15:00:58Z

When this switch is added to the crosswalk command-line, a SpecialStoragePolicy
is enabled which allows crosswalk to use the total free space available on the
device, instead of Chromium's standard of 10% of available space.

Implementation of https://crosswalk-project.org/jira/browse/XWALK-7413

rakuco · 2016-11-18T10:48:49Z

ping @lincsoon @xzhan96
@alxndrsn: thanks for the patch. can you add the appropriate copyright headers to the files you've added?

crosswalk-trybot · 2016-11-18T14:21:22Z

Testing patch series with medic/crosswalk@df296ff as its head.

Bot	Status
Crosswalk Android-X86	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4868)
Crosswalk Android x86-64	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1918)
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4858)

alxndrsn · 2016-11-18T14:22:14Z

Thanks, @rakuco. copyright messages added; I hope these are correct.

crosswalk-trybot · 2016-11-21T15:38:30Z

Testing patch series with medic/crosswalk@00dac1a as its head.

Bot	Status
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4869)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1919)
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4859)

sunlin-link · 2016-11-23T07:23:11Z

@fujunwei Please have a look

fujunwei · 2016-11-24T01:23:41Z

runtime/browser/xwalk_browser_context.cc

@@ -212,6 +213,14 @@ XWalkBrowserContext::GetGuestManager() {
 }

 storage::SpecialStoragePolicy* XWalkBrowserContext::GetSpecialStoragePolicy() {
+  LOG(WARNING) << "XWalkBrowserContext::GetSpecialStoragePolicy() :: " <<


Simplify or move the LOG.

fujunwei · 2016-11-24T01:24:20Z

runtime/browser/xwalk_browser_context.cc

+  // TODO cache the XwalkSpecialStoragePolicy instance?
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kUnlimitedStorage))
+    return new xwalk::XWalkSpecialStoragePolicy();


Cache the instance.

I'll have a go at implementing this now, but some questions:

Should it be initialised lazily?

If so, does initialisation need to be thread-safe?

For now, I will assume that initialising the XWalkSpecialStoragePolicy in the constructor of XWalkBrowserContext is appropriate.

Instance now created at XWalkBrowserContext instantiation time iff required.

fujunwei · 2016-11-24T01:26:51Z

runtime/browser/xwalk_special_storage_policy.cc

+
+#include <iostream>
+
+#include "xwalk/runtime/browser/xwalk_special_storage_policy.h"


First statement

xzhan96 · 2016-11-24T01:56:41Z

Use BUG=XWALK-7413 instead of "Implementation of https://crosswalk-project.org/jira/browse/XWALK-7413"

crosswalk-trybot · 2016-11-24T16:39:35Z

Testing patch series with medic/crosswalk@2da9da4 as its head.

Bot	Status
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4870)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1920)
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4860)

crosswalk-trybot · 2016-11-24T16:40:28Z

Testing patch series with medic/crosswalk@0cdf78b as its head.

Bot	Status
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4861)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1921)
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4871)

crosswalk-trybot · 2016-11-24T16:41:16Z

Testing patch series with medic/crosswalk@166cc1b as its head.

Bot	Status
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4862)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1922)
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4872)

crosswalk-trybot · 2016-11-25T18:22:24Z

Testing patch series with medic/crosswalk@02d2601 as its head.

Bot	Status
Crosswalk Android-X86	[In Progress](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4873)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1923)
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4863)

rakuco · 2016-11-30T09:52:26Z

@lincsoon @fujunwei @xzhan96: re-ping

xzhan96 · 2016-12-01T01:36:41Z

LGTM

fujunwei · 2016-12-01T02:46:30Z

LGTM

rakuco

There are just some minor adjustments left before we're able to merge this. Sorry it's taking so long.

rakuco · 2016-12-01T14:24:45Z

runtime/browser/xwalk_browser_context.h

@@ -147,6 +147,8 @@ class XWalkBrowserContext
  std::unique_ptr<XWalkSSLHostStateDelegate> ssl_host_state_delegate_;
  std::unique_ptr<content::PermissionManager> permission_manager_;

+  storage::SpecialStoragePolicy* special_storiage_policy_;


It looks like this is being leaked; I think you can follow what we do with the pointers above this one and use a std::unique_ptr here. I also think you can instantiate it in GetSpecialStoragePolicy() itself like the other getters do.

Done, and I fixed the spelling mistake too :¬)

rakuco · 2016-12-01T14:28:05Z

runtime/browser/xwalk_special_storage_policy.cc

+
+#include "xwalk/runtime/browser/xwalk_special_storage_policy.h"
+
+#include <iostream>


I don't think this include is required.

Agreed, and removed.

crosswalk-trybot · 2016-12-01T16:56:47Z

Testing patch series with medic/crosswalk@a168292 as its head.

Bot	Status
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4874)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1924)
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4864)

crosswalk-trybot · 2016-12-01T17:03:20Z

Testing patch series with medic/crosswalk@24a14c5 as its head.

Bot	Status
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4865)
Crosswalk Android x86-64	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1925)
Crosswalk Android-X86	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4875)

rakuco

lgtm!

rakuco

Almost there, std::unique_ptr seems to have broken the build.

rakuco · 2016-12-01T17:08:04Z

runtime/browser/xwalk_special_storage_policy.h

+  bool IsStorageDurable(const GURL& origin) override;
+
+ protected:
+  ~XWalkSpecialStoragePolicy() override;


Hmm, this is breaking the build now that you're using a unique_ptr.

Ah, just saw this. I'll take a look.

Hey @rakuco I don't understand the compile error. Is it clear to you what's wrong?

crosswalk-trybot · 2016-12-01T17:59:29Z

Testing patch series with medic/crosswalk@e158f39 as its head.

Bot	Status
Crosswalk Android-X86	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4876)
Crosswalk Android x86-64	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1926)
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4866)

crosswalk-trybot · 2016-12-01T18:17:27Z

Testing patch series with medic/crosswalk@0a6f06e as its head.

Bot	Status
Crosswalk Android-X86	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4877)
Crosswalk Android x86-64	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1927)
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4867)

rakuco · 2016-12-01T19:33:24Z

Looking at the latest build error, I think you'll need to use a scoped_refptr here like the code in Chromium does: see ProfileImpl::GetExtensionSpecialStoragePolicy() in src/chrome, for example (the code basically remains the same, you just need to switch to a different smart pointer type).

crosswalk-trybot · 2016-12-02T11:26:07Z

Testing patch series with medic/crosswalk@3d52973 as its head.

Bot	Status
Crosswalk Android-X86	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4878)
Crosswalk Android x86-64	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1928)
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4868)

rakuco · 2016-12-02T11:31:32Z

runtime/browser/xwalk_browser_context.h

@@ -146,6 +146,7 @@ class XWalkBrowserContext
  PartitionPathContextGetterMap context_getters_;
  std::unique_ptr<XWalkSSLHostStateDelegate> ssl_host_state_delegate_;
  std::unique_ptr<content::PermissionManager> permission_manager_;
+  scoped_refptr<ExtensionSpecialStoragePolicy> special_storage_policy_;


I thiknk you mean scoped_refptr<XWalkSpecialStoragePolicy> here.

crosswalk-trybot · 2016-12-02T12:30:22Z

Testing patch series with medic/crosswalk@3ab2048 as its head.

Bot	Status
Crosswalk Android-X86	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4879)
Crosswalk Android x86-64	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1929)
Crosswalk Linux	[FAILED 💔](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4869)

rakuco · 2016-12-02T12:33:46Z

runtime/browser/xwalk_browser_context.h

@@ -146,6 +146,7 @@ class XWalkBrowserContext
  PartitionPathContextGetterMap context_getters_;
  std::unique_ptr<XWalkSSLHostStateDelegate> ssl_host_state_delegate_;
  std::unique_ptr<content::PermissionManager> permission_manager_;
+  scoped_refptr<XWalkSpecialStoragePolicy> special_storage_policy_;


You forgot to include the appropriate header declaring XWalkSpecialStoragePolicy...

When this switch is added to the crosswalk command-line, a SpecialStoragePolicy is enabled which allows crosswalk to use the total free space available on the device, instead of Chromium's standard of 10% of available space. BUG=XWALK-7413

crosswalk-trybot · 2016-12-02T13:31:51Z

Testing patch series with medic/crosswalk@716cad2 as its head.

Bot	Status
Crosswalk Android-X86	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android-X86/builds/4880)
Crosswalk Android x86-64	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Android x86-64/builds/1930)
Crosswalk Linux	[SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Crosswalk Linux/builds/4870)

rakuco

Success at last!

alxndrsn · 2016-12-02T14:00:13Z

Great! Thanks for your patience :¬)

rakuco · 2016-12-02T14:41:05Z

@alxndrsn: there was just one Android test failure (unrelated to your change) reported in our Buildbots.

Suggestions:

Use git cherry-pick -x and cherry-pick your commit to the crosswalk-23 branch.
Once it lands, close XWALK-7413 and set the "Fix version/s" field accordingly.

alxndrsn · 2016-12-02T17:45:19Z

@rakuco like this? #3961

alxndrsn force-pushed the patch-time branch from 84e58ea to df296ff Compare November 18, 2016 14:21

alxndrsn force-pushed the patch-time branch from df296ff to 00dac1a Compare November 21, 2016 15:38

fujunwei reviewed Nov 24, 2016

View reviewed changes

alxndrsn mentioned this pull request Nov 24, 2016

Provide a way to use a bundled version of Chromium 51+ in medic-android medic/cht-core#2801

Closed

alxndrsn force-pushed the patch-time branch from 00dac1a to 2da9da4 Compare November 24, 2016 16:39

alxndrsn force-pushed the patch-time branch from 2da9da4 to 0cdf78b Compare November 24, 2016 16:40

alxndrsn force-pushed the patch-time branch from 0cdf78b to 166cc1b Compare November 24, 2016 16:41

alxndrsn force-pushed the patch-time branch from 166cc1b to 02d2601 Compare November 25, 2016 18:22

rakuco suggested changes Dec 1, 2016

View reviewed changes

alxndrsn force-pushed the patch-time branch from 02d2601 to a168292 Compare December 1, 2016 16:56

alxndrsn force-pushed the patch-time branch from a168292 to 24a14c5 Compare December 1, 2016 17:03

rakuco approved these changes Dec 1, 2016

View reviewed changes

rakuco suggested changes Dec 1, 2016

View reviewed changes

alxndrsn force-pushed the patch-time branch from 24a14c5 to e158f39 Compare December 1, 2016 17:59

alxndrsn force-pushed the patch-time branch from e158f39 to 0a6f06e Compare December 1, 2016 18:17

alxndrsn force-pushed the patch-time branch from 0a6f06e to 3d52973 Compare December 2, 2016 11:26

rakuco reviewed Dec 2, 2016

View reviewed changes

alxndrsn force-pushed the patch-time branch from 3d52973 to 3ab2048 Compare December 2, 2016 12:30

rakuco reviewed Dec 2, 2016

View reviewed changes

alxndrsn force-pushed the patch-time branch from 3ab2048 to 716cad2 Compare December 2, 2016 13:31

rakuco approved these changes Dec 2, 2016

View reviewed changes

rakuco merged commit 45837ec into crosswalk-project:master Dec 2, 2016

alxndrsn mentioned this pull request Dec 2, 2016

Implement --unlimited-storage switch for crosswalk #3961

Merged


		#include <iostream>

		#include "xwalk/runtime/browser/xwalk_special_storage_policy.h"

Implement --unlimited-storage switch for crosswalk #3958

Implement --unlimited-storage switch for crosswalk #3958

Conversation

alxndrsn commented Nov 17, 2016

rakuco commented Nov 18, 2016

crosswalk-trybot commented Nov 18, 2016 • edited Loading

alxndrsn commented Nov 18, 2016 • edited Loading

crosswalk-trybot commented Nov 21, 2016 • edited Loading

sunlin-link commented Nov 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alxndrsn Nov 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xzhan96 commented Nov 24, 2016

crosswalk-trybot commented Nov 24, 2016 • edited Loading

crosswalk-trybot commented Nov 24, 2016 • edited Loading

crosswalk-trybot commented Nov 24, 2016 • edited Loading

crosswalk-trybot commented Nov 25, 2016 • edited Loading

rakuco commented Nov 30, 2016

xzhan96 commented Dec 1, 2016

fujunwei commented Dec 1, 2016

rakuco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crosswalk-trybot commented Dec 1, 2016 • edited Loading

crosswalk-trybot commented Dec 1, 2016 • edited Loading

rakuco left a comment

Choose a reason for hiding this comment

rakuco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crosswalk-trybot commented Dec 1, 2016 • edited Loading

crosswalk-trybot commented Dec 1, 2016 • edited Loading

rakuco commented Dec 1, 2016

crosswalk-trybot commented Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

crosswalk-trybot commented Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

crosswalk-trybot commented Dec 2, 2016 • edited Loading

rakuco left a comment

Choose a reason for hiding this comment

alxndrsn commented Dec 2, 2016

rakuco commented Dec 2, 2016

alxndrsn commented Dec 2, 2016

crosswalk-trybot commented Nov 18, 2016 •

edited

Loading

alxndrsn commented Nov 18, 2016 •

edited

Loading

crosswalk-trybot commented Nov 21, 2016 •

edited

Loading

alxndrsn Nov 24, 2016 •

edited

Loading

crosswalk-trybot commented Nov 24, 2016 •

edited

Loading

crosswalk-trybot commented Nov 24, 2016 •

edited

Loading

crosswalk-trybot commented Nov 24, 2016 •

edited

Loading

crosswalk-trybot commented Nov 25, 2016 •

edited

Loading

crosswalk-trybot commented Dec 1, 2016 •

edited

Loading

crosswalk-trybot commented Dec 1, 2016 •

edited

Loading

crosswalk-trybot commented Dec 1, 2016 •

edited

Loading

crosswalk-trybot commented Dec 1, 2016 •

edited

Loading

crosswalk-trybot commented Dec 2, 2016 •

edited

Loading

crosswalk-trybot commented Dec 2, 2016 •

edited

Loading

crosswalk-trybot commented Dec 2, 2016 •

edited

Loading