Exclude files from FileSelectChooser if they can't convert to WebStrings.

chrome/browser/file_select_helper.cc

 // Copyright (c) 2012 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 "chrome/browser/file_select_helper.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
@@ skipping 302 matching lines @@
         file_system_context, site_instance->GetSiteURL(), files,
         base::Bind(
             &FileSelectHelper::NotifyRenderFrameHostAndEndAfterConversion,
             this));
     return;
   }
 #endif  // defined(OS_CHROMEOS)
 
   std::vector<content::FileChooserFileInfo> chooser_files;
   for (const auto& file : files) {
+    // Exclude files whose paths can't be converted into WebStrings, since Blink
+    // won't be able to handle those.
+    if (file.local_path.AsUTF8Unsafe().empty())

[asanka, 2017/04/13 17:21:22]

Shall we move this logic to a static function and call it from
NotifyRenderFrameHostAndEndAfterConversion()?

This way:
* That function can be unit tested separately.
* Filtering logic will be applied to ChromeOS's file chooser results as well.
Technically those filenames should always be valid UTF-8. But we should apply
filtering consistently.

For an end-to-end test, we could rely on something like what we did in
ppapi_filechooser_browsertest.cc where we switch out the SelectFileDialogFactory
and replace it with something that'll act like the user selected some hardcoded
filename.

It'll be trivial to add  a test to ppapi_filechooser_browsertest to test the
logic being added here, but that'd violate the principle of least surprise for
what's tested where :-)

There *has* to be an end-to-end test where we exercise FileSelectHelper based on
a renderer initiated "open" request other than PPAPI. But I couldn't find one.

[Charlie Reis, 2017/04/14 19:28:15]

Thanks for the suggestion!  If we're going to be filtering the vector of files
after we build it, though, I might as well do it in RenderFrameHostImpl.  That
covers even more cases (including content_browsertests that mock out
FileSelectHelper), and it keeps both the fix and test closer to the code that
does the renderer kill.

I've updated the CL to do that-- can you take another look?

+      continue;
+
     content::FileChooserFileInfo chooser_file;
     chooser_file.file_path = file.local_path;
     chooser_file.display_name = file.display_name;
     chooser_files.push_back(chooser_file);
   }
 
   NotifyRenderFrameHostAndEndAfterConversion(chooser_files);
 }
 
 void FileSelectHelper::NotifyRenderFrameHostAndEndAfterConversion(
@@ skipping 360 matching lines @@
 
 // static
 base::FilePath FileSelectHelper::GetSanitizedFileName(
     const base::FilePath& suggested_filename) {
   if (suggested_filename.empty())
     return base::FilePath();
   return net::GenerateFileName(
       GURL(), std::string(), std::string(), suggested_filename.AsUTF8Unsafe(),
       std::string(), l10n_util::GetStringUTF8(IDS_DEFAULT_DOWNLOAD_FILENAME));
 }