SpeculativeCrashFix: Revert 2 CLs after which we started getting UAF reports

Reverts:
* crrev.com/c/1597540: Which seems to have introduced the UAF.
  See crbug.com/973928#c21
* crrev.com/c/1731698: Since that speculative CL did not fix the issue.

Since I'm out of ideas about what's causing the crash, this CL will
help establish that crrev.com/c/1597540 did indeed cause the issue.

R=dvadym

Bug: 948767,973928
Change-Id: I08e2023a607153c248676ef2c6203674dc2cabdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1736113
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Auto-Submit: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684325}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -732,22 +732,12 @@ void ChromePasswordManagerClient::DidStartNavigation(
 
 #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
 void ChromePasswordManagerClient::OnPaste() {
-  // TODO(vakh): This method should just call |GetTextFromClipboard()| directly
-  // but it seems to be causing crbug.com/973928 so for now, call the clipboard
-  // API directly to see if that fixes the issue.
-  // See https://crbug.com/973928#c21 for details.
+  ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread();
   base::string16 text;
-  ui::Clipboard::GetForCurrentThread()->ReadText(ui::ClipboardType::kCopyPaste,
-                                                 &text);
+  clipboard->ReadText(ui::ClipboardType::kCopyPaste, &text);
+  was_on_paste_called_ = true;
   password_reuse_detection_manager_.OnPaste(std::move(text));
 }
-
-base::string16 ChromePasswordManagerClient::GetTextFromClipboard() {
-  base::string16 text;
-  ui::Clipboard::GetForCurrentThread()->ReadText(ui::ClipboardType::kCopyPaste,
-                                                 &text);
-  return text;
-}
 #endif
 
 gfx::RectF ChromePasswordManagerClient::GetBoundsInScreenSpace(

chrome/browser/password_manager/chrome_password_manager_client.h

@@ -201,6 +201,7 @@ class ChromePasswordManagerClient
   bool has_binding_for_credential_manager() const {
     return content_credential_manager_.HasBinding();
   }
+  bool was_on_paste_called() const { return was_on_paste_called_; }
 #endif
 #if defined(OS_ANDROID)
   PasswordAccessoryController* GetOrCreatePasswordAccessory();
@@ -215,12 +216,6 @@ class ChromePasswordManagerClient
   ChromePasswordManagerClient(content::WebContents* web_contents,
                               autofill::AutofillClient* autofill_client);
 
-  // content::WebContentsObserver override
-#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
-  void OnPaste() override;
-  base::string16 GetTextFromClipboard();
-#endif
-
  private:
   friend class content::WebContentsUserData<ChromePasswordManagerClient>;
 
@@ -229,6 +224,9 @@ class ChromePasswordManagerClient
       content::NavigationHandle* navigation_handle) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
+#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
+  void OnPaste() override;
+#endif
 
 // TODO(crbug.com/706392): Fix password reuse detection for Android.
 #if !defined(OS_ANDROID)
@@ -319,6 +317,9 @@ class ChromePasswordManagerClient
   // WebContents. Used for testing.
   bool was_store_ever_called_ = false;
 
+  // Whether OnPaste() was called from this ChromePasswordManagerClient
+  bool was_on_paste_called_ = false;
+
   // Helper for performing logic that is common between
   // ChromePasswordManagerClient and IOSChromePasswordManagerClient.
   password_manager::PasswordManagerClientHelper helper_;

chrome/browser/password_manager/password_manager_browsertest.cc

@@ -45,7 +45,6 @@
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/password_manager/core/browser/http_auth_manager.h"
 #include "components/password_manager/core/browser/http_auth_observer.h"
-#include "components/password_manager/core/browser/mock_password_store.h"
 #include "components/password_manager/core/browser/new_password_form_manager.h"
 #include "components/password_manager/core/browser/test_password_store.h"
 #include "components/password_manager/core/common/password_manager_features.h"
@@ -61,14 +60,12 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
-#include "content/public/test/web_contents_tester.h"
 #include "net/base/filename_util.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "third_party/blink/public/platform/web_input_event.h"
-#include "ui/base/clipboard/test/test_clipboard.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/geometry/point.h"
@@ -4033,22 +4030,5 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest, FormDynamicallyChanged) {
   WaitForElementValue("password_field", "pw");
 }
 
-IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTest,
-                       CheckOnPasteCalledForPasteEvent) {
-  ::ui::Clipboard::SetClipboardForCurrentThread(
-      std::make_unique<::ui::TestClipboard>());
-  const std::string password = "password";
-  static_cast<::ui::TestClipboard*>(::ui::Clipboard::GetForCurrentThread())
-      ->WriteText(password.data(), password.length());
-  NavigateToFile("/password/password_form.html");
-  CustomPasswordManagerClient* client =
-      static_cast<CustomPasswordManagerClient*>(
-          ChromePasswordManagerClient::FromWebContents(WebContents()));
-  content::SimulateKeyPress(WebContents(), ::ui::DomKey::PASTE,
-                            ::ui::DomCode::PASTE, ::ui::VKEY_PASTE, true, true,
-                            false, true);
-  EXPECT_EQ(client->pasted_value(), base::ASCIIToUTF16("password"));
-}
-
 }  // namespace
 }  // namespace password_manager

chrome/browser/password_manager/password_manager_test_base.cc

@@ -62,6 +62,26 @@ class PasswordStoreResultsObserver
   DISALLOW_COPY_AND_ASSIGN(PasswordStoreResultsObserver);
 };
 
+// Custom class is required to enable password generation.
+class CustomPasswordManagerClient : public ChromePasswordManagerClient {
+ public:
+  using ChromePasswordManagerClient::ChromePasswordManagerClient;
+
+  static void CreateForWebContentsWithAutofillClient(
+      content::WebContents* contents,
+      autofill::AutofillClient* autofill_client) {
+    ASSERT_FALSE(FromWebContents(contents));
+    contents->SetUserData(UserDataKey(),
+                          base::WrapUnique(new CustomPasswordManagerClient(
+                              contents, autofill_client)));
+  }
+
+  // PasswordManagerClient:
+  password_manager::SyncState GetPasswordSyncState() const override {
+    return password_manager::SYNCING_NORMAL_ENCRYPTION;
+  }
+};
+
 // ManagePasswordsUIController subclass to capture the UI events.
 class CustomManagePasswordsUIController : public ManagePasswordsUIController {
  public:
@@ -283,25 +303,6 @@ enum ReturnCodes {  // Possible results of the JavaScript code.
 };
 
 }  // namespace
-void CustomPasswordManagerClient::CreateForWebContentsWithAutofillClient(
-    content::WebContents* contents,
-    autofill::AutofillClient* autofill_client) {
-  ASSERT_FALSE(FromWebContents(contents));
-  contents->SetUserData(UserDataKey(),
-                        base::WrapUnique(new CustomPasswordManagerClient(
-                            contents, autofill_client)));
-}
-
-// PasswordManagerClient:
-password_manager::SyncState CustomPasswordManagerClient::GetPasswordSyncState()
-    const {
-  return password_manager::SYNCING_NORMAL_ENCRYPTION;
-}
-
-void CustomPasswordManagerClient::OnPaste() {
-  pasted_value_ = ChromePasswordManagerClient::GetTextFromClipboard();
-  ChromePasswordManagerClient::OnPaste();
-}
 
 NavigationObserver::NavigationObserver(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),

chrome/browser/password_manager/password_manager_test_base.h

@@ -9,7 +9,6 @@
 
 #include "base/macros.h"
 #include "base/run_loop.h"
-#include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/ssl/cert_verifier_browser_test.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
@@ -22,26 +21,6 @@ struct PasswordForm;
 
 class ManagePasswordsUIController;
 
-// Custom class is required to enable password generation.
-class CustomPasswordManagerClient : public ChromePasswordManagerClient {
- public:
-  using ChromePasswordManagerClient::ChromePasswordManagerClient;
-  static void CreateForWebContentsWithAutofillClient(
-      content::WebContents* contents,
-      autofill::AutofillClient* autofill_client);
-
-  // PasswordManagerClient:
-  password_manager::SyncState GetPasswordSyncState() const override;
-
-  void OnPaste() override;
-
-  base::string16 pasted_value() { return pasted_value_; }
-
- private:
-  // Represents the text passed to PasswordResuseManager's OnPaste() method.
-  base::string16 pasted_value_;
-};
-
 class NavigationObserver : public content::WebContentsObserver {
  public:
   explicit NavigationObserver(content::WebContents* web_contents);