SavedFilesService interface in //extensions

SavedFilesService interface in //extensions

CLOSED: see https://codereview.chromium.org/2960433002/

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL:

* Adds an extensions::SavedFilesServiceInterface, consisting of a few
  functions from apps::SavedFilesService
* Makes apps::SavedFilesService derive from
  extensions::SavedFilesServiceInterface
* Moves apps::SavedFileEntry struct into extensions::

BUG=729713
R=benwells@chromium.org

[michaelpg, 2017-06-20 23:23:23]

Description was changed from

==========
SavedFilesService interface in //extensions

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL moves a few functions into an interface in //extensions, which is
called extensions::SavedFilesService. The existing apps::SavedFilesService
is now an "Impl": apps::SavedFilesServiceImpl.

BUG=729713
R=benwells@chromium.org
==========

to

==========
SavedFilesService interface in //extensions

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL:

* Adds an extensions::SavedFilesService interface, consisting of a few
  functions from the original apps::SavedFilesService.
* Renames apps::SavedFilesService => apps::SavedFilesServiceImpl and
  derives it from extensions::SavedFilesService
* Moves apps::SavedFileEntry struct into extensions::SavedFilesService
  as Entry

BUG=729713
R=benwells@chromium.org
==========

[michaelpg, 2017-06-20 23:24:20]

PTAL. I believe this allows for what you suggested in
https://codereview.chromium.org/2934143002/diff/60001/extensions/browser/api/...

[benwells, 2017-06-22 07:56:09]

https://codereview.chromium.org/2951003002/diff/20001/apps/app_restore_servic...
File apps/app_restore_service_browsertest.cc (right):

https://codereview.chromium.org/2951003002/diff/20001/apps/app_restore_servic...
apps/app_restore_service_browsertest.cc:136: SavedFilesServiceImpl*
saved_files_service =
Hmmm ... getting an Impl class like this looks a bit strange. After seeing this
I think the second suggestion I had wrt naming would be better.

That is, keep SavedFilesService as it is (i.e. no SavedFilesService) but
introduce a SavedFilesServiceInterface.

Then users of the class would not be dealing with Impls but 'proper' classes. It
would also make the fallout from this change much less.

WDYT?

PS sorry to be a pain.

[michaelpg, 2017-06-23 18:31:46]

Description was changed from

==========
SavedFilesService interface in //extensions

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL:

* Adds an extensions::SavedFilesService interface, consisting of a few
  functions from the original apps::SavedFilesService.
* Renames apps::SavedFilesService => apps::SavedFilesServiceImpl and
  derives it from extensions::SavedFilesService
* Moves apps::SavedFileEntry struct into extensions::SavedFilesService
  as Entry

BUG=729713
R=benwells@chromium.org
==========

to

==========
SavedFilesService interface in //extensions

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL:

* Adds an extensions::SavedFilesServiceInterface, consisting of a few
  functions from apps::SavedFilesService
* Makes apps::SavedFilesService derive from
  extensions::SavedFilesServiceInterface
* Moves apps::SavedFileEntry struct into extensions::

BUG=729713
R=benwells@chromium.org
==========

[michaelpg, 2017-06-23 20:23:11]

Description was changed from

==========
SavedFilesService interface in //extensions

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL:

* Adds an extensions::SavedFilesServiceInterface, consisting of a few
  functions from apps::SavedFilesService
* Makes apps::SavedFilesService derive from
  extensions::SavedFilesServiceInterface
* Moves apps::SavedFileEntry struct into extensions::

BUG=729713
R=benwells@chromium.org
==========

to

==========
SavedFilesService interface in //extensions

CLOSED: see https://codereview.chromium.org/2960433002/

apps::SavedFilesService provides apps with a LRU queue of "retained"
file entries they are allowed access to. See chrome.fileSystem API docs:
https://developer.chrome.com/apps/fileSystem#method-retainEntry

Chrome extension APIs can already use the service in //apps, but APIs
implemented in //extensions cannot (they can't depend on //apps). This
change enables moving chrome.fileSystem into //extensions.

This CL:

* Adds an extensions::SavedFilesServiceInterface, consisting of a few
  functions from apps::SavedFilesService
* Makes apps::SavedFilesService derive from
  extensions::SavedFilesServiceInterface
* Moves apps::SavedFileEntry struct into extensions::

BUG=729713
R=benwells@chromium.org
==========

[michaelpg, 2017-06-23 20:44:04]

Rietveld is forcing me to create a new issue for this for some reason, so I've
closed this one and uploaded to https://codereview.chromium.org/2960433002/.

https://codereview.chromium.org/2951003002/diff/20001/apps/app_restore_servic...
File apps/app_restore_service_browsertest.cc (right):

https://codereview.chromium.org/2951003002/diff/20001/apps/app_restore_servic...
apps/app_restore_service_browsertest.cc:136: SavedFilesServiceImpl*
saved_files_service =
On 2017/06/22 07:56:09, benwells wrote:
> Hmmm ... getting an Impl class like this looks a bit strange. After seeing
this
> I think the second suggestion I had wrt naming would be better.
> 
> That is, keep SavedFilesService as it is (i.e. no SavedFilesService) but
> introduce a SavedFilesServiceInterface.
> 
> Then users of the class would not be dealing with Impls but 'proper' classes.
It
> would also make the fallout from this change much less.
> 
> WDYT?
> 
> PS sorry to be a pain.

Done. It does make the change tighter, thanks. I also reverted the
SavedFileEntry => ::Entry change because SavedFilesServiceInterface::Entry was a
bit much. SavedFileEntry now has its own header file though.