Issue 1066269 in chromium: Create separate build target for chrome/browser/payments/android

8 views
Skip to first unread message

danyao via monorail

unread,
Mar 30, 2020, 7:17:22 PM3/30/20
to paymen...@chromium.org
Status: Available
Owner: ----
CC: paymen...@chromium.org
Components: UI>Browser>Payments
OS: Android
Pri: 3
Type: Task

New issue 1066269 by dan...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269

Today add new files to //chrome/browser/payments/android requires updating //chrome/browser/BUILD.gn. Creating a new static_library rule will have three benefits:

1) Allowing add payments-specific files without continuing to bloat the top-level BUILD.gn.
2) Better encapsulate payments code and clarify dependency on other components

See crbug.com/2125770 for a prototype.

A blocker is extracting //chrome/browser/autofill source files into its own library as well because without this step, there is a circular dependency between //chrome/browser -> //chrome/browser/payments/android -> //chrome/browser.

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment or make updates.

yfriedman via monorail

unread,
Mar 31, 2020, 9:06:37 AM3/31/20
to paymen...@chromium.org
Updates:
Cc: hnaka...@chromium.org

Comment #1 on issue 1066269 by yfri...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c1

Sounds great! Would be awesome to align with ongoing modularizing efforts in go/clank-hoa

Henrique - any pointers for folks?

hnakashima via monorail

unread,
Mar 31, 2020, 3:30:48 PM3/31/20
to paymen...@chromium.org
Updates:
Cc: twell...@chromium.org dtra...@chromium.org mdj...@chromium.org

Comment #2 on issue 1066269 by hnaka...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c2

Though this bug and the prototype refer to native code, the level of decoupling that .payment already has in Java means there is a great opportunity to modularize both sides here.

I did a quick audit based on java imports from chrome.browser.payments and found these to be the dependencies:

Already being modularized:
- ChromeActivity
- .tabmodel
- .tab
- .widget.bottomsheet

Dependencies left to modularize:
- .autofill
- .compositor.layouts (PaymentRequestImpl.java)
- .ssl. ChromeSecurityStateModelDelegate (PaymentRequestImpl.java)
- ChromeVersionInfo
- WebContentsFactory (PaymentHandlerCoordinator.java)
- .offlinepages.OfflinePageUtils (PaymentHandlerToolbarMediator.java)
- .page_info.PageInfoController (PaymentHandlerToolbarMediator.java)
- .omnibox.ChromeAutocompleteSchemeClassifier (PaymentRequestHeader.java)
- .signin.IdentityServicesProvider (PaymentRequestUI.java)

That's a relatively short list, blocked mostly by .autofill (like danyao@ mentioned about native) on the Java side - besides .tabmodel and .bottomsheet that are being worked on.

twellington via monorail

unread,
Mar 31, 2020, 4:21:07 PM3/31/20
to paymen...@chromium.org

Comment #3 on issue 1066269 by twell...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c3

One quick note -- ChromeActivity isn't being "modularized" (moved to its own build target). Rather, we are asking that features drop all dependencies on ChromeActivity since it generally creates circular dependencies and ChromeActivity is our main "glue" class.

dtrainor via monorail

unread,
Apr 1, 2020, 4:22:40 PM4/1/20
to paymen...@chromium.org

Comment #4 on issue 1066269 by dtra...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c4

Yeah this sounds awesome. Happy to help in any way. Just let us know!

I wonder if some of those dependencies are actually hard dependencies or things that could be pulled out through the public interface (e.g. TabModel becomes ObservableSupplier of <Tab> and a <Boolean> one for incognito status or something). Obviously I'd love to have those dependencies modularized as well, but it might be worth looking into if some of those dependencies are non-trivial to modularize themselves.

maxlg via monorail

unread,
Dec 11, 2020, 2:17:11 PM12/11/20
to paymen...@chromium.org
Updates:
Cc: ma...@chromium.org

Comment #7 on issue 1066269 by ma...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c7

(No comment was entered for this change.)

maxlg via monorail

unread,
Jan 25, 2021, 11:33:26 AM1/25/21
to paymen...@chromium.org
Updates:
Blockedon: 1169926

Comment #8 on issue 1066269 by ma...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c8

sheriffbot via monorail

unread,
Jan 25, 2022, 11:58:24 AM1/25/22
to paymen...@chromium.org
Updates:
Labels: Hotlist-Recharge-Cold
Status: Untriaged

Comment #9 on issue 1066269 by sheriffbot: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c9

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

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

maxlg via monorail

unread,
Feb 20, 2022, 11:18:57 PM2/20/22
to paymen...@chromium.org
Updates:
Status: Available

Comment #10 on issue 1066269 by ma...@chromium.org: Create separate build target for chrome/browser/payments/android
https://bugs.chromium.org/p/chromium/issues/detail?id=1066269#c10

Still being blocked
Reply all
Reply to author
Forward
0 new messages