|
|
DescriptionImplementing sign-in promo histograms for bookmark
Histograms implemented:
- Number of times the sign-in promo has been displayed when the bookmark view
is closed without user actions on the sign-in promo view.
- Number of times the sign-in promo has been displayed when the user taps on it
to sign-in.
- Number of times the sign-in promo has been displayed when the user taps on
the X button to close it.
Also the sign-in promo view is automatically hidden when after being displayed
20 times.
BUG=709286
Review-Url: https://codereview.chromium.org/2942923002
Cr-Commit-Position: refs/heads/master@{#482994}
Committed: https://chromium.googlesource.com/chromium/src/+/a24738a4699277f620a0ed12a183fa475be03580
Patch Set 1 #
Total comments: 16
Patch Set 2 : . #
Total comments: 4
Patch Set 3 : Adding spaces in comments #Patch Set 4 : A little cleanup #
Total comments: 5
Patch Set 5 : Inline histograms #Patch Set 6 : Fixing dependencies & tests #Patch Set 7 : Merge #
Total comments: 9
Patch Set 8 : Renaming histograms #Dependent Patchsets: Messages
Total messages: 72 (56 generated)
Description was changed from ========== Implementing sign-in promo count for bookmarks BUG= ========== to ========== Implementing sign-in promo count for bookmarks BUG=709286 ==========
Patchset #4 (id:60001) has been deleted
Patchset #3 (id:40001) has been deleted
Patchset #3 (id:80001) has been deleted
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Implementing sign-in promo count for bookmarks BUG=709286 ========== to ========== Implementing sign-in promo histograms for bookmark Histograms implemented: - Number of times the sign-in promo has been displayed when the bookmark view is closed without user actions on the sign-in promo view. - Number of times the sign-in promo has been displayed when the user taps on it to sign-in. - Number of times the sign-in promo has been displayed when the user taps on the X button to close it. Also the sign-in promo view is automatically hidden when after being displayed 20 times. BUG=709286 ==========
jlebel@chromium.org changed reviewers: + sdefresne@chromium.org
Hello Sylvain, Can you review this patch? Thanks,
Patchset #1 (id:100001) has been deleted
https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:46: @property(nonatomic) std::string countTilSigninHistogramName; Shouldn't this be "countTillSigninHistogramName"? https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:49: @property(nonatomic) std::string countTilXHistogramName; Shouldn't this be "countTillXHistogramName"? https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm:83: // The histogram used to record the number of time was seen before the bookmark Why are those constants here and not in signin_promo_view_mediator.mm? Passing them via std::string property causes lots of allocations and copying for no benefits as far as I can see. If you really need the values to be properties, maybe have them of type "const char*" with a comment that they have to point to static storage. https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm:89: const char kBookmarksCountTilSignin[] = Shouldn't this be "kBookmarksCountTillSignin"? https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm:93: const char kBookmarkCountTilX[] = "MobileSignInPromo.BookmarkManager.CountTilX"; Shouldn't this be "kBookmarkCountTillX"? https://codereview.chromium.org/2942923002/diff/120001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32303: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin"> ditto Til -> Till or maybe Until? https://codereview.chromium.org/2942923002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32313: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilX"> ditto
https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:46: @property(nonatomic) std::string countTilSigninHistogramName; On 2017/06/19 09:07:50, sdefresne wrote: > Shouldn't this be "countTillSigninHistogramName"? According to google, both works (it looks like "till" is more format). But "til" has been asked by my american PM. https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:49: @property(nonatomic) std::string countTilXHistogramName; On 2017/06/19 09:07:50, sdefresne wrote: > Shouldn't this be "countTillXHistogramName"? See above. https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm:83: // The histogram used to record the number of time was seen before the bookmark On 2017/06/19 09:07:50, sdefresne wrote: > Why are those constants here and not in signin_promo_view_mediator.mm? Passing > them via std::string property causes lots of allocations and copying for no > benefits as far as I can see. If you really need the values to be properties, > maybe have them of type "const char*" with a comment that they have to point to > static storage. The sign-in promo mediator is used also for in "other devices". Those histograms should not be updated anywhere else. I'm using those properties to activate the histograms only for bookmark. The same histograms will be implemented soon for settings view. Settings view doesn't use yet the mediator (for historical reasons). But it should, and it will. So with those properties, both views will have their own histograms. https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm:89: const char kBookmarksCountTilSignin[] = On 2017/06/19 09:07:50, sdefresne wrote: > Shouldn't this be "kBookmarksCountTillSignin"? See signin_promo_view_mediator.h https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/bookmarks/bookmark_collection_view.mm:93: const char kBookmarkCountTilX[] = "MobileSignInPromo.BookmarkManager.CountTilX"; On 2017/06/19 09:07:50, sdefresne wrote: > Shouldn't this be "kBookmarkCountTillX"? See signin_promo_view_mediator.h https://codereview.chromium.org/2942923002/diff/120001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32303: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin"> On 2017/06/19 09:07:50, sdefresne wrote: > ditto Til -> Till or maybe Until? See signin_promo_view_mediator.h https://codereview.chromium.org/2942923002/diff/120001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32313: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilX"> On 2017/06/19 09:07:50, sdefresne wrote: > ditto See signin_promo_view_mediator.h
lgtm lgtm https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:46: @property(nonatomic) std::string countTilSigninHistogramName; On 2017/06/19 14:38:12, jlebel wrote: > On 2017/06/19 09:07:50, sdefresne wrote: > > Shouldn't this be "countTillSigninHistogramName"? > > According to google, both works (it looks like "till" is more format). But "til" > has been asked by my american PM. Ack. Didn't knew it was an US vs UK english difference. https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:47: // clicked per impression.The value should point to static storage. nit: space after '.' https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:50: // clicked per impression.The value should point to static storage. nit: space after '.'
lgtm https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:46: @property(nonatomic) std::string countTilSigninHistogramName; On 2017/06/19 14:38:12, jlebel wrote: > On 2017/06/19 09:07:50, sdefresne wrote: > > Shouldn't this be "countTillSigninHistogramName"? > > According to google, both works (it looks like "till" is more format). But "til" > has been asked by my american PM. Ack. Didn't knew it was an US vs UK english difference. https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:47: // clicked per impression.The value should point to static storage. nit: space after '.' https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:50: // clicked per impression.The value should point to static storage. nit: space after '.'
Thanks https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/120001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:46: @property(nonatomic) std::string countTilSigninHistogramName; On 2017/06/22 12:40:38, sdefresne wrote: > On 2017/06/19 14:38:12, jlebel wrote: > > On 2017/06/19 09:07:50, sdefresne wrote: > > > Shouldn't this be "countTillSigninHistogramName"? > > > > According to google, both works (it looks like "till" is more format). But > "til" > > has been asked by my american PM. > > Ack. Didn't knew it was an US vs UK english difference. Acknowledged. https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h (right): https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:47: // clicked per impression.The value should point to static storage. On 2017/06/22 12:40:39, sdefresne wrote: > nit: space after '.' Done. https://codereview.chromium.org/2942923002/diff/140001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h:50: // clicked per impression.The value should point to static storage. On 2017/06/22 12:40:38, sdefresne wrote: > nit: space after '.' Done.
jlebel@chromium.org changed reviewers: + asvitkine@chromium.org
Hello Alexei, Can you review my patch to add histograms? Thanks,
Sorry Alexei, I didn't "very slow". I'm going to send my patch to someone else.
jlebel@chromium.org changed reviewers: + rkaplow@chromium.org - asvitkine@chromium.org
Hello Robert, Can you review my patch to add new histograms to Chrome iOS? Thanks,
sorry for us being slow! https://codereview.chromium.org/2942923002/diff/180001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.mm (right): https://codereview.chromium.org/2942923002/diff/180001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.mm:157: UMA_HISTOGRAM_COUNTS_100(_dismissalCountHistogramName, displayedCount); people generally inline the names of the histograms in the macro since it's required to be constant anyway https://codereview.chromium.org/2942923002/diff/180001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/180001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32303: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin"> I would mention what this is counting - something like ButtonClicksUntilSignin ? https://codereview.chromium.org/2942923002/diff/180001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32303: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin"> units required for all histograms
Patchset #5 (id:200001) has been deleted
Thanks for your help. PS: I don't call that slow :) Thanks for your reactivity. https://codereview.chromium.org/2942923002/diff/180001/ios/chrome/browser/ui/... File ios/chrome/browser/ui/authentication/signin_promo_view_mediator.mm (right): https://codereview.chromium.org/2942923002/diff/180001/ios/chrome/browser/ui/... ios/chrome/browser/ui/authentication/signin_promo_view_mediator.mm:157: UMA_HISTOGRAM_COUNTS_100(_dismissalCountHistogramName, displayedCount); On 2017/06/23 19:33:50, rkaplow wrote: > people generally inline the names of the histograms in the macro since it's > required to be constant anyway Done. https://codereview.chromium.org/2942923002/diff/180001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/180001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32303: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin"> On 2017/06/23 19:33:50, rkaplow wrote: > I would mention what this is counting - something like > ButtonClicksUntilSignin > ? > Does it work like that? Sorry, I'm new in histograms.
The CQ bit was checked by jlebel@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: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by jlebel@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: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
The CQ bit was checked by jlebel@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: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by jlebel@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...
Patchset #7 (id:260001) has been deleted
Patchset #6 (id:240001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
The CQ bit was checked by jlebel@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.
The CQ bit was checked by jlebel@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 #7 (id:300001) has been deleted
The CQ bit was checked by jlebel@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.
lgtm mostly LG - still some suggestions related to the name of the histograms. LMK if you want me to re-review https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32493: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin" I'd prefer a more specific name, i.e. Clicks versus 'Count'. Also looking again - why is this 'til'? I don't see any temporal restriction here (based on the description). Could this be clearer as SigninClicks? https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32494: + units="display count until sign-in"> Usuallu we keep the unit short (something like 'clicks'). https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32504: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilX" similar - XButtonClicks or something? That seems much clearer to me https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32514: + units="display count until bookmark closed"> dismissals as unit I would suggest
Patchset #8 (id:340001) has been deleted
Thanks for your inputs. https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32493: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin" On 2017/06/27 21:13:57, rkaplow wrote: > I'd prefer a more specific name, i.e. Clicks versus 'Count'. > > Also looking again - why is this 'til'? I don't see any temporal restriction > here (based on the description). > > Could this be clearer as SigninClicks? The bucket is number of time the sign-in promo has been displayed when the user clicks on the sign-in buttons. If it's ok with you I change it to: MobileSignInPromo.BookmarkManager.ImpressionsTilSigninButton https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32494: + units="display count until sign-in"> On 2017/06/27 21:13:57, rkaplow wrote: > Usuallu we keep the unit short (something like 'clicks'). Done. https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32504: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilX" On 2017/06/27 21:13:57, rkaplow wrote: > similar - XButtonClicks or something? That seems much clearer to me I choose: MobileSignInPromo.BookmarkManager.ImpressionsTilXButton https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32514: + units="display count until bookmark closed"> On 2017/06/27 21:13:57, rkaplow wrote: > dismissals as unit I would suggest Done.
The CQ bit was checked by jlebel@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...
Patchset #8 (id:360001) has been deleted
The CQ bit was checked by jlebel@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...
https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2942923002/diff/320001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:32493: +<histogram name="MobileSignInPromo.BookmarkManager.CountTilSignin" On 2017/06/28 11:10:31, jlebel wrote: > On 2017/06/27 21:13:57, rkaplow wrote: > > I'd prefer a more specific name, i.e. Clicks versus 'Count'. > > > > Also looking again - why is this 'til'? I don't see any temporal restriction > > here (based on the description). > > > > Could this be clearer as SigninClicks? > > The bucket is number of time the sign-in promo has been displayed when the user > clicks on the sign-in buttons. If it's ok with you I change it to: > MobileSignInPromo.BookmarkManager.ImpressionsTilSigninButton I meant MobileSignInPromo.BookmarkManager.ImpressionsTilSigninButtons since we have more than one sign-in button.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jlebel@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sdefresne@chromium.org, rkaplow@chromium.org Link to the patchset: https://codereview.chromium.org/2942923002/#ps380001 (title: "Renaming histograms")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 380001, "attempt_start_ts": 1498663277649200, "parent_rev": "982b113bfbb230977334534a2a9c8a8a633747cd", "commit_rev": "443cf9c785bdab90c62e58b2f5d2c64b37546f66"}
CQ is committing da patch. Bot data: {"patchset_id": 380001, "attempt_start_ts": 1498663277649200, "parent_rev": "7d5b74f1738443fff936e2d0e5815a26a5aa7b2f", "commit_rev": "a24738a4699277f620a0ed12a183fa475be03580"}
Message was sent while issue was closed.
Description was changed from ========== Implementing sign-in promo histograms for bookmark Histograms implemented: - Number of times the sign-in promo has been displayed when the bookmark view is closed without user actions on the sign-in promo view. - Number of times the sign-in promo has been displayed when the user taps on it to sign-in. - Number of times the sign-in promo has been displayed when the user taps on the X button to close it. Also the sign-in promo view is automatically hidden when after being displayed 20 times. BUG=709286 ========== to ========== Implementing sign-in promo histograms for bookmark Histograms implemented: - Number of times the sign-in promo has been displayed when the bookmark view is closed without user actions on the sign-in promo view. - Number of times the sign-in promo has been displayed when the user taps on it to sign-in. - Number of times the sign-in promo has been displayed when the user taps on the X button to close it. Also the sign-in promo view is automatically hidden when after being displayed 20 times. BUG=709286 Review-Url: https://codereview.chromium.org/2942923002 Cr-Commit-Position: refs/heads/master@{#482994} Committed: https://chromium.googlesource.com/chromium/src/+/a24738a4699277f620a0ed12a183... ==========
Message was sent while issue was closed.
Committed patchset #8 (id:380001) as https://chromium.googlesource.com/chromium/src/+/a24738a4699277f620a0ed12a183... |