Support for file listing in privet local filesystem

Support for file listing in privet local filesystem

List files locally using privet.

BUG=338868
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251936

[Noam Samuel, 2014-01-28 23:18:57]

vitalybuka@ for general review
thestig@ for chrome_content_client.cc changes

[Noam Samuel, 2014-01-28 23:19:34]

+thestig@ for real this time

[Lei Zhang, 2014-01-28 23:22:58]

chrome_content_browser_client.cc lgtm

https://codereview.chromium.org/141703022/diff/1/chrome/chrome_tests_unit.gypi
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/141703022/diff/1/chrome/chrome_tests_unit.gyp...
chrome/chrome_tests_unit.gypi:1014:
'browser/local_discovery/storage/path_util_unittest.cc',
nit: alphabetical order

[Vitaly Buka (NO REVIEWS), 2014-02-03 23:32:32]

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
File chrome/browser/local_discovery/storage/path_util_unittest.cc (right):

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/path_util_unittest.cc:12:
"/privet/MyId._privet._tcp.local/Some name/some/path")));
why not to use URI instead of fake FilePath

privet://MyId._privet._tcp.local/Some name/some/path

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.h (right):

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/privet_filesystem_backend.h:24: explicit
PrivetFileSystemBackend(fileapi::ExternalMountPoints* mount_points,
-explicit

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
File chrome/browser/local_discovery/storage/privet_filesystem_operations.cc
(right):

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/privet_filesystem_operations.cc:22:
delegate_(delegate) {
it would be nice to have DCHECK() for threads in beginning of each method

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/privet_filesystem_operations.cc:123:
weak_factory_.GetWeakPtr(),
race condition again

[Noam Samuel, 2014-02-04 00:06:36]

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
File chrome/browser/local_discovery/storage/path_util_unittest.cc (right):

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/path_util_unittest.cc:12:
"/privet/MyId._privet._tcp.local/Some name/some/path")));
On 2014/02/03 23:32:33, Vitaly Buka wrote:
> why not to use URI instead of fake FilePath
> 
> privet://MyId._privet._tcp.local/Some name/some/path

Hm. I don't think that would work. The VolumeManager uses file paths as its data
model, and as far as I can tell, while there is some sort of model for URLs in
the webkit filesystem API itself, it's mostly to provide an "origin" for
per-domain storage, not to create multiple actual protocols.

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
File chrome/browser/local_discovery/storage/privet_filesystem_backend.h (right):

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/privet_filesystem_backend.h:24: explicit
PrivetFileSystemBackend(fileapi::ExternalMountPoints* mount_points,
On 2014/02/03 23:32:33, Vitaly Buka wrote:
> -explicit

Done.

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
File chrome/browser/local_discovery/storage/privet_filesystem_operations.cc
(right):

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/privet_filesystem_operations.cc:22:
delegate_(delegate) {
On 2014/02/03 23:32:33, Vitaly Buka wrote:
> it would be nice to have DCHECK() for threads in beginning of each method

Done.

https://codereview.chromium.org/141703022/diff/20001/chrome/browser/local_dis...
chrome/browser/local_discovery/storage/privet_filesystem_operations.cc:123:
weak_factory_.GetWeakPtr(),
On 2014/02/03 23:32:33, Vitaly Buka wrote:
> race condition again

OK for this one. Will fix on next one.

[Vitaly Buka (NO REVIEWS), 2014-02-04 00:51:47]

lgtm

[Noam Samuel, 2014-02-04 00:52:14]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-04 00:53:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/80001

[commit-bot: I haz the power, 2014-02-04 00:53:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 00:53:31]

Failed to apply patch for chrome/chrome_browser.gypi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/chrome_browser.gypi
  Hunk #1 FAILED at 1003.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/chrome_browser.gypi.rej

Patch:       chrome/chrome_browser.gypi
Index: chrome/chrome_browser.gypi
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index
3a5eeb6445093290231e6b35c8e74b8bd436697d..b38f2bde69049259b8af73ae8d51b90432a4de06
100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1003,12 +1003,16 @@
         'browser/local_discovery/service_discovery_device_lister.h',
         'browser/local_discovery/service_discovery_shared_client.cc',
         'browser/local_discovery/service_discovery_shared_client.h',
+        'browser/local_discovery/storage/path_util.cc',
+        'browser/local_discovery/storage/path_util.h',
         'browser/local_discovery/storage/privet_filesystem_async_util.cc',
         'browser/local_discovery/storage/privet_filesystem_async_util.h',
         'browser/local_discovery/storage/privet_filesystem_backend.cc',
         'browser/local_discovery/storage/privet_filesystem_backend.h',
         'browser/local_discovery/storage/privet_filesystem_constants.cc',
         'browser/local_discovery/storage/privet_filesystem_constants.h',
+        'browser/local_discovery/storage/privet_filesystem_operations.cc',
+        'browser/local_discovery/storage/privet_filesystem_operations.h',
         'browser/local_discovery/storage/privet_volume_lister.cc',
         'browser/local_discovery/storage/privet_volume_lister.h',
         'browser/mac/dock.h',

[commit-bot: I haz the power, 2014-02-04 00:53:33]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 00:53:36]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 00:53:38]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 00:53:38]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 00:53:51]

