New issue
Advanced search Search tips

Issue 684233 link

Starred by 1 user
Status: Verified
Owner:
nya@chromium.org
Closed: Mar 2017
Cc:
weifangsun@chromium.org
fukino@chromium.org
nya@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 692586
Blocking:
issue 691121



Sign in to add a comment

Files App: Watch for media view updates

Project Member Reported by weifangsun@chromium.org, Jan 24 2017 
Currently, the Media View folders (for ARC++) need explicit user action in order to refresh the files/folders. We should watch for these updates and reflect the changes in the UI with needing the explicit refresh.

Comment 1 by weifangsun@chromium.org, Feb 11 2017

Blocking: 691121

Comment 2 by weifangsun@chromium.org, Feb 11 2017

Cc: fukino@chromium.org

Comment 3 by nya@chromium.org, Feb 16 2017

Blockedon: 692586
Cc: weifangsun@chromium.org
Owner: nya@chromium.org

Comment 4 by nya@chromium.org, Feb 23 2017

Labels: OS-Chrome Pri-2 Type-Bug

Comment 5 by nya@chromium.org, Feb 23 2017

Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/533aed06060aba3689e57e04bb352f14cfc7ca25

commit 533aed06060aba3689e57e04bb352f14cfc7ca25
Author: nya <nya@chromium.org>
Date: Fri Feb 24 06:37:58 2017

mediaview: IPC definitions to watch for document changes.

New IPC methods will allow Chrome to watch for changes in Android
documents provider.

This patch contains just mojom changes and minimal changes to
FakeFileSystemInstance. Implementation of the fake is coming in the
next patch.

BUG= chromium:684233 
TEST=trybot

