New issue
Advanced search Search tips

Issue 859761 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Investigate if CreditCardSaveManager::IsCreditCardUploadEnabled() can ignore observer_for_testing_

Project Member Reported by jsaul@google.com, Jul 3

Issue description

CreditCardSaveManager::IsCreditCardUploadEnabled() uses observer_for_testing_ to automatically return true if assumed to be in a browsertest:
https://cs.chromium.org/chromium/src/components/autofill/core/browser/credit_card_save_manager.cc?l=185&rcl=0d352cd953739e59a0d8b07375867ffaf4e7d8b7

However, as part of https://chromium-review.googlesource.com/c/chromium/src/+/1123776, new code to log in the user during browsertests exists.  Investigate if this is enough to bypass the observer_for_testing_ hack, or if additional sync service setup needs to happen as well.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/995f8099303a3eb47660661b5093538d08d544d3

commit 995f8099303a3eb47660661b5093538d08d544d3
Author: Florian Uunk <feuunk@chromium.org>
Date: Mon Nov 26 17:20:49 2018

Make SaveCardBubble browsertest use the sync state

The browsertests used to rely on the save manager being hardcoded to
upload to Google if the observer_for_testing was set. This makes it
impossible to test that the logic for showing the upload prompt works
properly.

BUG= 906630 , 859761

Change-Id: Id7352a869388aac4ff36e1284750e22ac68c91b3
Reviewed-on: https://chromium-review.googlesource.com/c/1348335
Commit-Queue: Florian Uunk <feuunk@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610866}
[modify] https://crrev.com/995f8099303a3eb47660661b5093538d08d544d3/chrome/browser/ui/views/autofill/save_card_bubble_views_browsertest.cc
[modify] https://crrev.com/995f8099303a3eb47660661b5093538d08d544d3/chrome/browser/ui/views/autofill/save_card_bubble_views_browsertest_base.cc
[modify] https://crrev.com/995f8099303a3eb47660661b5093538d08d544d3/chrome/browser/ui/views/autofill/save_card_bubble_views_browsertest_base.h
[modify] https://crrev.com/995f8099303a3eb47660661b5093538d08d544d3/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/995f8099303a3eb47660661b5093538d08d544d3/components/autofill/core/browser/credit_card_save_manager.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b54154ace3a231159f9abe78c134dfd62f353ef6

commit b54154ace3a231159f9abe78c134dfd62f353ef6
Author: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Date: Wed Nov 28 20:15:20 2018

Revert "Make SaveCardBubble browsertest use the sync state"

This reverts commit 995f8099303a3eb47660661b5093538d08d544d3.

Reason for revert: Breaks browser tests on continues builders (consistent failures) See crbug.com/908787

Original change's description:
> Make SaveCardBubble browsertest use the sync state
> 
> The browsertests used to rely on the save manager being hardcoded to
> upload to Google if the observer_for_testing was set. This makes it
> impossible to test that the logic for showing the upload prompt works
> properly.
> 
> BUG= 906630 , 859761
> 
> Change-Id: Id7352a869388aac4ff36e1284750e22ac68c91b3
> Reviewed-on: https://chromium-review.googlesource.com/c/1348335
> Commit-Queue: Florian Uunk <feuunk@chromium.org>
> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
> Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610866}

TBR=vasilii@chromium.org,sebsg@chromium.org,feuunk@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  906630 , 859761
Change-Id: I145d2b56295af6014b6a94c44beb38b1bf6b9e01
Reviewed-on: https://chromium-review.googlesource.com/c/1352680
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611844}
[modify] https://crrev.com/b54154ace3a231159f9abe78c134dfd62f353ef6/chrome/browser/ui/views/autofill/save_card_bubble_views_browsertest.cc
[modify] https://crrev.com/b54154ace3a231159f9abe78c134dfd62f353ef6/chrome/browser/ui/views/autofill/save_card_bubble_views_browsertest_base.cc
[modify] https://crrev.com/b54154ace3a231159f9abe78c134dfd62f353ef6/chrome/browser/ui/views/autofill/save_card_bubble_views_browsertest_base.h
[modify] https://crrev.com/b54154ace3a231159f9abe78c134dfd62f353ef6/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/b54154ace3a231159f9abe78c134dfd62f353ef6/components/autofill/core/browser/credit_card_save_manager.cc

Sign in to add a comment