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

Issue 3140008: Refactors mouse watching out of TabStrip into a standalone class. (Closed)

Created:
10 years, 4 months ago by sky
Modified:
10 years, 4 months ago
Reviewers:
Jay Civelli
CC:
chromium-reviews, Paul Godavari, ben+cc_chromium.org
Visibility:
Public.

Description

Refactors mouse watching out of TabStrip into a standalone class. BUG=27797 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56424

Patch Set 1 #

Patch Set 2 : Updated comments #

Total comments: 4

Patch Set 3 : Incorporated review feedback #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+248 lines, -135 lines) Patch
M chrome/browser/views/download_shelf_view.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/views/tabs/tab_strip.h View 1 2 7 chunks +7 lines, -26 lines 0 comments Download
M chrome/browser/views/tabs/tab_strip.cc View 1 2 7 chunks +10 lines, -108 lines 0 comments Download
A views/mouse_watcher.h View 1 chunk +75 lines, -0 lines 1 comment Download
A views/mouse_watcher.cc View 1 chunk +153 lines, -0 lines 0 comments Download
M views/views.gyp View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
sky
10 years, 4 months ago (2010-08-11 18:45:06 UTC) #1
Ben Goodger (Google)
I think this class should actually live in views/ also. http://codereview.chromium.org/3140008/diff/2001/3002 File chrome/browser/views/mouse_watcher.cc (right): http://codereview.chromium.org/3140008/diff/2001/3002#newcode37 ...
10 years, 4 months ago (2010-08-11 19:04:47 UTC) #2
sky
All updated with new snapshot. http://codereview.chromium.org/3140008/diff/2001/3004 File chrome/browser/views/tabs/tab_strip.cc (right): http://codereview.chromium.org/3140008/diff/2001/3004#newcode453 chrome/browser/views/tabs/tab_strip.cc:453: void TabStrip::MouseMovedOutOfView() { On ...
10 years, 4 months ago (2010-08-11 20:24:08 UTC) #3
sky
Jay, could you review this as Ben is out? Thanks, -Scott
10 years, 4 months ago (2010-08-17 17:15:57 UTC) #4
Jay Civelli
10 years, 4 months ago (2010-08-17 17:37:13 UTC) #5
LGTM

http://codereview.chromium.org/3140008/diff/9001/10005
File views/mouse_watcher.h (right):

http://codereview.chromium.org/3140008/diff/9001/10005#newcode53
views/mouse_watcher.h:53: bool is_observing() { return observer_.get() != NULL;
}
Shouldn't this getter be const?

Powered by Google App Engine
This is Rietveld 408576698