Skip to content

Commit

Permalink
Revert "[IBAN local save] Add manage IBAN option after IBAN saved."
Browse files Browse the repository at this point in the history
This reverts commit 152299a.

Reason for revert: This is causing consistent failures on the Linux CFI builder (CFI meaning control flow integrity).  The failure is:

../../chrome/browser/ui/views/autofill/payments/save_payment_icon_view.cc:58:12: runtime error: control flow integrity check for type 'autofill::SaveIbanBubbleView' failed during base-to-derived cast (vtable address 0x56424e739618)
0x56424e739618: note: vtable is of type 'autofill::ManageSavedIbanBubbleView'
 42 56 00 00  40 d2 46 44 42 56 00 00  00 d5 46 44 42 56 00 00  80 d6 46 44 42 56 00 00  f0 d8 46 44
              ^
    #0 0x5642445ddef0 in autofill::SavePaymentIconView::GetBubble() const ./../../chrome/browser/ui/views/autofill/payments/save_payment_icon_view.cc:58:12
    #1 0x5642445e02c2 in PageActionIconView::IsBubbleShowing() const ./../../chrome/browser/ui/views/page_action/page_action_icon_view.cc:98:10
    #2 0x5642445e0780 in PageActionIconView::GetTooltipText(gfx::Point const&) const ./../../chrome/browser/ui/views/page_action/page_action_icon_view.cc:127:10

and more detail can be seen in the log at https://ci.chromium.org/p/chromium/builders/ci/Linux%20CFI/24601

It appears that this CL needed to modify the code in autofill::SavePaymentIconView::GetBubble() but did not do so.

Original change's description:
> [IBAN local save] Add manage IBAN option after IBAN saved.
>
> This CL introduces manage IBAN mode after the user accepts saving IBAN.
> The user can click Chrome omnibox icon to reopen bubble, the user can
> either close the bubble or redirect to Chrome payment settings page to
> view list of IBANs.
>
> Screenshot:
> with nickname: https://screenshot.googleplex.com/7AoywRzg3v5YjLo
> without nickname: https://screenshot.googleplex.com/3DmmfsQhAnNqJUK
>
> UI mock:
> https://screenshot.googleplex.com/4m4ZveVRdx97ikW
>
> Bug: 1349109
> Change-Id: I43a7c2541f8a877063bb277c7f8000bd412f57f7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4209257
> Reviewed-by: Siyu An <siyua@chromium.org>
> Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
> Reviewed-by: Vinny Persky <vinnypersky@google.com>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Commit-Queue: Qihui Zhao <qihuizhao@google.com>
> Cr-Commit-Position: refs/heads/main@{#1106482}

Bug: 1349109
Change-Id: Ib43c596529164ca017f3a9ca897235b88cc8f18f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4261750
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: David Baron <dbaron@chromium.org>
Owners-Override: David Baron <dbaron@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1106747}
  • Loading branch information
