Save <input type=file> from premature garbage collection

If an <input type=file> is detached from the DOM and programmatically
clicked using .click(), then sometimes it was garbage collected before
the user finished choosing a file, which would cause the 'change' event
handler on the input element to not get called.

Bug: 996269
Change-Id: I839bb76058b1e8f619732003ba6d82ed9ab0f66d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875454Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713603}

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

@@ -160,6 +160,8 @@ void FileChooser::DidCloseChooser() {
       chrome_client_impl_->UnregisterPopupOpeningObserver(client_);
   }
 
+  if (client_)
+    client_->DisconnectFileChooser();
   Release();
 }
 

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

@@ -60,13 +60,14 @@ class CORE_EXPORT FileChooserClient : public PopupOpeningObserver {
   // DisconnectFileChooser().
   FileChooser* FileChooserOrNull() const { return chooser_.get(); }
 
- protected:
-  FileChooser* NewFileChooser(const mojom::blink::FileChooserParams&);
-  bool HasConnectedFileChooser() const { return chooser_.get(); }
-
   // This should be called if a user chose files or cancel the dialog.
   void DisconnectFileChooser();
 
+  FileChooser* NewFileChooser(const mojom::blink::FileChooserParams&);
+
+ protected:
+  bool HasConnectedFileChooser() const { return chooser_.get(); }
+
  private:
   scoped_refptr<FileChooser> chooser_;
 };
@@ -96,7 +97,7 @@ class FileChooser : public RefCounted<FileChooser> {
   // mojom::blink::FileChooser callback
   void DidChooseFiles(mojom::blink::FileChooserResultPtr result);
 
-  WeakPersistent<FileChooserClient> client_;
+  Persistent<FileChooserClient> client_;
   mojom::blink::FileChooserParamsPtr params_;
   Persistent<ChromeClientImpl> chrome_client_impl_;
   mojo::Remote<mojom::blink::FileChooser> file_chooser_;

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

@@ -482,6 +482,8 @@ class CORE_EXPORT ChromeClient : public GarbageCollected<ChromeClient> {
   virtual void DidUpdateTextAutosizerPageInfo(const WebTextAutosizerPageInfo&) {
   }
 
+  virtual void DocumentDetached(Document&) {}
+
  protected:
   ChromeClient() = default;
 

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

@@ -1280,4 +1280,11 @@ void ChromeClientImpl::DidUpdateTextAutosizerPageInfo(
   web_view_->Client()->DidUpdateTextAutosizerPageInfo(page_info);
 }
 
+void ChromeClientImpl::DocumentDetached(Document& document) {
+  for (auto& it : file_chooser_queue_) {
+    if (it->FrameOrNull() == document.GetFrame())
+      it->DisconnectClient();
+  }
+}
+
 }  // namespace blink

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

@@ -268,6 +268,8 @@ class CORE_EXPORT ChromeClientImpl final : public ChromeClient {
 
   void DidUpdateTextAutosizerPageInfo(const WebTextAutosizerPageInfo&) override;
 
+  void DocumentDetached(Document&) override;
+
  private:
   bool IsChromeClientImpl() const override { return true; }
 

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

@@ -274,8 +274,8 @@ TEST_F(FileChooserQueueTest, DerefQueuedChooser) {
   auto* client2 = MakeGarbageCollected<MockFileChooserClient>(frame);
   mojom::blink::FileChooserParams params;
   params.title = g_empty_string;
-  scoped_refptr<FileChooser> chooser1 = FileChooser::Create(client1, params);
-  scoped_refptr<FileChooser> chooser2 = FileChooser::Create(client2, params);
+  scoped_refptr<FileChooser> chooser1 = client1->NewFileChooser(params);
+  scoped_refptr<FileChooser> chooser2 = client2->NewFileChooser(params);
 
   chrome_client_impl_->OpenFileChooser(frame, chooser1);
   chrome_client_impl_->OpenFileChooser(frame, chooser2);

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

@@ -318,6 +318,8 @@ void Page::DocumentDetached(Document* document) {
 
   if (agent_metrics_collector_)
     agent_metrics_collector_->DidDetachDocument(*document);
+
+  GetChromeClient().DocumentDetached(*document);
 }
 
 bool Page::OpenedByDOM() const {