New issue
Advanced search Search tips

Issue 776474 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Only record aborts for navigations in the main frame for payment request

Project Member Reported by se...@chromium.org, Oct 19 2017

Issue description

There is a bug in the current implementation that causes navigations in an iframe to be seen as an abort for the Payment Request even though it's not canceled.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 20 2017

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

commit d56b3e4213be1de11246db3aafbb1dd76ec6862d
Author: sebsg <sebsg@chromium.org>
Date: Fri Oct 20 18:08:08 2017

[Payments] Only record aborts for nav. in main frame for PR.

Merchant aborts could be recorded when an iframe navigated or if there
were other changes related to the same document. A history pushState
for example.

Since the only way a navigation can close a Payment Request is when the
main frame is navigating to a different document, that's the only
situation in which we should record the abort.

This CL only affects metrics, there should be no user visible change.

Bug:  776474 
Change-Id: Ibe8ffc41afbafdf645dc7d970bfc4632ce03e04a
Reviewed-on: https://chromium-review.googlesource.com/728859
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510491}
[modify] https://crrev.com/d56b3e4213be1de11246db3aafbb1dd76ec6862d/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/d56b3e4213be1de11246db3aafbb1dd76ec6862d/components/payments/content/payment_request.cc
[modify] https://crrev.com/d56b3e4213be1de11246db3aafbb1dd76ec6862d/components/payments/content/payment_request.h
[modify] https://crrev.com/d56b3e4213be1de11246db3aafbb1dd76ec6862d/components/payments/content/payment_request_web_contents_manager.cc
[add] https://crrev.com/d56b3e4213be1de11246db3aafbb1dd76ec6862d/components/test/data/payments/payment_request_free_shipping_with_iframe_test.html

Comment 2 by ma...@chromium.org, Oct 23 2017

Fixed?

Comment 3 by se...@chromium.org, Oct 23 2017

Labels: Merge-Request-63
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 24 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by gov...@chromium.org, Oct 25 2017

Please merge you change to M63 branch 3239 by 4:00 PM PT, tomorrow (Wednesday). Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 25 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79b6d2ab7efc7a9dd815ef7172621bafa540231b

commit 79b6d2ab7efc7a9dd815ef7172621bafa540231b
Author: sebsg <sebsg@chromium.org>
Date: Wed Oct 25 15:59:26 2017

Merge-63 [Payments] Only record aborts for nav. in main frame for PR.

Merchant aborts could be recorded when an iframe navigated or if there
were other changes related to the same document. A history pushState
for example.

Since the only way a navigation can close a Payment Request is when the
main frame is navigating to a different document, that's the only
situation in which we should record the abort.

This CL only affects metrics, there should be no user visible change.

TBR=sebsg@chromium.org

(cherry picked from commit d56b3e4213be1de11246db3aafbb1dd76ec6862d)

Bug:  776474 
Change-Id: Ibe8ffc41afbafdf645dc7d970bfc4632ce03e04a
Reviewed-on: https://chromium-review.googlesource.com/728859
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#510491}
Reviewed-on: https://chromium-review.googlesource.com/737582
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#218}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/79b6d2ab7efc7a9dd815ef7172621bafa540231b/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/79b6d2ab7efc7a9dd815ef7172621bafa540231b/components/payments/content/payment_request.cc
[modify] https://crrev.com/79b6d2ab7efc7a9dd815ef7172621bafa540231b/components/payments/content/payment_request.h
[modify] https://crrev.com/79b6d2ab7efc7a9dd815ef7172621bafa540231b/components/payments/content/payment_request_web_contents_manager.cc
[add] https://crrev.com/79b6d2ab7efc7a9dd815ef7172621bafa540231b/components/test/data/payments/payment_request_free_shipping_with_iframe_test.html

Comment 7 by se...@chromium.org, Oct 25 2017

Status: Fixed (was: Started)

Sign in to add a comment