SavedFilesService interface in //extensions

apps/app_restore_service_browsertest.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "apps/app_restore_service.h"
 #include "apps/app_restore_service_factory.h"
-#include "apps/saved_files_service.h"
+#include "apps/saved_files_service_impl.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/extensions/api/file_system/file_system_api.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/api/file_system/saved_files_service.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/extension_test_message_listener.h"
 
 using extensions::Extension;
 using extensions::ExtensionPrefs;
 using extensions::ExtensionSystem;
 using extensions::FileSystemChooseEntryFunction;
+using extensions::SavedFilesService;
 
 // TODO(benwells): Move PlatformAppBrowserTest to apps namespace in apps
 // component.
 using extensions::PlatformAppBrowserTest;
 
 namespace apps {
 
 // Tests that a running app is recorded in the preferences as such.
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, RunningAppsAreRecorded) {
   content::WindowedNotificationObserver extension_suspended(
@@ skipping 91 matching lines @@
 
   FileSystemChooseEntryFunction::SkipPickerAndAlwaysSelectPathForTest(
       &temp_file);
   FileSystemChooseEntryFunction::RegisterTempExternalFileSystemForTest(
       "temp", temp_directory.GetPath());
 
   const Extension* extension = LoadAndLaunchPlatformApp(
       "file_access_saved_to_prefs_test", "fileWritten");
   ASSERT_TRUE(extension);
 
-  SavedFilesService* saved_files_service = SavedFilesService::Get(profile());
+  SavedFilesServiceImpl* saved_files_service =

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

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 20:44:04]

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.

+      SavedFilesServiceImpl::Get(profile());
 
-  std::vector<SavedFileEntry> file_entries =
+  std::vector<SavedFilesService::Entry> file_entries =
       saved_files_service->GetAllFileEntries(extension->id());
   // One for the read-only file entry and one for the writable file entry.
   ASSERT_EQ(2u, file_entries.size());
 
   extension_suspended.Wait();
   file_entries = saved_files_service->GetAllFileEntries(extension->id());
   // File entries should be cleared when the extension is suspended.
   ASSERT_TRUE(file_entries.empty());
 }
 
@@ skipping 23 matching lines @@
 
   ExtensionTestMessageListener access_ok_listener(
       "restartedFileAccessOK", false);
 
   const Extension* extension =
       LoadAndLaunchPlatformApp("file_access_restored_test", "fileWritten");
   ASSERT_TRUE(extension);
 
   ExtensionPrefs* extension_prefs =
       ExtensionPrefs::Get(browser()->profile());
-  SavedFilesService* saved_files_service = SavedFilesService::Get(profile());
-  std::vector<SavedFileEntry> file_entries =
+  SavedFilesServiceImpl* saved_files_service =
+      SavedFilesServiceImpl::Get(profile());
+  std::vector<SavedFilesService::Entry> file_entries =
       saved_files_service->GetAllFileEntries(extension->id());
   extension_suspended.Wait();
 
   // Simulate a restart by populating the preferences as if the browser didn't
   // get time to clean itself up.
   extension_prefs->SetExtensionRunning(extension->id(), true);
-  for (std::vector<SavedFileEntry>::const_iterator it = file_entries.begin();
+  for (std::vector<SavedFilesService::Entry>::const_iterator it =
+           file_entries.begin();
        it != file_entries.end(); ++it) {
     saved_files_service->RegisterFileEntry(
         extension->id(), it->id, it->path, it->is_directory);
   }
 
   apps::AppRestoreServiceFactory::GetForBrowserContext(browser()->profile())
       ->HandleStartup(true);
 
   access_ok_listener.WaitUntilSatisfied();
 }
 
 }  // namespace apps