Record Journey Logger events when the page goes away. |
||||||||||
Issue descriptionPayment Request instances tied to a page must record the appropriate Journey Logger events when the page/tab goes away.
,
Sep 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/81b057f734465e321da2ba8221d29605a34cba75 commit 81b057f734465e321da2ba8221d29605a34cba75 Author: Mohamad Ahmadi <mahmadi@chromium.org> Date: Fri Sep 01 20:50:44 2017 [Payment Request] Records metrics when request is aborted or page goes away Refactors the abort logic and helper functions. Records the appropriate journey logger metrics when the request is aborted. Records the appropriate journey logger metrics when the page/tab goes away. Adds the remainder of the journey logger tests. Bug: 761357 , 602666 Change-Id: I65a1196a530c2bded086e1edd94d915f3ff8acc8 Reviewed-on: https://chromium-review.googlesource.com/646877 Commit-Queue: mahmadi (Moe) <mahmadi@chromium.org> Reviewed-by: Marc-Antoine Courteau <macourteau@chromium.org> Cr-Commit-Position: refs/heads/master@{#499287} [modify] https://crrev.com/81b057f734465e321da2ba8221d29605a34cba75/ios/chrome/browser/ui/payments/payment_request_journey_logger_egtest.mm [modify] https://crrev.com/81b057f734465e321da2ba8221d29605a34cba75/ios/chrome/browser/ui/payments/payment_request_manager.mm
,
Sep 5 2017
,
Sep 8 2017
,
Sep 9 2017
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/81962dd0b997caed055fa5589c52f724aff342d2 commit 81962dd0b997caed055fa5589c52f724aff342d2 Author: Mohamad Ahmadi <mahmadi@chromium.org> Date: Mon Sep 11 17:49:11 2017 [Payment Request] Records metrics when request is aborted or page goes away Refactors the abort logic and helper functions. Records the appropriate journey logger metrics when the request is aborted. Records the appropriate journey logger metrics when the page/tab goes away. Adds the remainder of the journey logger tests. TBR=mahmadi@chromium.org (cherry picked from commit 81b057f734465e321da2ba8221d29605a34cba75) Bug: 761357 , 602666 Change-Id: I65a1196a530c2bded086e1edd94d915f3ff8acc8 Reviewed-on: https://chromium-review.googlesource.com/646877 Commit-Queue: mahmadi (Moe) <mahmadi@chromium.org> Reviewed-by: Marc-Antoine Courteau <macourteau@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#499287} Reviewed-on: https://chromium-review.googlesource.com/660400 Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#132} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/81962dd0b997caed055fa5589c52f724aff342d2/ios/chrome/browser/ui/payments/payment_request_journey_logger_egtest.mm [modify] https://crrev.com/81962dd0b997caed055fa5589c52f724aff342d2/ios/chrome/browser/ui/payments/payment_request_manager.mm
,
Sep 12 2017
The CL that was cherry-picked depends on https://chromium-review.googlesource.com/c/chromium/src/+/646660 that has not been cherry-picked, thus breaking the build with the following error: ../../ios/chrome/browser/ui/payments/payment_request_manager.mm:1078:23: error: no member named 'IsRendererInitiated' in 'web::NavigationContext' navigation->IsRendererInitiated() ~~~~~~~~~~ ^ 1 error generated. Will revert the cherry-picking.
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0aa757d1ebc735a2533a8a77a11bd352f378a99e commit 0aa757d1ebc735a2533a8a77a11bd352f378a99e Author: Sylvain Defresne <sdefresne@chromium.org> Date: Tue Sep 12 16:23:56 2017 Revert "[Payment Request] Records metrics when request is aborted or page goes away" This reverts commit 81962dd0b997caed055fa5589c52f724aff342d2. Reason for revert: this CL depends on https://chromium-review.googlesource.com/c/chromium/src/+/646660 that has not been cherry-picked in branch 3202 thus breaking the build with the following error: ../../ios/chrome/browser/ui/payments/payment_request_manager.mm:1078:23: error: no member named 'IsRendererInitiated' in 'web::NavigationContext' navigation->IsRendererInitiated() ~~~~~~~~~~ ^ 1 error generated. Original change's description: > [Payment Request] Records metrics when request is aborted or page goes away > > Refactors the abort logic and helper functions. > Records the appropriate journey logger metrics when the request is aborted. > Records the appropriate journey logger metrics when the page/tab goes away. > Adds the remainder of the journey logger tests. > > TBR=mahmadi@chromium.org > > (cherry picked from commit 81b057f734465e321da2ba8221d29605a34cba75) > > Bug: 761357 , 602666 > Change-Id: I65a1196a530c2bded086e1edd94d915f3ff8acc8 > Reviewed-on: https://chromium-review.googlesource.com/646877 > Commit-Queue: mahmadi (Moe) <mahmadi@chromium.org> > Reviewed-by: Marc-Antoine Courteau <macourteau@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#499287} > Reviewed-on: https://chromium-review.googlesource.com/660400 > Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> > Cr-Commit-Position: refs/branch-heads/3202@{#132} > Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} TBR=macourteau@chromium.org,mahmadi@chromium.org Change-Id: Idca82702136652d17d07b2c315243a7c925db34e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 761357 , 602666 Reviewed-on: https://chromium-review.googlesource.com/663540 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#168} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/0aa757d1ebc735a2533a8a77a11bd352f378a99e/ios/chrome/browser/ui/payments/payment_request_journey_logger_egtest.mm [modify] https://crrev.com/0aa757d1ebc735a2533a8a77a11bd352f378a99e/ios/chrome/browser/ui/payments/payment_request_manager.mm
,
Sep 12 2017
Back to requesting the merge I guess, but need to merge the following two CLs (in order): - https://chromium-review.googlesource.com/c/chromium/src/+/646660 - https://chromium-review.googlesource.com/c/chromium/src/+/646877
,
Sep 12 2017
This list should have been: - https://chromium-review.googlesource.com/c/chromium/src/+/646660 - https://chromium-review.googlesource.com/c/chromium/src/+/660400
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f88833b45c3d5b832e51a4ec2759c90252d2ca7 commit 3f88833b45c3d5b832e51a4ec2759c90252d2ca7 Author: Mohamad Ahmadi <mahmadi@chromium.org> Date: Tue Sep 12 16:43:46 2017 Adds IsRendererInitiated to NavigationContext. This mimics content/public/browser/navigation_handle.h. IsRenderInitiated will return true if the current navigation was initiated by the renderer, and false if it was user initiated. TBR=macourteau@chromium.org (cherry picked from commit c6d51440206327ee177c949a4e7e1e9a8120a555) Bug: 761357 Change-Id: I094442e501a17a4741edca228810533e4b08253f Reviewed-on: https://chromium-review.googlesource.com/646660 Commit-Queue: Marc-Antoine Courteau <macourteau@chromium.org> Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org> Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#499188} Reviewed-on: https://chromium-review.googlesource.com/663562 Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#170} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/public/test/fakes/crw_test_web_state_observer.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/public/test/fakes/fake_navigation_context.h [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/public/test/fakes/fake_navigation_context.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/public/test/fakes/test_web_state_observer.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/public/web_state/navigation_context.h [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/navigation_context_impl_unittest.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/web_state_impl_unittest.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/web_state/web_state_observer_bridge_unittest.mm [modify] https://crrev.com/3f88833b45c3d5b832e51a4ec2759c90252d2ca7/ios/web/webui/crw_web_ui_manager_unittest.mm
,
Sep 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d2d189703dde91d6431ff952d31bbfa7f3dcb577 commit d2d189703dde91d6431ff952d31bbfa7f3dcb577 Author: Moe Ahmadi <mahmadi@chromium.org> Date: Tue Sep 12 17:47:07 2017 Revert "Revert "[Payment Request] Records metrics when request is aborted or page goes away"" This reverts commit 0aa757d1ebc735a2533a8a77a11bd352f378a99e. Reason for revert: <INSERT REASONING HERE> Original change's description: > Revert "[Payment Request] Records metrics when request is aborted or page goes away" > > This reverts commit 81962dd0b997caed055fa5589c52f724aff342d2. > > Reason for revert: this CL depends on https://chromium-review.googlesource.com/c/chromium/src/+/646660 that has not been cherry-picked in branch 3202 thus breaking the build with the following error: > > ../../ios/chrome/browser/ui/payments/payment_request_manager.mm:1078:23: error: no member named 'IsRendererInitiated' in 'web::NavigationContext' > navigation->IsRendererInitiated() > ~~~~~~~~~~ ^ > 1 error generated. > > Original change's description: > > [Payment Request] Records metrics when request is aborted or page goes away > > > > Refactors the abort logic and helper functions. > > Records the appropriate journey logger metrics when the request is aborted. > > Records the appropriate journey logger metrics when the page/tab goes away. > > Adds the remainder of the journey logger tests. > > > > TBR=mahmadi@chromium.org > > > > (cherry picked from commit 81b057f734465e321da2ba8221d29605a34cba75) > > > > Bug: 761357 , 602666 > > Change-Id: I65a1196a530c2bded086e1edd94d915f3ff8acc8 > > Reviewed-on: https://chromium-review.googlesource.com/646877 > > Commit-Queue: mahmadi (Moe) <mahmadi@chromium.org> > > Reviewed-by: Marc-Antoine Courteau <macourteau@chromium.org> > > Cr-Original-Commit-Position: refs/heads/master@{#499287} > > Reviewed-on: https://chromium-review.googlesource.com/660400 > > Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> > > Cr-Commit-Position: refs/branch-heads/3202@{#132} > > Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} > > TBR=macourteau@chromium.org,mahmadi@chromium.org > > Change-Id: Idca82702136652d17d07b2c315243a7c925db34e > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 761357 , 602666 > Reviewed-on: https://chromium-review.googlesource.com/663540 > Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> > Cr-Commit-Position: refs/branch-heads/3202@{#168} > Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} TBR=macourteau@chromium.org,sdefresne@chromium.org,mahmadi@chromium.org Change-Id: I481de4fd41e4b83218283ffc8b4dbf29a7332f2d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 761357 , 602666 Reviewed-on: https://chromium-review.googlesource.com/663565 Reviewed-by: Moe Ahmadi <mahmadi@chromium.org> Reviewed-by: Marc-Antoine Courteau <macourteau@chromium.org> Cr-Commit-Position: refs/branch-heads/3202@{#174} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} [modify] https://crrev.com/d2d189703dde91d6431ff952d31bbfa7f3dcb577/ios/chrome/browser/ui/payments/payment_request_journey_logger_egtest.mm [modify] https://crrev.com/d2d189703dde91d6431ff952d31bbfa7f3dcb577/ios/chrome/browser/ui/payments/payment_request_manager.mm
,
Sep 12 2017
,
Sep 12 2017
mahmadi@ please provide the rationale as to why do we need to merge this please. How will happen if we don't take this? What could be impacted in the worst case scenario if we merge this.
,
Sep 12 2017
I am seeing a lot of stuffs on payment request that needs to be merged. Was this feature code complete by Feature freeze?
,
Sep 13 2017
Approving this as per our email conversation.
,
Sep 18 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 18 2017
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by mahmadi@chromium.org
, Sep 1 2017