Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2138643004:
[Autofill] Add support for signin promo on Android (Closed)
|
Created:
4 years, 5 months ago by Mathieu Modified:
4 years, 5 months ago CC:
chromium-reviews, rouslan+autofill_chromium.org, estade+watch_chromium.org, vabr+watchlistautofill_chromium.org, jdonnelly+autofillwatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@aaa Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Autofill] Add support for signin promo on Android
Will open the AccountSigninActivity.
BUG=626383
TEST=manual
Committed: https://crrev.com/664a4cfcc194ee6b0f20069db9bfd116cfe5b32a
Cr-Commit-Position: refs/heads/master@{#406394}
Patch Set 1 : initial #Patch Set 2 : signin metric #
Total comments: 4
Patch Set 3 : addressed comments #Patch Set 4 : fix return type #
Total comments: 2
Patch Set 5 : signin_promo_util #
Total comments: 4
Patch Set 6 : include #Patch Set 7 : another include #
Total comments: 8
Patch Set 8 : addressed comments #
Total comments: 6
Patch Set 9 : addressed comments #
Total comments: 4
Patch Set 10 : nits #Messages
Total messages: 61 (40 generated)
Description was changed from ========== [Autofill] Add support for signin promo on Android Will open the signin manager. BUG=626383 ========== to ========== [Autofill] Add support for signin promo on Android Will open the signin manager. BUG=626383 ==========
mathp@chromium.org changed reviewers: + estade@chromium.org, gogerald@chromium.org
Description was changed from ========== [Autofill] Add support for signin promo on Android Will open the signin manager. BUG=626383 ========== to ========== [Autofill] Add support for signin promo on Android Will open the AccountSigninActivity. BUG=626383 TEST=manual ==========
mathp@chromium.org changed reviewers: + miguelg@chromium.org
Hi, please take a look. Evan: chrome_autofill_client.cc Miguel: chrome/browser/ui/android/ Ganggui: please review signin changes. It feels like the newly added Intent flags should have been there all along, let me know if would prefer a different way. Thanks
https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:58: | Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TOP); It might be better to use FLAG_ACTIVITY_NEW_TASK for native call only. Without using this flag, this activity will be put in the same task of the launching activity, and all the activities in the same task will remain in the same order in the same group ( https://developer.android.com/reference/android/content/Intent.html#FLAG_ACTI...). https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java (right): https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java:563: public static void openAccountSigninActivity(Context context, int accessPoint) { AccountSigninActivity depends on SigninManager to check sign in status and sign in user. It might be better to define a static help method, like startIfAllowed, in AccountSigninActivity called by native directly.
Patchset #1 (id:1) has been deleted
Patchset #3 (id:60001) has been deleted
Please take another look? https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:58: | Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TOP); On 2016/07/11 18:53:43, gogerald1 wrote: > It might be better to use FLAG_ACTIVITY_NEW_TASK for native call only. Without > using this flag, this activity will be put in the same task of the launching > activity, and all the activities in the same task will remain in the same order > in the same group ( > https://developer.android.com/reference/android/content/Intent.html#FLAG_ACTI...). Done. https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java (right): https://codereview.chromium.org/2138643004/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java:563: public static void openAccountSigninActivity(Context context, int accessPoint) { On 2016/07/11 18:53:43, gogerald1 wrote: > AccountSigninActivity depends on SigninManager to check sign in status and sign > in user. It might be better to define a static help method, like startIfAllowed, > in AccountSigninActivity called by native directly. I've added a static method to SigninPromoUtil.java and created the jni headers so that chrome_autofill_client.cc can use it. Let me know if this is what you had in mind.
https://codereview.chromium.org/2138643004/diff/100001/chrome/browser/ui/auto... File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): https://codereview.chromium.org/2138643004/diff/100001/chrome/browser/ui/auto... chrome/browser/ui/autofill/chrome_autofill_client.cc:355: return !signin->AuthInProgress() && signin->IsSigninAllowed() && this looks suspiciously similar to the implementation of ShouldShowPromo
Thanks https://codereview.chromium.org/2138643004/diff/100001/chrome/browser/ui/auto... File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): https://codereview.chromium.org/2138643004/diff/100001/chrome/browser/ui/auto... chrome/browser/ui/autofill/chrome_autofill_client.cc:355: return !signin->AuthInProgress() && signin->IsSigninAllowed() && On 2016/07/12 18:16:22, Evan Stade wrote: > this looks suspiciously similar to the implementation of ShouldShowPromo Unfortunately signing_promo.h/cc is not included on Android/iOS. https://cs.chromium.org/chromium/src/chrome/chrome_browser.gypi?rcl=0&l=1455
On 2016/07/12 18:47:26, Mathieu Perreault wrote: > Thanks > > https://codereview.chromium.org/2138643004/diff/100001/chrome/browser/ui/auto... > File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): > > https://codereview.chromium.org/2138643004/diff/100001/chrome/browser/ui/auto... > chrome/browser/ui/autofill/chrome_autofill_client.cc:355: return > !signin->AuthInProgress() && signin->IsSigninAllowed() && > On 2016/07/12 18:16:22, Evan Stade wrote: > > this looks suspiciously similar to the implementation of ShouldShowPromo > > Unfortunately signing_promo.h/cc is not included on Android/iOS. > > https://cs.chromium.org/chromium/src/chrome/chrome_browser.gypi?rcl=0&l=1455 we should either compile it on android or move the relevant logic to a shared location
Patchset #5 (id:120001) has been deleted
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #5 (id:140001) has been deleted
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mathp@chromium.org changed reviewers: + rogerta@chromium.org
+rogerta for new cross-platform file signin_promo_util. Thanks, I've moved the common ShouldShowPromo to a cross-platform location.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm with nits and a question https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/signin/... File chrome/browser/signin/signin_promo_util.h (right): https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/signin/... chrome/browser/signin/signin_promo_util.h:10: // Cross-platform utility functions for signin promos. nit: not sure this comment is helpful enough to warrant inclusion https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/ui/auto... File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/ui/auto... chrome/browser/ui/autofill/chrome_autofill_client.cc:355: valid_context = !!chrome::FindBrowserWithWebContents(web_contents()); nit: #if defined(OS_IOS) [...] #elif !defined(OS_ANDROID) if (!chrome::FindBrowserWithWebContents(web_contents())) return false; #endif return signin::[...] https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/ui/auto... chrome/browser/ui/autofill/chrome_autofill_client.cc:373: ->ShowAvatarBubbleFromAvatarButton( do we know that there will be an avatar button?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with comments https://codereview.chromium.org/2138643004/diff/140002/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/140002/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:55: Context context, @AccessPoint int accessPoint, boolean useNewTaskFlag) { doc useNewTaskFlag above? https://codereview.chromium.org/2138643004/diff/140002/chrome/browser/android... File chrome/browser/android/signin/signin_promo_util_android.h (right): https://codereview.chromium.org/2138643004/diff/140002/chrome/browser/android... chrome/browser/android/signin/signin_promo_util_android.h:17: static void OpenAccountSigninActivityForPromo( rename OpenAccountSigninActivityForPromo to StartAccountSigninActivityForPromo to be consistent with the name in AccountSigninActivity? Android also names the interface StartActivity,
Thanks all! rogerta, miguelc: please have a look https://codereview.chromium.org/2138643004/diff/140002/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/140002/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:55: Context context, @AccessPoint int accessPoint, boolean useNewTaskFlag) { On 2016/07/15 17:55:42, gogerald1 wrote: > doc useNewTaskFlag above? Done. https://codereview.chromium.org/2138643004/diff/140002/chrome/browser/android... File chrome/browser/android/signin/signin_promo_util_android.h (right): https://codereview.chromium.org/2138643004/diff/140002/chrome/browser/android... chrome/browser/android/signin/signin_promo_util_android.h:17: static void OpenAccountSigninActivityForPromo( On 2016/07/15 17:55:42, gogerald1 wrote: > rename OpenAccountSigninActivityForPromo to StartAccountSigninActivityForPromo > to be consistent with the name in AccountSigninActivity? Android also names the > interface StartActivity, Done. https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/signin/... File chrome/browser/signin/signin_promo_util.h (right): https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/signin/... chrome/browser/signin/signin_promo_util.h:10: // Cross-platform utility functions for signin promos. On 2016/07/15 17:32:22, Evan Stade wrote: > nit: not sure this comment is helpful enough to warrant inclusion Done. https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/ui/auto... File chrome/browser/ui/autofill/chrome_autofill_client.cc (right): https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/ui/auto... chrome/browser/ui/autofill/chrome_autofill_client.cc:355: valid_context = !!chrome::FindBrowserWithWebContents(web_contents()); On 2016/07/15 17:32:22, Evan Stade wrote: > nit: > > #if defined(OS_IOS) > [...] > #elif !defined(OS_ANDROID) > if (!chrome::FindBrowserWithWebContents(web_contents())) > return false; > #endif > > return signin::[...] Done. https://codereview.chromium.org/2138643004/diff/190001/chrome/browser/ui/auto... chrome/browser/ui/autofill/chrome_autofill_client.cc:373: ->ShowAvatarBubbleFromAvatarButton( On 2016/07/15 17:32:22, Evan Stade wrote: > do we know that there will be an avatar button? This was pointed out by the signin team as the method to call because they are transitioning to a new Modal signup and ShowAvatarBubbleFromAvatarButton will know which flow to trigger.
lgtm
mathp@chromium.org changed reviewers: + tedchoc@chromium.org - miguelg@chromium.org
-miguelg (OOO) +tedchoc, please have a look at the chrome/browser/ui/android, chrome/browser/android, chrome/android bits!
https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:53: * @param useNewTaskFlag - Whether to start this Activity in a new task. while consistent in this file, the - between the name and the sentence isn't correct https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:106: == SigninAccessPoint.AUTOFILL_DROPDOWN : "invalid access point: " Hmm...why change the wrapping of the : (i.e. why not have the : on the following line? is that not allowed...I don't recall)? It was ugly before, but this seems....uglier. https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java (right): https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java:55: public static void openAccountSigninActivityForPromo(Context context, int accessPoint) { why public? if it is only called by native, then it should be private. Do you actually want this to be in a new task, or are you doing it because you only have the app context? You have the WebContents on the native side. You could do ContentViewCore::FromWebContents(web_contents())->GetJavaObject(); Better yet, would be to do: CVC::FromWebContents(web_contents())->GetWindowAndroid()->GetJavaObject(); With a WindowAndroid, you can do getActivity().get() and you'd have an activity. In that case, you wouldn't need a new task and you could drop all the booleans everywhere. Granted if you want NEW_TASK explicitly, then ignore that rambling comment.
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #9 (id:230001) has been deleted
The CQ bit was checked by mathp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #9 (id:250001) has been deleted
Thanks Ted, PTAL! https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:53: * @param useNewTaskFlag - Whether to start this Activity in a new task. On 2016/07/19 00:46:04, Ted C wrote: > while consistent in this file, the - between the name and the sentence isn't > correct Acknowledged. https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:106: == SigninAccessPoint.AUTOFILL_DROPDOWN : "invalid access point: " On 2016/07/19 00:46:04, Ted C wrote: > Hmm...why change the wrapping of the : (i.e. why not have the : on the following > line? is that not allowed...I don't recall)? > > It was ugly before, but this seems....uglier. Done. Let's blame `git cl format' https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java (right): https://codereview.chromium.org/2138643004/diff/210001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java:55: public static void openAccountSigninActivityForPromo(Context context, int accessPoint) { On 2016/07/19 00:46:04, Ted C wrote: > why public? if it is only called by native, then it should be private. Do you > actually want this to be in a new task, or are you doing it because you only > have the app context? > > You have the WebContents on the native side. You could do > ContentViewCore::FromWebContents(web_contents())->GetJavaObject(); > > Better yet, would be to do: > CVC::FromWebContents(web_contents())->GetWindowAndroid()->GetJavaObject(); > > With a WindowAndroid, you can do getActivity().get() and you'd have an activity. > In that case, you wouldn't need a new task and you could drop all the booleans > everywhere. > > Granted if you want NEW_TASK explicitly, then ignore that rambling comment. Much cleaner, thanks for the insight!
lgtm w/ a couple small nits that probably could never happen in practice https://codereview.chromium.org/2138643004/diff/270001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java (right): https://codereview.chromium.org/2138643004/diff/270001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java:55: AccountSigninActivity.startIfAllowed(window.getActivity().get(), accessPoint); In theory, the activity can be null. Might want to just get the Activity ref above and if null, return? https://codereview.chromium.org/2138643004/diff/270001/chrome/browser/android... File chrome/browser/android/signin/signin_promo_util_android.cc (right): https://codereview.chromium.org/2138643004/diff/270001/chrome/browser/android... chrome/browser/android/signin/signin_promo_util_android.cc:22: content_view_core->GetWindowAndroid()->GetJavaObject().obj(), While reparenting, the window android can be null. But since this is show via a user action, it "shouldn't" be possible. If we are unsure, I think it would be also fine to null check the window android ref as well.
https://codereview.chromium.org/2138643004/diff/190001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/190001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:58: if (useNewTaskFlag) { another thing if you find yourself doing this in the future. Instead of passing the boolean, you could do something like this: if (WindowAndroid.activityFromContext(context) == null) { intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); } Granted, you still need to pull the context from the window android as the app context will never be true in that case, but it would prevent you from needing the callers to know
The CQ bit was checked by mathp@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gogerald@chromium.org, estade@chromium.org, rogerta@chromium.org, tedchoc@chromium.org Link to the patchset: https://codereview.chromium.org/2138643004/#ps290001 (title: "nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
In the CQ! https://codereview.chromium.org/2138643004/diff/190001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java (right): https://codereview.chromium.org/2138643004/diff/190001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java:58: if (useNewTaskFlag) { On 2016/07/19 20:21:48, Ted C wrote: > another thing if you find yourself doing this in the future. Instead of passing > the boolean, you could do something like this: > > if (WindowAndroid.activityFromContext(context) == null) { > intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK); > } > > Granted, you still need to pull the context from the window android as the app > context will never be true in that case, but it would prevent you from needing > the callers to know Acknowledged. https://codereview.chromium.org/2138643004/diff/270001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java (right): https://codereview.chromium.org/2138643004/diff/270001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/signin/SigninPromoUtil.java:55: AccountSigninActivity.startIfAllowed(window.getActivity().get(), accessPoint); On 2016/07/19 20:09:19, Ted C wrote: > In theory, the activity can be null. > > Might want to just get the Activity ref above and if null, return? Done. https://codereview.chromium.org/2138643004/diff/270001/chrome/browser/android... File chrome/browser/android/signin/signin_promo_util_android.cc (right): https://codereview.chromium.org/2138643004/diff/270001/chrome/browser/android... chrome/browser/android/signin/signin_promo_util_android.cc:22: content_view_core->GetWindowAndroid()->GetJavaObject().obj(), On 2016/07/19 20:09:19, Ted C wrote: > While reparenting, the window android can be null. But since this is show via a > user action, it "shouldn't" be possible. If we are unsure, I think it would be > also fine to null check the window android ref as well. Done.
Message was sent while issue was closed.
Description was changed from ========== [Autofill] Add support for signin promo on Android Will open the AccountSigninActivity. BUG=626383 TEST=manual ========== to ========== [Autofill] Add support for signin promo on Android Will open the AccountSigninActivity. BUG=626383 TEST=manual ==========
Message was sent while issue was closed.
Committed patchset #10 (id:290001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== [Autofill] Add support for signin promo on Android Will open the AccountSigninActivity. BUG=626383 TEST=manual ========== to ========== [Autofill] Add support for signin promo on Android Will open the AccountSigninActivity. BUG=626383 TEST=manual Committed: https://crrev.com/664a4cfcc194ee6b0f20069db9bfd116cfe5b32a Cr-Commit-Position: refs/heads/master@{#406394} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/664a4cfcc194ee6b0f20069db9bfd116cfe5b32a Cr-Commit-Position: refs/heads/master@{#406394} |
