Source/modules/quota/DEPS - Issue 1154573005: Expose Durable Storage API to script

Side by Side Diff: Source/modules/quota/DEPS

Issue 1154573005: Expose Durable Storage API to script (Closed) Base URL: svn://svn.chromium.org/blink/trunk

Patch Set: update comment Created 5 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

« LayoutTests/storage/quota/durability-basics.html ('K') | « Source/modules/modules.gypi ('k') | Source/modules/quota/NavigatorStorageQuota.h » ('j') | Source/modules/quota/NavigatorStorageQuota.idl » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 include_rules = [	1 include_rules = [

2 "+bindings",	2 "+bindings",

3 "+core",	3 "+core",

4 "+heap",	4 "+heap",

5 "-modules",	5 "-modules",

6 "+modules/ModulesExport.h",	6 "+modules/ModulesExport.h",

	7 # TODO(mlamouri): Remove modules/permissions. https://crbug.com/510948

	8 "+modules/permissions",
	mlamouri (slow - plz ping) 2015/08/04 13:15:19 I don't think that's right. The end goal of https: I don't think that's right. The end goal of https://crbug.com/510948 isn't to remove modules/permissions. dgrogan 2015/08/06 23:52:02 That issue is the only reason we need this extra d Show quoted text On 2015/08/04 13:15:19, Mounir Lamouri wrote: > I don't think that's right. The end goal of https://crbug.com/510948 isn't to > remove modules/permissions. That issue is the only reason we need this extra dependency. How's the updated phrasing? mlamouri (slow - plz ping) 2015/08/07 10:19:14 You need the dependency to access the PermissionCl Show quoted text On 2015/08/06 at 23:52:02, dgrogan wrote: > On 2015/08/04 13:15:19, Mounir Lamouri wrote: > > I don't think that's right. The end goal of https://crbug.com/510948 isn't to > > remove modules/permissions. > > That issue is the only reason we need this extra dependency. > > How's the updated phrasing? You need the dependency to access the PermissionClient. You might very likely need to use the permissions module for that anyway. dgrogan 2015/08/07 17:24:54 I don't think that's true. StorageManager.cpp's in Show quoted text On 2015/08/07 10:19:14, Mounir Lamouri wrote: > On 2015/08/06 at 23:52:02, dgrogan wrote: > > On 2015/08/04 13:15:19, Mounir Lamouri wrote: > > > I don't think that's right. The end goal of https://crbug.com/510948 isn't > to > > > remove modules/permissions. > > > > That issue is the only reason we need this extra dependency. > > > > How's the updated phrasing? > > You need the dependency to access the PermissionClient. I don't think that's true. StorageManager.cpp's include of public/platform/modules/permissions/WebPermissionClient.h should suffice. Show quoted text > You might very likely > need to use the permissions module for that anyway. dgrogan 2015/08/07 17:37:56 To add more detail, I imagine that 510948 is fixed Show quoted text On 2015/08/07 17:24:54, dgrogan wrote: > On 2015/08/07 10:19:14, Mounir Lamouri wrote: > > On 2015/08/06 at 23:52:02, dgrogan wrote: > > > On 2015/08/04 13:15:19, Mounir Lamouri wrote: > > > > I don't think that's right. The end goal of https://crbug.com/510948 isn't > > to > > > > remove modules/permissions. > > > > > > That issue is the only reason we need this extra dependency. > > > > > > How's the updated phrasing? > > > > You need the dependency to access the PermissionClient. > > I don't think that's true. StorageManager.cpp's include of > public/platform/modules/permissions/WebPermissionClient.h should suffice. To add more detail, I imagine that 510948 is fixed by adding a method somewhere that takes an ExecutionContext and returns a WebPermissionClient. Right now that method lives in modules/permissions, hence the need for the extra DEPS line. We could move that method to WebPermissionClient. Or maybe we add a new method to ExecutionContext itself: `WebPermissionClient* ExecutionContext::getPermissionClient()`. Such a method would also obviate the need for this extra DEPS line. I don't know, maybe this extra DEPS line isn't such a big deal. Can one module depend on another? +haraken for his thoughts on that.
7 "+modules/quota",	9 "+modules/quota",

8 "+platform",	10 "+platform",

9 "+public/platform",	11 "+public/platform",

10 "-web",	12 "-web",

11 ]	13 ]

OLD	NEW