Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2512903002:
Add bookmark bar/manager actions to track drag and drop. (Closed)
DescriptionAdd bookmark bar/manager actions to track drag and drop.
This CL adds UMA that will allow analysis of bookmark drag and drop
source and targets.
BUG=665660
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/6e523087878a0b07c4781fa4b793638a3829b075
Cr-Commit-Position: refs/heads/master@{#434253}
Patch Set 1 #
Total comments: 4
Patch Set 2 : own BookmarkBar_DragEnd #
Messages
Total messages: 24 (11 generated)
Description was changed from ========== Add bookmark bar/manager actions to track drag and drop. This CL adds UMA that will allow analysis of bookmark drag and drop source and targets. BUG=665660 ========== to ========== Add bookmark bar/manager actions to track drag and drop. This CL adds UMA that will allow analysis of bookmark drag and drop source and targets. BUG=665660 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
calamity@chromium.org changed reviewers: + asvitkine@chromium.org, tsergeant@chromium.org
https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right): https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:1184: content::RecordAction(base::UserMetricsAction("BookmarkBar_DragEnd")); Are you missing this in histograms.xml?
https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right): https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:1184: content::RecordAction(base::UserMetricsAction("BookmarkBar_DragEnd")); On 2016/11/18 16:52:15, Alexei Svitkine (very slow) wrote: > Are you missing this in histograms.xml? This already exists in actions.xml, but was only implemented on Mac. Adding it here implements it on other desktop platforms.
https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right): https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:1184: content::RecordAction(base::UserMetricsAction("BookmarkBar_DragEnd")); On 2016/11/21 00:49:58, calamity wrote: > On 2016/11/18 16:52:15, Alexei Svitkine (very slow) wrote: > > Are you missing this in histograms.xml? > > This already exists in actions.xml, but was only implemented on Mac. Adding it > here implements it on other desktop platforms. Ah, I see. It does not appear to have an owner or description in actions.xml. If it's interesting to you, please add yourself as owner and provide a description. (Otherwise, if it's not interesting, then I suggest removing it in the Mac code rather than adding it in views.)
lgtm lgtm
On 2016/11/21 22:04:57, tsergeant wrote: > lgtm > > lgtm a patch so nice I lg-ed it twice (oops)
The CQ bit was checked by calamity@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.
https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right): https://codereview.chromium.org/2512903002/diff/1/chrome/browser/ui/views/boo... chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:1184: content::RecordAction(base::UserMetricsAction("BookmarkBar_DragEnd")); On 2016/11/21 15:47:06, Alexei Svitkine (slow) wrote: > On 2016/11/21 00:49:58, calamity wrote: > > On 2016/11/18 16:52:15, Alexei Svitkine (very slow) wrote: > > > Are you missing this in histograms.xml? > > > > This already exists in actions.xml, but was only implemented on Mac. Adding it > > here implements it on other desktop platforms. > > Ah, I see. > > It does not appear to have an owner or description in actions.xml. If it's > interesting to you, please add yourself as owner and provide a description. > > (Otherwise, if it's not interesting, then I suggest removing it in the Mac code > rather than adding it in views.) Done.
lgtm
calamity@chromium.org changed reviewers: + thestig@chromium.org
+thestig for chrome/browser OWNERS.
The CQ bit was checked by thestig@chromium.org
lgtm
The patchset sent to the CQ was uploaded after l-g-t-m from tsergeant@chromium.org Link to the patchset: https://codereview.chromium.org/2512903002/#ps20001 (title: "own BookmarkBar_DragEnd")
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": 20001, "attempt_start_ts": 1479925452437070,
"parent_rev": "6b8f0f332ede3611e9af1877fd7e9de9bd5f3722", "commit_rev":
"be7c7dc2caa165a16e9ee309f4e8244bc8df13de"}
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add bookmark bar/manager actions to track drag and drop. This CL adds UMA that will allow analysis of bookmark drag and drop source and targets. BUG=665660 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Add bookmark bar/manager actions to track drag and drop. This CL adds UMA that will allow analysis of bookmark drag and drop source and targets. BUG=665660 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/6e523087878a0b07c4781fa4b793638a3829b075 Cr-Commit-Position: refs/heads/master@{#434253} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/6e523087878a0b07c4781fa4b793638a3829b075 Cr-Commit-Position: refs/heads/master@{#434253} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