CQ bit was unchecked on CL. Ignoring.

[Noam Samuel, 2014-02-04 19:04:17]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-04 19:13:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/270001

[commit-bot: I haz the power, 2014-02-04 19:13:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-04 19:13:31]

Failed to apply patch for chrome/chrome_browser.gypi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/chrome_browser.gypi
  Hunk #1 FAILED at 1012.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/chrome_browser.gypi.rej

Patch:       chrome/chrome_browser.gypi
Index: chrome/chrome_browser.gypi
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index
09932b80f68419be39810df3e11f8267bd3c8938..a1bd9cdf733d618733652d0d26fcc03edf90dc6d
100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1012,12 +1012,16 @@
         'browser/local_discovery/service_discovery_device_lister.h',
         'browser/local_discovery/service_discovery_shared_client.cc',
         'browser/local_discovery/service_discovery_shared_client.h',
+        'browser/local_discovery/storage/path_util.cc',
+        'browser/local_discovery/storage/path_util.h',
         'browser/local_discovery/storage/privet_filesystem_async_util.cc',
         'browser/local_discovery/storage/privet_filesystem_async_util.h',
         'browser/local_discovery/storage/privet_filesystem_backend.cc',
         'browser/local_discovery/storage/privet_filesystem_backend.h',
         'browser/local_discovery/storage/privet_filesystem_constants.cc',
         'browser/local_discovery/storage/privet_filesystem_constants.h',
+        'browser/local_discovery/storage/privet_filesystem_operations.cc',
+        'browser/local_discovery/storage/privet_filesystem_operations.h',
         'browser/local_discovery/storage/privet_volume_lister.cc',
         'browser/local_discovery/storage/privet_volume_lister.h',
         'browser/mac/dock.h',

[Noam Samuel, 2014-02-05 22:23:47]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-05 22:29:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/270001

[commit-bot: I haz the power, 2014-02-05 23:11:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-05 23:11:05]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[Noam Samuel, 2014-02-07 21:53:46]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-07 21:55:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/580001

[commit-bot: I haz the power, 2014-02-07 22:25:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/580001

[commit-bot: I haz the power, 2014-02-07 23:43:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-07 23:43:09]

Retried try job too often on android_clang_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[Noam Samuel, 2014-02-11 19:07:50]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-11 19:10:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/580001

[commit-bot: I haz the power, 2014-02-11 20:19:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-11 20:19:24]

Retried try job too often on android_clang_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[Noam Samuel, 2014-02-12 00:12:33]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-12 00:13:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/1190001

[commit-bot: I haz the power, 2014-02-12 01:08:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-12 01:08:21]

Retried try job too often on mac_rel for step(s) app_list_unittests,
cacheinvalidation_unittests, cc_unittests, check_deps, check_deps2git,
chromedriver_unittests, components_unittests, content_browsertests,
content_unittests, crypto_unittests, google_apis_unittests, gpu_unittests,
ipc_tests, jingle_unittests, media_unittests, message_center_unittests,
nacl_integration, ppapi_unittests, printing_unittests, remoting_unittests,
sql_unittests, sync_integration_tests, sync_unit_tests,
telemetry_perf_unittests, telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Noam Samuel, 2014-02-12 01:11:55]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-12 01:12:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/1190001

