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

Issue 656033007: Minor cleanup in BookmarkBarView (Closed)

Created:
6 years, 2 months ago by sky
Modified:
6 years, 2 months ago
Reviewers:
msw
CC:
chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Minor cleanup in BookmarkBarView Nukes LayoutItems and folds into Layout as that is the only place calling into it. Removes early out if parent() is NULL in Layout. No point in doing that. BUG=416641 TEST=none R=msw@chromium.org Committed: https://crrev.com/7ec19e142e70a0085514b972cd9c4d9ce4460c16 Cr-Commit-Position: refs/heads/master@{#299920}

Patch Set 1 #

Total comments: 1

Patch Set 2 : fix test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -127 lines) Patch
M chrome/browser/ui/views/bookmarks/bookmark_bar_view.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc View 2 chunks +103 lines, -110 lines 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc View 1 1 chunk +5 lines, -14 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
sky
https://codereview.chromium.org/656033007/diff/1/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc File chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc (right): https://codereview.chromium.org/656033007/diff/1/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc#newcode786 chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc:786: int x = kLeftMargin; The only difference from old ...
6 years, 2 months ago (2014-10-15 16:10:41 UTC) #1
msw
lgtm
6 years, 2 months ago (2014-10-15 17:54:32 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/656033007/1
6 years, 2 months ago (2014-10-15 18:00:38 UTC) #4
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_swarming/builds/24232) linux_chromium_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_swarming/builds/25882)
6 years, 2 months ago (2014-10-15 19:05:09 UTC) #6
sky
I had to update test code as it was working around the old behavior of ...
6 years, 2 months ago (2014-10-16 17:00:26 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/656033007/20001
6 years, 2 months ago (2014-10-16 17:02:24 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
6 years, 2 months ago (2014-10-16 17:51:09 UTC) #10
commit-bot: I haz the power
6 years, 2 months ago (2014-10-16 17:51:48 UTC) #11
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/7ec19e142e70a0085514b972cd9c4d9ce4460c16
Cr-Commit-Position: refs/heads/master@{#299920}

Powered by Google App Engine
This is Rietveld 408576698