dbaron authored and Chromium LUCI CQ committed Feb 17, 2023
1 parent 8198e38 commit d58aa8a
Show file tree
Hide file tree
Showing 23 changed files with 220 additions and 581 deletions.
8 changes: 3 additions & 5 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4217,9 +4217,6 @@ static_library("ui") {
"autofill/edit_address_profile_dialog_controller.h",
"autofill/edit_address_profile_dialog_controller_impl.cc",
"autofill/edit_address_profile_dialog_controller_impl.h",
"autofill/payments/iban_bubble_controller.h",
"autofill/payments/iban_bubble_controller_impl.cc",
"autofill/payments/iban_bubble_controller_impl.h",
"autofill/payments/local_card_migration_bubble_controller_impl.cc",
"autofill/payments/local_card_migration_bubble_controller_impl.h",
"autofill/payments/local_card_migration_controller_observer.h",
Expand All @@ -4237,6 +4234,9 @@ static_library("ui") {
"autofill/payments/save_card_bubble_controller_impl.cc",
"autofill/payments/save_card_bubble_controller_impl.h",
"autofill/payments/save_card_ui.h",
"autofill/payments/save_iban_bubble_controller.h",
"autofill/payments/save_iban_bubble_controller_impl.cc",
"autofill/payments/save_iban_bubble_controller_impl.h",
"autofill/payments/save_iban_ui.h",
"autofill/payments/save_payment_icon_controller.cc",
"autofill/payments/save_payment_icon_controller.h",
Expand Down Expand Up @@ -4359,8 +4359,6 @@ static_library("ui") {
"views/autofill/payments/local_card_migration_error_dialog_view.h",
"views/autofill/payments/local_card_migration_icon_view.cc",
"views/autofill/payments/local_card_migration_icon_view.h",
"views/autofill/payments/manage_saved_iban_bubble_view.cc",
"views/autofill/payments/manage_saved_iban_bubble_view.h",
"views/autofill/payments/migratable_card_view.cc",
"views/autofill/payments/migratable_card_view.h",
"views/autofill/payments/offer_notification_bubble_views.cc",
Expand Down
11 changes: 5 additions & 6 deletions chrome/browser/ui/autofill/autofill_bubble_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@ class OfferNotificationBubbleController;
class SaveUpdateAddressProfileBubbleController;
class EditAddressProfileDialogController;
class SaveCardBubbleController;
class IbanBubbleController;
class SaveIbanBubbleController;
class SaveUPIBubble;
class SaveUPIBubbleController;
class VirtualCardManualFallbackBubbleController;
class VirtualCardEnrollBubbleController;
enum class IbanBubbleType;

// TODO(crbug.com/1337392): consider removing this class and give the logic back
// to each bubble's controller. This class serves also the avatar button /
Expand All @@ -48,10 +47,10 @@ class AutofillBubbleHandler {
LocalCardMigrationBubbleController* controller,
bool is_user_gesture) = 0;

virtual AutofillBubbleBase* ShowIbanBubble(content::WebContents* web_contents,
IbanBubbleController* controller,
bool is_user_gesture,
IbanBubbleType bubble_type) = 0;
virtual AutofillBubbleBase* ShowSaveIbanBubble(
content::WebContents* web_contents,
SaveIbanBubbleController* controller,
bool is_user_gesture) = 0;

virtual AutofillBubbleBase* ShowOfferNotificationBubble(
content::WebContents* web_contents,
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/ui/autofill/chrome_autofill_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include "chrome/browser/ui/autofill/payments/card_unmask_otp_input_dialog_controller_impl.h"
#include "chrome/browser/ui/autofill/payments/create_card_unmask_prompt_view.h"
#include "chrome/browser/ui/autofill/payments/credit_card_scanner_controller.h"
#include "chrome/browser/ui/autofill/payments/iban_bubble_controller_impl.h"
#include "chrome/browser/ui/autofill/payments/save_iban_bubble_controller_impl.h"
#include "chrome/browser/ui/autofill/payments/virtual_card_enroll_bubble_controller_impl.h"
#include "chrome/browser/ui/autofill/risk_util.h"
#include "chrome/browser/ui/autofill/save_update_address_profile_bubble_controller_impl.h"
Expand Down Expand Up @@ -545,9 +545,9 @@ void ChromeAutofillClient::ConfirmSaveIBANLocally(
const IBAN& iban,
bool should_show_prompt,
LocalSaveIBANPromptCallback callback) {
// Do lazy initialization of IbanBubbleControllerImpl.
IbanBubbleControllerImpl::CreateForWebContents(web_contents());
IbanBubbleControllerImpl::FromWebContents(web_contents())
// Do lazy initialization of SaveIbanBubbleControllerImpl.
SaveIbanBubbleControllerImpl::CreateForWebContents(web_contents());
SaveIbanBubbleControllerImpl::FromWebContents(web_contents())
->OfferLocalSave(iban, should_show_prompt, std::move(callback));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_IBAN_BUBBLE_CONTROLLER_H_
#define CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_IBAN_BUBBLE_CONTROLLER_H_
#ifndef CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_SAVE_IBAN_BUBBLE_CONTROLLER_H_
#define CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_SAVE_IBAN_BUBBLE_CONTROLLER_H_

#include <string>

Expand All @@ -17,19 +17,19 @@ class AutofillBubbleBase;
class IBAN;
enum class IbanBubbleType;

// Interface that exposes controller functionality to save and manage IBAN
// bubbles.
class IbanBubbleController {
// Interface that exposes controller functionality to save IBAN bubbles.
class SaveIbanBubbleController {
public:
IbanBubbleController() = default;
IbanBubbleController(const IbanBubbleController&) = delete;
IbanBubbleController& operator=(const IbanBubbleController&) = delete;
virtual ~IbanBubbleController() = default;
SaveIbanBubbleController() = default;
SaveIbanBubbleController(const SaveIbanBubbleController&) = delete;
SaveIbanBubbleController& operator=(const SaveIbanBubbleController&) = delete;
virtual ~SaveIbanBubbleController() = default;

// Returns a reference to the IbanBubbleController associated with the
// Returns a reference to the SaveIbanBubbleController associated with the
// given `web_contents`. If controller does not exist, this will create the
// controller from the `web_contents` then return the reference.
static IbanBubbleController* GetOrCreate(content::WebContents* web_contents);
static SaveIbanBubbleController* GetOrCreate(
content::WebContents* web_contents);

// Returns the title that should be displayed in the bubble.
virtual std::u16string GetWindowTitle() const = 0;
Expand All @@ -38,16 +38,14 @@ class IbanBubbleController {
virtual std::u16string GetAcceptButtonText() const = 0;
virtual std::u16string GetDeclineButtonText() const = 0;

// Returns the IBAN that will be saved in save bubble view or the IBAN that
// has been saved in manage bubble view.
// Returns the IBAN that will be saved if the user accepts.
virtual const IBAN& GetIBAN() const = 0;

virtual AutofillBubbleBase* GetSaveBubbleView() const = 0;

// Interaction.
virtual void OnAcceptButton(const std::u16string& nickname) = 0;
virtual void OnSaveButton(const std::u16string& nickname) = 0;
virtual void OnCancelButton() = 0;
virtual void OnManageSavedIbanExtraButtonClicked() = 0;
virtual void OnBubbleClosed(PaymentsBubbleClosedReason closed_reason) = 0;

// Returns the current state of the bubble.
Expand All @@ -56,4 +54,4 @@ class IbanBubbleController {

} // namespace autofill

#endif // CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_IBAN_BUBBLE_CONTROLLER_H_
#endif // CHROME_BROWSER_UI_AUTOFILL_PAYMENTS_SAVE_IBAN_BUBBLE_CONTROLLER_H_

0 comments on commit d58aa8a

Please sign in to comment.