Review-Url: https://codereview.chromium.org/2714433003
Cr-Commit-Position: refs/heads/master@{#452775}

[modify] https://crrev.com/533aed06060aba3689e57e04bb352f14cfc7ca25/components/arc/common/file_system.mojom
[modify] https://crrev.com/533aed06060aba3689e57e04bb352f14cfc7ca25/components/arc/test/fake_file_system_instance.cc
[modify] https://crrev.com/533aed06060aba3689e57e04bb352f14cfc7ca25/components/arc/test/fake_file_system_instance.h
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 24 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3130418a5b9bb3e1bb389976617e3bd112ec37aa

commit 3130418a5b9bb3e1bb389976617e3bd112ec37aa
Author: nya <nya@chromium.org>
Date: Fri Feb 24 06:46:58 2017

mediaview: Implement FakeFileSystemInstance.

This patch adds fake implementations of new FileSystemInstance methods
introduced in https://codereview.chromium.org/2714433003.

BUG= chromium:684233 
TEST=trybot

Review-Url: https://codereview.chromium.org/2709613006
Cr-Commit-Position: refs/heads/master@{#452776}

[modify] https://crrev.com/3130418a5b9bb3e1bb389976617e3bd112ec37aa/components/arc/test/fake_file_system_instance.cc
[modify] https://crrev.com/3130418a5b9bb3e1bb389976617e3bd112ec37aa/components/arc/test/fake_file_system_instance.h
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 2 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6cbbd4f60f2285cf33ed2a47658ab3e3e7380eaa

commit 6cbbd4f60f2285cf33ed2a47658ab3e3e7380eaa
Author: nya <nya@chromium.org>
Date: Thu Mar 02 16:49:26 2017

mediaview: Change the contract of watcher callbacks.

In initial contract, watchers could be automatically uninstalled without
explicit RemoveWatcher() calls if watched directories are deleted.
However it does not align with the contract of storage::WatcherManager
(the primary user of watchers) which leaves watchers even after watched
directories are removed. Using different contracts may make it slightly
difficult to understand, so this patch will update the Mojo IPC contract
to align with WatcherManager's.

Usually we should not update the contract of existing Mojo IPC methods,
but it is okay in this case because we have not implemented those
methods in both Chrome and Android yet. One exception is
https://codereview.chromium.org/2709613006/ which implemented a fake of
FileSystemInstance, but it (unexpectedly) follows the new contract, so
we do not need to make changes to it.

BUG= chromium:684233 
TEST=trybot

Review-Url: https://codereview.chromium.org/2718303002
Cr-Commit-Position: refs/heads/master@{#454285}

[modify] https://crrev.com/6cbbd4f60f2285cf33ed2a47658ab3e3e7380eaa/components/arc/common/file_system.mojom

Comment 12 by nya@chromium.org, Mar 6 2017

Labels: Merge-Request-58
Status: Fixed (was: Started)
Watcher implemented. Missed the branch cut a bit, so I'm requesting cherry-pick.
Project Member

Comment 13 by sheriffbot@chromium.org, Mar 6 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), bhthompson@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 6 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38e21206c01bdbb584ba2deb8dc23d5774755ccc

commit 38e21206c01bdbb584ba2deb8dc23d5774755ccc
Author: Shuhei Takahashi <nya@chromium.org>
Date: Mon Mar 06 03:30:44 2017

mediaview: Support watchers in ArcFileSystemOperationRunner.

AddWatcher() and RemoveWatcher() are added.
Also, ArcFileSystemOperationRunner implements FileSystemHost to
receive watcher callbacks from the Android container.

BUG= chromium:684233 
TEST=trybot

Review-Url: https://codereview.chromium.org/2715493002
Cr-Commit-Position: refs/heads/master@{#454589}
(cherry picked from commit 1cf2cd5fd31329f2cb7de2cd2df919648b5e331b)

Review-Url: https://codereview.chromium.org/2736603002 .
Cr-Commit-Position: refs/branch-heads/3029@{#12}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/38e21206c01bdbb584ba2deb8dc23d5774755ccc/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.cc
[modify] https://crrev.com/38e21206c01bdbb584ba2deb8dc23d5774755ccc/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h
[modify] https://crrev.com/38e21206c01bdbb584ba2deb8dc23d5774755ccc/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_unittest.cc
[modify] https://crrev.com/38e21206c01bdbb584ba2deb8dc23d5774755ccc/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_util.cc
[modify] https://crrev.com/38e21206c01bdbb584ba2deb8dc23d5774755ccc/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_util.h
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c700fa7ae3641919406e695b2a393d7691ca2086

commit c700fa7ae3641919406e695b2a393d7691ca2086
Author: Shuhei Takahashi <nya@chromium.org>
Date: Mon Mar 06 03:32:59 2017

mediaview: Support watchers in ArcDocumentsProviderRoot.

Watchers are not always correct by design. See the long comments at
AddWatcher().

BUG= chromium:684233 
TEST=trybot

Review-Url: https://codereview.chromium.org/2715473003
Cr-Commit-Position: refs/heads/master@{#454590}
(cherry picked from commit 7049abb88b1e21acdd6345400183cd1df7762a84)

Review-Url: https://codereview.chromium.org/2731883002 .
Cr-Commit-Position: refs/branch-heads/3029@{#13}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/c700fa7ae3641919406e695b2a393d7691ca2086/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.cc
[modify] https://crrev.com/c700fa7ae3641919406e695b2a393d7691ca2086/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h
[modify] https://crrev.com/c700fa7ae3641919406e695b2a393d7691ca2086/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root_unittest.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Mar 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16f238bf78097782309f57a55685b50c8d806495

commit 16f238bf78097782309f57a55685b50c8d806495
Author: Shuhei Takahashi <nya@chromium.org>
Date: Mon Mar 06 03:34:44 2017

mediaview: Implement ArcDocumentsProviderWatcherManager.

ArcDocumentsProviderWatcherManager simply calls ArcDocumentsProviderRoot.

BUG= chromium:684233 
TEST=trybot
TEST=Media view refreshes on media provider changes.

Review-Url: https://codereview.chromium.org/2709013003
Cr-Commit-Position: refs/heads/master@{#454598}
(cherry picked from commit 7b44260f27f6b52b3a1b81e81cdedeb8164b4497)

Review-Url: https://codereview.chromium.org/2736613002 .
Cr-Commit-Position: refs/branch-heads/3029@{#14}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/16f238bf78097782309f57a55685b50c8d806495/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/16f238bf78097782309f57a55685b50c8d806495/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_backend_delegate.cc
[modify] https://crrev.com/16f238bf78097782309f57a55685b50c8d806495/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_backend_delegate.h
[add] https://crrev.com/16f238bf78097782309f57a55685b50c8d806495/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_watcher_manager.cc
[add] https://crrev.com/16f238bf78097782309f57a55685b50c8d806495/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_watcher_manager.h
[modify] https://crrev.com/16f238bf78097782309f57a55685b50c8d806495/chrome/browser/chromeos/fileapi/file_system_backend.cc

Comment 17 by abod...@chromium.org, Apr 5 2017

Status: Verified (was: Fixed)
►

Sign in to add a comment