<input type=file>: Fix a crash in ChromeClientImpl::DidCompleteFileChooser().

This is a regression by http://crrev.com/579748.

If we request opening multiple file choosers, and one of the
corresponding blink::FileChooser object is deallocated before calling
its OpenFileChooser(), ChromeClientImpl had a stale
pointer. ChromeClientImpl should have Vector<scoped_refptr<FileChooser>>
instead of Vector<FileChooser*>.

Bug: 877269
Change-Id: I296e6ac83b858e1b74fecff87e7a2fa7f9b96765
Reviewed-on: https://chromium-review.googlesource.com/1188009Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585753}

third_party/blink/renderer/core/html/forms/file_chooser.h

@@ -64,7 +64,7 @@ struct FileChooserFileInfo {
   const FileMetadata metadata;
 };
 
-class FileChooserClient : public PopupOpeningObserver {
+class CORE_EXPORT FileChooserClient : public PopupOpeningObserver {
  public:
   virtual void FilesChosen(const Vector<FileChooserFileInfo>&) = 0;
   virtual LocalFrame* FrameOrNull() const = 0;
@@ -84,8 +84,9 @@ class FileChooserClient : public PopupOpeningObserver {
 class FileChooser : public RefCounted<FileChooser>,
                     public WebFileChooserCompletion {
  public:
-  static scoped_refptr<FileChooser> Create(FileChooserClient*,
-                                           const WebFileChooserParams&);
+  CORE_EXPORT static scoped_refptr<FileChooser> Create(
+      FileChooserClient*,
+      const WebFileChooserParams&);
   ~FileChooser() override;
 
   LocalFrame* FrameOrNull() const {

third_party/blink/renderer/core/page/chrome_client_impl.cc

@@ -612,7 +612,7 @@ void ChromeClientImpl::OpenFileChooser(
 
 void ChromeClientImpl::DidCompleteFileChooser(FileChooser& chooser) {
   if (!file_chooser_queue_.IsEmpty() &&
-      file_chooser_queue_.front() != &chooser) {
+      file_chooser_queue_.front().get() != &chooser) {
     // This function is called even if |chooser| wasn't stored in
     // file_chooser_queue_.
     return;
@@ -620,7 +620,7 @@ void ChromeClientImpl::DidCompleteFileChooser(FileChooser& chooser) {
   file_chooser_queue_.EraseAt(0);
   if (file_chooser_queue_.IsEmpty())
     return;
-  FileChooser* next_chooser = file_chooser_queue_.front();
+  FileChooser* next_chooser = file_chooser_queue_.front().get();
   if (next_chooser->OpenFileChooser(*this))
     return;
   // Choosing failed, so try the next chooser.

third_party/blink/renderer/core/page/chrome_client_impl.h

@@ -248,10 +248,12 @@ class CORE_EXPORT ChromeClientImpl final : public ChromeClient {
 
   WebViewImpl* web_view_;  // Weak pointer.
   HeapHashSet<WeakMember<PopupOpeningObserver>> popup_opening_observers_;
-  Vector<FileChooser*> file_chooser_queue_;
+  Vector<scoped_refptr<FileChooser>> file_chooser_queue_;
   Cursor last_set_mouse_cursor_for_testing_;
   bool cursor_overridden_;
   bool did_request_non_empty_tool_tip_;
+
+  FRIEND_TEST_ALL_PREFIXES(FileChooserQueueTest, DerefQueuedChooser);
 };
 
 DEFINE_TYPE_CASTS(ChromeClientImpl,

third_party/blink/renderer/core/page/chrome_client_impl_test.cc

@@ -42,6 +42,7 @@
 #include "third_party/blink/renderer/core/html/forms/color_chooser_client.h"
 #include "third_party/blink/renderer/core/html/forms/date_time_chooser.h"
 #include "third_party/blink/renderer/core/html/forms/date_time_chooser_client.h"
+#include "third_party/blink/renderer/core/html/forms/file_chooser.h"
 #include "third_party/blink/renderer/core/html/forms/html_select_element.h"
 #include "third_party/blink/renderer/core/loader/frame_load_request.h"
 #include "third_party/blink/renderer/core/page/page.h"
@@ -239,4 +240,64 @@ TEST_F(PagePopupSuppressionTest, SuppressDateTimeChooser) {
   EXPECT_TRUE(CanOpenDateTimeChooser());
 }
 
+// A WebLocalFrameClient which makes FileChooser::OpenFileChooser() success.
+class FrameClientForFileChooser : public FrameTestHelpers::TestWebFrameClient {
+  bool RunFileChooser(const WebFileChooserParams& params,
+                      WebFileChooserCompletion* chooser_completion) override {
+    return true;
+  }
+};
+
+// A FileChooserClient which makes FileChooser::OpenFileChooser() success.
+class MockFileChooserClient
+    : public GarbageCollectedFinalized<MockFileChooserClient>,
+      public FileChooserClient {
+  USING_GARBAGE_COLLECTED_MIXIN(MockFileChooserClient);
+
+ public:
+  explicit MockFileChooserClient(LocalFrame* frame) : frame_(frame) {}
+  void Trace(Visitor* visitor) override {
+    visitor->Trace(frame_);
+    FileChooserClient::Trace(visitor);
+  }
+
+ private:
+  // FilesChosen() and WillOpenPopup() are never called in the test.
+  void FilesChosen(const Vector<FileChooserFileInfo>&) override {}
+  void WillOpenPopup() override {}
+
+  LocalFrame* FrameOrNull() const override { return frame_; }
+
+  Member<LocalFrame> frame_;
+};
+
+class FileChooserQueueTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    web_view_ = helper_.Initialize(&frame_client_);
+    chrome_client_impl_ =
+        ToChromeClientImpl(&web_view_->GetPage()->GetChromeClient());
+  }
+
+  FrameTestHelpers::WebViewHelper helper_;
+  FrameClientForFileChooser frame_client_;
+  WebViewImpl* web_view_;
+  Persistent<ChromeClientImpl> chrome_client_impl_;
+};
+
+TEST_F(FileChooserQueueTest, DerefQueuedChooser) {
+  LocalFrame* frame = helper_.LocalMainFrame()->GetFrame();
+  auto* client = new MockFileChooserClient(frame);
+  WebFileChooserParams params;
+  scoped_refptr<FileChooser> chooser1 = FileChooser::Create(client, params);
+  scoped_refptr<FileChooser> chooser2 = FileChooser::Create(client, params);
+
+  chrome_client_impl_->OpenFileChooser(frame, chooser1);
+  chrome_client_impl_->OpenFileChooser(frame, chooser2);
+  EXPECT_EQ(2u, chrome_client_impl_->file_chooser_queue_.size());
+  chooser2.reset();
+  chrome_client_impl_->DidCompleteFileChooser(*chooser1);
+  EXPECT_EQ(1u, chrome_client_impl_->file_chooser_queue_.size());
+}
+
 }  // namespace blink