[sync] Fix android open-settings promos not showing [chromium/src : main]

0 views
Skip to first unread message

Mahmoud Rashad (Gerrit)

unread,
Jul 2, 2022, 4:04:01 AM7/2/22
to browser-comp...@chromium.org, Marc Treib, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Marc Treib.

View Change

1 comment:

To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
Gerrit-Change-Number: 3740569
Gerrit-PatchSet: 1
Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
Gerrit-Attention: Marc Treib <tr...@chromium.org>
Gerrit-Comment-Date: Sat, 02 Jul 2022 08:03:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Marc Treib (Gerrit)

unread,
Jul 4, 2022, 4:11:35 AM7/4/22
to Mahmoud Rashad, browser-comp...@chromium.org, Marc Treib, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Mahmoud Rashad.

Patch set 1:Code-Review +1

View Change

    To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
    Gerrit-Change-Number: 3740569
    Gerrit-PatchSet: 1
    Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
    Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Comment-Date: Mon, 04 Jul 2022 08:11:19 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Marc Treib (Gerrit)

    unread,
    Jul 4, 2022, 4:12:01 AM7/4/22
    to Mahmoud Rashad, browser-comp...@chromium.org, Marc Treib, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Mahmoud Rashad.

    View Change

    1 comment:

    • Commit Message:

      • Patch Set #1, Line 10: without any feature flags

        nit: What does this refer to? What feature flags would there be?

    To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
    Gerrit-Change-Number: 3740569
    Gerrit-PatchSet: 1
    Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
    Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Comment-Date: Mon, 04 Jul 2022 08:11:47 +0000

    Mahmoud Rashad (Gerrit)

    unread,
    Jul 5, 2022, 3:51:05 AM7/5/22
    to browser-comp...@chromium.org, Marc Treib, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Marc Treib.

    View Change

    2 comments:

    • Commit Message:

      • Patch Set #1, Line 10: without any feature flags

        nit: What does this refer to? What feature flags would there be?

      • I meant this change is not behind any feature flags, I thinks it's better now.

    • Patchset:

    To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
    Gerrit-Change-Number: 3740569
    Gerrit-PatchSet: 2
    Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
    Gerrit-Attention: Marc Treib <tr...@chromium.org>
    Gerrit-Comment-Date: Tue, 05 Jul 2022 07:50:56 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Marc Treib <tr...@chromium.org>
    Gerrit-MessageType: comment

    Marc Treib (Gerrit)

    unread,
    Jul 6, 2022, 8:32:42 AM7/6/22
    to Mahmoud Rashad, browser-comp...@chromium.org, Marc Treib, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Mahmoud Rashad.

    Patch set 2:Code-Review +1

    View Change

    1 comment:

    • Commit Message:

      • I meant this change is not behind any feature flags, I thinks it's better now.

        Ack, thanks!

    To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
    Gerrit-Change-Number: 3740569
    Gerrit-PatchSet: 2
    Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
    Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Comment-Date: Wed, 06 Jul 2022 12:32:26 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Mahmoud Rashad <mmra...@google.com>

    Mahmoud Rashad (Gerrit)

    unread,
    Jul 6, 2022, 8:38:39 AM7/6/22
    to browser-comp...@chromium.org, Marc Treib, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Mahmoud Rashad.

    Patch set 2:Commit-Queue +2

    View Change

      To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
      Gerrit-Change-Number: 3740569
      Gerrit-PatchSet: 2
      Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
      Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Comment-Date: Wed, 06 Jul 2022 12:38:31 +0000

      Chromium LUCI CQ (Gerrit)

      unread,
      Jul 6, 2022, 9:53:02 AM7/6/22
      to Mahmoud Rashad, browser-comp...@chromium.org, Marc Treib, chromium...@chromium.org

      Chromium LUCI CQ submitted this change.

      View Change


      Approvals: Mahmoud Rashad: Commit Marc Treib: Looks good to me
      [sync] Fix android open-settings promos not showing

      This CL makes sure that android open-settings promos show when no data
      is selected to sync.

      This change is not behind any feature flags.

      Fixed: 1341324
      Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
      Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3740569
      Reviewed-by: Marc Treib <tr...@chromium.org>
      Commit-Queue: Mahmoud Rashad <mmra...@google.com>
      Cr-Commit-Position: refs/heads/main@{#1021163}
      ---
      M chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java
      M chrome/android/java/src/org/chromium/chrome/browser/signin/SyncPromoView.java
      2 files changed, 23 insertions(+), 2 deletions(-)

      diff --git a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java
      index 7880f29..135f05a 100644
      --- a/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java
      +++ b/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPromoHeader.java
      @@ -187,7 +187,8 @@
      SharedPreferencesManager.getInstance().readInt(
      ChromePreferenceKeys.SIGNIN_AND_SYNC_PROMO_SHOW_COUNT)
      < MAX_SIGNIN_AND_SYNC_PROMO_SHOW_COUNT;
      - if (!mSyncService.isSyncRequested() && impressionLimitNotReached) {
      + if ((!mSyncService.isSyncRequested() || mSyncService.getChosenDataTypes().isEmpty())
      + && impressionLimitNotReached) {
      return SyncPromoState.PROMO_FOR_SYNC_TURNED_OFF_STATE;
      }
      return SyncPromoState.NO_PROMO;
      diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/SyncPromoView.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/SyncPromoView.java
      index 87be5f5..5b190c4 100644
      --- a/chrome/android/java/src/org/chromium/chrome/browser/signin/SyncPromoView.java
      +++ b/chrome/android/java/src/org/chromium/chrome/browser/signin/SyncPromoView.java
      @@ -92,7 +92,8 @@

      private void update() {
      ViewState viewState;
      - if (!SyncService.get().isSyncRequested()) {
      + if (!SyncService.get().isSyncRequested()
      + || SyncService.get().getChosenDataTypes().isEmpty()) {
      viewState = getStateForEnableChromeSync();
      } else {
      viewState = getStateForStartUsing();

      To view, visit change 3740569. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I3f46e0668c328e9735ce19df88ff36dc5941ae3d
      Gerrit-Change-Number: 3740569
      Gerrit-PatchSet: 3
      Gerrit-Owner: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Reviewer: Marc Treib <tr...@chromium.org>
      Gerrit-MessageType: merged
      Reply all
      Reply to author
      Forward
      0 new messages