Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(119)

Issue 529743002: [Password Manager] Restrict IsTheHotNewBubbleUIEnabled to Aura and Mac (Closed)

Created:
6 years, 3 months ago by vabr (Chromium)
Modified:
6 years, 3 months ago
Reviewers:
Mike West, dconnelly
CC:
chromium-reviews, gcasto+watchlist_chromium.org, mkwst+watchlist_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

[Password Manager] Restrict IsTheHotNewBubbleUIEnabled to Aura and Mac ChromePasswordManagerClient::IsTheHotNewBubbleUIEnabled needs to return false on Android, because there is no bubble for Android. Unintentionally, I (vabr) misinformed dconnelly in https://codereview.chromium.org/444603003/diff/20001/chrome/browser/password_manager/chrome_password_manager_client.cc#newcode357 that IsTheHotNewBubbleUIEnabled is not compiled on Android at all, whereas the truth is, it is only not compiled for Androd WebView. As a consequence, IsTheHotNewBubbleUIEnabled started to return possibly true on Android after https://codereview.chromium.org/505123002 landed, causing a crash on the bots. BUG=328847, 409622 Committed: https://crrev.com/89a280795dfc11b6fa3139f5760aae614399358e Cr-Commit-Position: refs/heads/master@{#292884}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -0 lines) Patch
M chrome/browser/password_manager/chrome_password_manager_client.cc View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
vabr (Chromium)
Daniel, Mike, Whoever gets to this easy review faster, please have a look. It's a ...
6 years, 3 months ago (2014-09-01 14:00:31 UTC) #2
dconnelly
lgtm
6 years, 3 months ago (2014-09-01 14:12:41 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vabr@chromium.org/529743002/1
6 years, 3 months ago (2014-09-01 14:15:49 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vabr@chromium.org/529743002/1
6 years, 3 months ago (2014-09-01 16:13:28 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vabr@chromium.org/529743002/1
6 years, 3 months ago (2014-09-01 16:40:11 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1) as 2815880667a3624154785cc0f84eedfe95b05d3a
6 years, 3 months ago (2014-09-01 17:02:52 UTC) #12
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:17:14 UTC) #13
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/89a280795dfc11b6fa3139f5760aae614399358e
Cr-Commit-Position: refs/heads/master@{#292884}

Powered by Google App Engine
This is Rietveld 408576698