[fsp] Show request logs in chrome://provided-file-systems.

[fsp] Show request logs in chrome://provided-file-systems.

This CL adds displaying live log from the request manager of the selected
file system.

TEST=Tested manually with a testing extension.
BUG=376095
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274436

[mtomasz, 2014-05-29 05:53:40]

@nkostylev: PTAL at */webui/*.
@jochen: PTAL at */resources/*.
@kinaba: PTAL at */file_system_provider/*.

Thanks.

[kinaba, 2014-05-29 07:51:14]

On 2014/05/29 05:53:40, mtomasz wrote:
> @nkostylev: PTAL at */webui/*.
> @jochen: PTAL at */resources/*.
> @kinaba: PTAL at */file_system_provider/*.
> 
> Thanks.

*/file_system_provider/* lgtm

[Nikita (slow), 2014-05-30 07:15:10]

lgtm with nits.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc (right):

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:88:
request_manager->RemoveObserver(this);
nit: Just file_system->GetRequestManager()->RemoveObserver(this);

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:96:
event.SetInteger("id", request_id);
nit: Please extract all IDs as constants, they're used in several methods.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:106:
event.SetInteger("id", request_id);
nit: You may introduce helper function that will fill in common fields like
id/eventType/time. And may as well call onRequestEvent JS function.

Prior to calling that function you may fill in additional data such as
hasMore/error etc.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:230:
file_system_provider::RequestManager* const request_manager =
nit: file_system->GetRequestManager()->RemoveObserver(this);

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:243:
file_system_provider::RequestManager* const request_manager =
nit: same here.

[mtomasz, 2014-06-02 03:16:24]

@jochen: Ping. Thanks.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc (right):

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:88:
request_manager->RemoveObserver(this);
On 2014/05/30 07:15:10, Nikita Kostylev wrote:
> nit: Just file_system->GetRequestManager()->RemoveObserver(this);

This doesn't happen now, but theoretically ProvidedFileSystemInterface allows to
return NULL here, so I'd prefer to have at least a DCHECK here.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:96:
event.SetInteger("id", request_id);
On 2014/05/30 07:15:10, Nikita Kostylev wrote:
> nit: Please extract all IDs as constants, they're used in several methods.

Done.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:106:
event.SetInteger("id", request_id);
On 2014/05/30 07:15:10, Nikita Kostylev wrote:
> nit: You may introduce helper function that will fill in common fields like
> id/eventType/time. And may as well call onRequestEvent JS function.
> 
> Prior to calling that function you may fill in additional data such as
> hasMore/error etc.

Done.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:230:
file_system_provider::RequestManager* const request_manager =
On 2014/05/30 07:15:10, Nikita Kostylev wrote:
> nit: file_system->GetRequestManager()->RemoveObserver(this);

As above.

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:243:
file_system_provider::RequestManager* const request_manager =
On 2014/05/30 07:15:10, Nikita Kostylev wrote:
> nit: same here.

As above.

[Nikita (slow), 2014-06-02 06:15:12]

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc (right):

https://codereview.chromium.org/301873002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/provided_file_systems_ui.cc:88:
request_manager->RemoveObserver(this);
On 2014/06/02 03:16:24, mtomasz wrote:
> On 2014/05/30 07:15:10, Nikita Kostylev wrote:
> > nit: Just file_system->GetRequestManager()->RemoveObserver(this);
> 
> This doesn't happen now, but theoretically ProvidedFileSystemInterface allows
to
> return NULL here, so I'd prefer to have at least a DCHECK here.

Either way it will crash if you try to access it but using DCHECK is fine too.

[jochen (gone - plz use gerrit), 2014-06-02 06:58:59]

can you please pick on of the c/b/resources OWNERS? I'm not that familiar with
all the conventions they want to enforce for html/css/js

[Nikita (slow), 2014-06-02 07:30:33]

On 2014/06/02 06:58:59, jochen wrote:
> can you please pick on of the c/b/resources OWNERS? I'm not that familiar with
> all the conventions they want to enforce for html/css/js

My lgtm covers that. I've looked at resources.

[mtomasz, 2014-06-02 07:43:54]

On 2014/06/02 07:30:33, Nikita Kostylev wrote:
> On 2014/06/02 06:58:59, jochen wrote:
> > can you please pick on of the c/b/resources OWNERS? I'm not that familiar
with
> > all the conventions they want to enforce for html/css/js
> 
> My lgtm covers that. I've looked at resources.

Thanks.

[mtomasz, 2014-06-02 07:43:57]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-06-02 07:44:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mtomasz@chromium.org/301873002/60001

[commit-bot: I haz the power, 2014-06-02 11:43:34]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/11636)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/14730)

[commit-bot: I haz the power, 2014-06-02 11:46:29]

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

[commit-bot: I haz the power, 2014-06-02 11:46:29]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[mtomasz, 2014-06-02 12:42:36]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-06-02 12:43:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mtomasz@chromium.org/301873002/60001

[commit-bot: I haz the power, 2014-06-02 13:18:39]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/11667)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/14760)

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

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

[commit-bot: I haz the power, 2014-06-02 13:21:02]

Try jobs failed on following builders:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[mtomasz, 2014-06-03 00:25:02]

The CQ bit was checked by mtomasz@chromium.org

[commit-bot: I haz the power, 2014-06-03 00:25:44]

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

[commit-bot: I haz the power, 2014-06-03 08:54:38]

Change committed as 274436