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

Issue 2245443002: Omnibox: Remove Overly-Aggressive DCHECK (focus must occur before open) (Closed)

Created:
4 years, 4 months ago by Mark P
Modified:
4 years, 4 months ago
Reviewers:
Peter Kasting, rkaplow
CC:
chromium-reviews, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Omnibox: Remove Overly-Aggressive DCHECK (focus must occur before open) And revise a related histogram. BUG=635785 Committed: https://crrev.com/b0afef87f6ef1567a88011c97ac86dc09db75256 Cr-Commit-Position: refs/heads/master@{#411733}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -4 lines) Patch
M components/omnibox/browser/omnibox_edit_model.cc View 2 chunks +7 lines, -4 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +17 lines, -0 lines 2 comments Download

Messages

Total messages: 22 (10 generated)
Mark P
Please take a look, thanks. --mark
4 years, 4 months ago (2016-08-11 21:50:28 UTC) #3
Peter Kasting
LGTM. I worried about a potential case where we do a drag while the omnibox ...
4 years, 4 months ago (2016-08-11 21:58:27 UTC) #4
Mark P
+rkaplow to officially review my histograms.xml change. --mark
4 years, 4 months ago (2016-08-11 22:08:29 UTC) #6
rkaplow
lgtm https://codereview.chromium.org/2245443002/diff/1/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2245443002/diff/1/tools/metrics/histograms/histograms.xml#newcode37115 tools/metrics/histograms/histograms.xml:37115: + histogram to the total number of omnibox ...
4 years, 4 months ago (2016-08-11 22:13:23 UTC) #7
Mark P
https://codereview.chromium.org/2245443002/diff/1/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2245443002/diff/1/tools/metrics/histograms/histograms.xml#newcode37115 tools/metrics/histograms/histograms.xml:37115: + histogram to the total number of omnibox events. ...
4 years, 4 months ago (2016-08-11 22:17:58 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2245443002/1
4 years, 4 months ago (2016-08-11 22:53:05 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/259827)
4 years, 4 months ago (2016-08-11 23:38:42 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2245443002/1
4 years, 4 months ago (2016-08-12 03:54:50 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/260052)
4 years, 4 months ago (2016-08-12 04:57:49 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2245443002/1
4 years, 4 months ago (2016-08-12 19:04:20 UTC) #18
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-12 19:33:54 UTC) #20
commit-bot: I haz the power
4 years, 4 months ago (2016-08-12 19:35:32 UTC) #22
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/b0afef87f6ef1567a88011c97ac86dc09db75256
Cr-Commit-Position: refs/heads/master@{#411733}

Powered by Google App Engine
This is Rietveld 408576698