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

Issue 42388: Enable non-recursive watches for Windows DirectoryWatcher.... (Closed)

Created:
11 years, 9 months ago by Paweł Hajdan Jr.
Modified:
9 years, 7 months ago
Reviewers:
M-A Ruel
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Enable non-recursive watches for Windows DirectoryWatcher. - enable test for it on Windows - clean up directory_watcher_unittest.cc http://crbug.com/5072 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12358

Patch Set 1 #

Total comments: 1

Patch Set 2 : limit ConsumeExtraNotifications usage to Vista and above #

Patch Set 3 : not only Vista, longer timeout #

Patch Set 4 : another attempt #

Patch Set 5 : much better approach #

Total comments: 1

Patch Set 6 : document behavior on Windows #

Patch Set 7 : fix the typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -57 lines) Patch
M base/directory_watcher.h View 1 chunk +4 lines, -0 lines 0 comments Download
M base/directory_watcher_unittest.cc View 1 2 3 4 5 6 9 chunks +37 lines, -50 lines 0 comments Download
M base/directory_watcher_win.cc View 1 1 chunk +1 line, -7 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Paweł Hajdan Jr.
I tried to fix this issue two times, now it's the third. Tested on Windows ...
11 years, 9 months ago (2009-03-19 12:03:41 UTC) #1
M-A Ruel
http://codereview.chromium.org/42388/diff/1/3 File base/directory_watcher_unittest.cc (right): http://codereview.chromium.org/42388/diff/1/3#newcode99 Line 99: void ConsumeExtraNotifications() { Oh I don't like that... ...
11 years, 9 months ago (2009-03-19 13:43:52 UTC) #2
Paweł Hajdan Jr.
On 2009/03/19 13:43:52, M-A wrote: > http://codereview.chromium.org/42388/diff/1/3 > File base/directory_watcher_unittest.cc (right): > > http://codereview.chromium.org/42388/diff/1/3#newcode99 > ...
11 years, 9 months ago (2009-03-19 18:41:35 UTC) #3
M-A Ruel
lhtm
11 years, 9 months ago (2009-03-19 18:50:42 UTC) #4
M-A Ruel
lgtm
11 years, 9 months ago (2009-03-19 19:04:12 UTC) #5
Paweł Hajdan Jr.
On 2009/03/19 18:41:35, Paweł Hajdan Jr. wrote: > > but AFAIK it is only necessary ...
11 years, 9 months ago (2009-03-20 16:44:14 UTC) #6
M-A Ruel
On 2009/03/20 16:44:14, Paweł Hajdan Jr. wrote: > On 2009/03/19 18:41:35, Paweł Hajdan Jr. wrote: ...
11 years, 9 months ago (2009-03-20 17:05:47 UTC) #7
Paweł Hajdan Jr.
So the code only sleeps on Windows now, but I had to increase the timeout ...
11 years, 9 months ago (2009-03-21 19:19:51 UTC) #8
Paweł Hajdan Jr.
I sent this changeset to a trybot once again, just to be sure. Guess what? ...
11 years, 9 months ago (2009-03-23 20:15:50 UTC) #9
M-A Ruel
On 2009/03/23 20:15:50, Paweł Hajdan Jr. wrote: > I sent this changeset to a trybot ...
11 years, 9 months ago (2009-03-23 20:28:56 UTC) #10
Paweł Hajdan Jr.
After our recent talk I changed the approach (trying to consume a fixed number of ...
11 years, 9 months ago (2009-03-24 14:57:17 UTC) #11
M-A Ruel
lgtm with the change in directory_watcher.h http://codereview.chromium.org/42388/diff/4002/5003 File base/directory_watcher_unittest.cc (right): http://codereview.chromium.org/42388/diff/4002/5003#newcode177 Line 177: // Create ...
11 years, 9 months ago (2009-03-24 15:13:54 UTC) #12
M-A Ruel
11 years, 9 months ago (2009-03-24 15:25:19 UTC) #13
lgtm

Powered by Google App Engine
This is Rietveld 408576698