[commit-bot: I haz the power, 2014-02-12 01:40:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-12 01:40:26]

Retried try job too often on win for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[Noam Samuel, 2014-02-12 16:46:14]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-12 16:46:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/1190001

[commit-bot: I haz the power, 2014-02-12 18:23:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-12 18:23:51]

Retried try job too often on mac_rel for step(s) app_list_unittests,
base_unittests, browser_tests, cacheinvalidation_unittests, cc_unittests,
check_deps, check_deps2git, chromedriver_unittests, components_unittests,
content_browsertests, content_unittests, crypto_unittests,
google_apis_unittests, gpu_unittests, interactive_ui_tests, ipc_tests,
jingle_unittests, media_unittests, message_center_unittests, nacl_integration,
net_unittests, ppapi_unittests, printing_unittests, remoting_unittests,
sql_unittests, sync_integration_tests, sync_unit_tests,
telemetry_perf_unittests, telemetry_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Noam Samuel, 2014-02-13 00:15:05]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-13 00:19:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/1670001

[commit-bot: I haz the power, 2014-02-13 01:09:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-13 01:09:03]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Noam Samuel, 2014-02-18 20:03:07]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-18 20:03:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/1970001

[commit-bot: I haz the power, 2014-02-18 20:03:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-18 20:03:58]

Failed to apply patch for chrome/browser/chrome_content_browser_client.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/chrome_content_browser_client.cc
  Hunk #1 FAILED at 45.
  Hunk #2 succeeded at 242 (offset 1 line).
  Hunk #3 succeeded at 2579 (offset 4 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/chrome_content_browser_client.cc.rej

Patch:       chrome/browser/chrome_content_browser_client.cc
Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc
b/chrome/browser/chrome_content_browser_client.cc
index
bbb5a8bbbcdb3d2e29e3ded70a6fd1f278f86361..9c72777dd3a81de0110b6b22f8d3c24d867b82ec
100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -45,7 +45,6 @@
 #include "chrome/browser/guestview/guestview.h"
 #include "chrome/browser/guestview/guestview_constants.h"
 #include "chrome/browser/guestview/webview/webview_guest.h"
-#include "chrome/browser/local_discovery/storage/privet_filesystem_backend.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.h"
 #include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h"
@@ -241,6 +240,11 @@
 #include "chrome/browser/spellchecker/spellcheck_message_filter.h"
 #endif
 
+
+#if defined(ENABLE_MDNS)
+#include "chrome/browser/local_discovery/storage/privet_filesystem_backend.h"
+#endif
+
 using blink::WebWindowFeatures;
 using base::FileDescriptor;
 using content::AccessTokenStore;
@@ -2570,12 +2574,15 @@ void
ChromeContentBrowserClient::GetAdditionalFileSystemBackends(
       new sync_file_system::SyncFileSystemBackend(
           Profile::FromBrowserContext(browser_context)));
 
+#if defined(ENABLE_MDNS)
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnablePrivetStorage)) {
     additional_backends->push_back(
         new local_discovery::PrivetFileSystemBackend(
-            fileapi::ExternalMountPoints::GetSystemInstance()));
+            fileapi::ExternalMountPoints::GetSystemInstance(),
+            browser_context));
   }
+#endif
 }
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)

[Noam Samuel, 2014-02-18 23:39:17]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-02-18 23:39:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/141703022/2030001

[commit-bot: I haz the power, 2014-02-19 02:36:02]

Change committed as 251936