Storage manager: Show overall size information and size of Downloads.

Storage manager: Show overall size information and size of Downloads.

On the storage manager overlay, this CL adds following functions.
- Shows overall storage information(total/used/available) using statvfs() on the local volume.
- Shows size of Downloads directory by computing directory size recursively.
- Opens up Files app when the label "Downloads" is clicked.
The layout inside the overlay is tentative.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
TEST=manually by flag #enable-storage-manager enabled.

Committed: https://crrev.com/205d6d81b48dc72872c07c0f56bdd730129e60a1
Cr-Commit-Position: refs/heads/master@{#399057}

[fukino, 2016-06-07 14:02:32]

Description was changed from

==========
Storage manager: Show overall size information and size of Downloads.

BUG=591958
==========

to

==========
Storage manager: Show overall size information and size of Downloads.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[fukino, 2016-06-07 14:09:58]

Description was changed from

==========
Storage manager: Show overall size information and size of Downloads.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Storage manager: Show overall size information and size of Downloads.

On the storage manager overlay, this CL adds following functions.
- Shows overall storage information(total/used/available) using statvfs() on the
local volume.
- Shows size of Downloads directory by computing directory size recursively.
- Opens up Files app when the label "Downloads" is clicked.
The layout inside the overlay is tentative.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
TEST=manually by flag #enable-storage-manager enabled.
==========

[fukino, 2016-06-07 14:12:02]

fukino@chromium.org changed reviewers:
+ dbeam@chromium.org

[fukino, 2016-06-07 14:12:02]

dbeam@, could you take a look?

[Dan Beam, 2016-06-08 02:07:13]

didn't get to this today (yet), but probably will look at tomorrow

sorry, at a summit where we're not supposed to use computers/phones

[fukino, 2016-06-08 02:17:04]

On 2016/06/08 02:07:13, Dan Beam wrote:
> didn't get to this today (yet), but probably will look at tomorrow
> 
> sorry, at a summit where we're not supposed to use computers/phones

No problem at all! Thank you for informing me.

[Dan Beam, 2016-06-08 18:35:35]

you know we may be launching the new, Material Design settings in M53-54, right?

[Dan Beam, 2016-06-09 04:26:03]

On 2016/06/08 18:35:35, Dan Beam wrote:
> you know we may be launching the new, Material Design settings in M53-54,
right?

sorry, ignore me: this is a cros-only thing and the release timeline is less
known for cros

[Dan Beam, 2016-06-09 04:49:40]

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos/storage_manager.css (right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos/storage_manager.css:13: font-size:
13px;
try to use a relative font size (i.e. em, %, rem)

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos/storage_manager.js (right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos/storage_manager.js:57:
$('storage-manager-size-capacity').innerHTML = sizeStat.totalSize;
can you use textContent instead?

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc
(right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc:28: if
(HANDLE_EINTR(statvfs(mount_path.c_str(), &stat)) == 0) {
can you share code with base::SysInfo::AmountOfFreeDiskSpace?

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc:103:
int64_t* total_size = new int64_t(0);
why are you using pointers for this?

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h (right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h:35: void
OnGetDownloadsSize(int64_t size);
can you document these?

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h:37:
base::WeakPtrFactory<StorageManagerHandler> weak_ptr_factory_;
IWYU: #include "base/memory/weak_ptr.h"

[fukino, 2016-06-09 05:00:13]

Thanks! I have two questions.

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc
(right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc:28: if
(HANDLE_EINTR(statvfs(mount_path.c_str(), &stat)) == 0) {
On 2016/06/09 04:49:40, Dan Beam wrote:
> can you share code with base::SysInfo::AmountOfFreeDiskSpace?

Should I add a function to know the total size of file system to base::SysInfo?

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc:103:
int64_t* total_size = new int64_t(0);
On 2016/06/09 04:49:40, Dan Beam wrote:
> why are you using pointers for this?

I wanted to pass two values (total_size and available_size) from
GetSizeStatOnBlockingPool to StorageManagerHandler::OnGetSizeStat. Is there a
better way to do it?

[fukino, 2016-06-09 15:39:38]

Thank you! Please take another look.

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos/storage_manager.css (right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos/storage_manager.css:13: font-size:
13px;
On 2016/06/09 04:49:40, Dan Beam wrote:
> try to use a relative font size (i.e. em, %, rem)

Done.

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos/storage_manager.js (right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos/storage_manager.js:57:
$('storage-manager-size-capacity').innerHTML = sizeStat.totalSize;
On 2016/06/09 04:49:40, Dan Beam wrote:
> can you use textContent instead?

Done.

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc
(right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.cc:28: if
(HANDLE_EINTR(statvfs(mount_path.c_str(), &stat)) == 0) {
On 2016/06/09 05:00:13, fukino wrote:
> On 2016/06/09 04:49:40, Dan Beam wrote:
> > can you share code with base::SysInfo::AmountOfFreeDiskSpace?
> 
> Should I add a function to know the total size of file system to
base::SysInfo?

I'm adding the same function to base::SysInfo.
https://codereview.chromium.org/2052663003/

Afther the CL lands, I'll update how to get disk space information.

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h (right):

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h:35: void
OnGetDownloadsSize(int64_t size);
On 2016/06/09 04:49:40, Dan Beam wrote:
> can you document these?

Done.

https://codereview.chromium.org/2041343002/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/storage_manager_handler.h:37:
base::WeakPtrFactory<StorageManagerHandler> weak_ptr_factory_;
On 2016/06/09 04:49:40, Dan Beam wrote:
> IWYU: #include "base/memory/weak_ptr.h"

Done.

[Dan Beam, 2016-06-09 17:24:30]

lgtm but i'd rather just wait for sysinfo stuff to land first, leaving it up to
you

[fukino, 2016-06-10 00:22:16]

On 2016/06/09 17:24:30, Dan Beam wrote:
> lgtm but i'd rather just wait for sysinfo stuff to land first, leaving it up
to
> you

To make following works which depends on this CL easier, let me land this
independently.
I'll update storage manager and file manager (which also call statvfs) as soon
as the sysinfo staff lands.

[fukino, 2016-06-10 00:22:33]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2016-06-10 00:23:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2041343002/20001

[commit-bot: I haz the power, 2016-06-10 00:42:25]

Description was changed from

==========
Storage manager: Show overall size information and size of Downloads.

On the storage manager overlay, this CL adds following functions.
- Shows overall storage information(total/used/available) using statvfs() on the
local volume.
- Shows size of Downloads directory by computing directory size recursively.
- Opens up Files app when the label "Downloads" is clicked.
The layout inside the overlay is tentative.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
TEST=manually by flag #enable-storage-manager enabled.
==========

to

==========
Storage manager: Show overall size information and size of Downloads.

On the storage manager overlay, this CL adds following functions.
- Shows overall storage information(total/used/available) using statvfs() on the
local volume.
- Shows size of Downloads directory by computing directory size recursively.
- Opens up Files app when the label "Downloads" is clicked.
The layout inside the overlay is tentative.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
TEST=manually by flag #enable-storage-manager enabled.
==========

[commit-bot: I haz the power, 2016-06-10 00:42:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-10 00:42:30]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-10 00:44:16]

Description was changed from

==========
Storage manager: Show overall size information and size of Downloads.

On the storage manager overlay, this CL adds following functions.
- Shows overall storage information(total/used/available) using statvfs() on the
local volume.
- Shows size of Downloads directory by computing directory size recursively.
- Opens up Files app when the label "Downloads" is clicked.
The layout inside the overlay is tentative.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
TEST=manually by flag #enable-storage-manager enabled.
==========

to

==========
Storage manager: Show overall size information and size of Downloads.

On the storage manager overlay, this CL adds following functions.
- Shows overall storage information(total/used/available) using statvfs() on the
local volume.
- Shows size of Downloads directory by computing directory size recursively.
- Opens up Files app when the label "Downloads" is clicked.
The layout inside the overlay is tentative.

BUG=591958
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
TEST=manually by flag #enable-storage-manager enabled.

Committed: https://crrev.com/205d6d81b48dc72872c07c0f56bdd730129e60a1
Cr-Commit-Position: refs/heads/master@{#399057}
==========

[commit-bot: I haz the power, 2016-06-10 00:44:17]

Patchset 2 (id:??) landed as
https://crrev.com/205d6d81b48dc72872c07c0f56bdd730129e60a1
Cr-Commit-Position: refs/heads/master@{#399057}