Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
287017:
Disable system suspend while downloading files on win32.... (Closed)
|
Created:
11 years, 2 months ago by bdonlan Modified:
9 years, 7 months ago CC:
chromium-reviews_googlegroups.com, John Grabowski, Paul Godavari, pam+watch_chromium.org, ben+cc_chromium.org Base URL:
http://src.chromium.org/svn/trunk/src/ Visibility:
Public. |
DescriptionDisable system suspend while downloading files on win32.
If the system goes into power-save sleep mode while downloading files,
the download fails. So, prevent sleep mode until the download finishes.
This patch introduces a new RAII class to request that the system's
power-save mode be disabled - PowerSaveBlocker.
This is only implemented for win32; other platforms are stubbed out.
This only partially implements bug 25420 it only attempts to handle the
downloading case.
Patch by Bryan Donlan <bdonlan@gmail.com>;
BUG=25420
TEST=Download a large file with the system sleep timeout set to a short interval.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=29873
Patch Set 1 #Patch Set 2 : '' #
Total comments: 2
Patch Set 3 : '' #Patch Set 4 : '' #Patch Set 5 : '' #Patch Set 6 : '' #
Total comments: 28
Patch Set 7 : '' #
Total comments: 15
Patch Set 8 : '' #Patch Set 9 : '' #Patch Set 10 : 'Fix #
Messages
Total messages: 37 (0 generated)
http://codereview.chromium.org/287017/diff/3001/15 File chrome/common/platform_util_win.cc (right): http://codereview.chromium.org/287017/diff/3001/15#newcode177 Line 177: SetThreadExecutionState(ES_CONTINUOUS | ES_SYSTEM_REQUIRED); If the thread that called SetThreadExecutionStat(ES_CONTINUOUS | ES_SYSTEM_REQUIRED) dies before calling EnableSleep(), does that mean the system will never automatically sleep?
http://codereview.chromium.org/287017/diff/3001/15 File chrome/common/platform_util_win.cc (right): http://codereview.chromium.org/287017/diff/3001/15#newcode177 Line 177: SetThreadExecutionState(ES_CONTINUOUS | ES_SYSTEM_REQUIRED); On 2009/10/19 18:06:51, Lei Zhang wrote: > If the thread that called SetThreadExecutionStat(ES_CONTINUOUS | > ES_SYSTEM_REQUIRED) dies before calling EnableSleep(), does that mean the system > will never automatically sleep? By my reading SetThreadExecutionState(ES_CONTINUOUS) applies only to the current thread - and in fact, on windows 7 at least that is the behavior I see (test program at http://chromium.pastebin.com/f36c4b215). Note that the screen blank timeout is independent from the system sleep timeout (and is not affected either by this patch or the above test program), so if you run that above program be aware that the screen may blank well before the suspend timer is re-enabled and triggers. So if the thread dies before calling EnableSleep, the call to SetThreadExecutionState is automatically cancelled, and sleep will be re-enabled (provided no other threads on the system are disabling it). There may be a case for putting a DCHECK in the TLS destructor though; if that case actually happens it's probably indicative of another bug elsewhere.
I am the wrong reviewer. Trying a couple other dudes.
On 2009/10/19 19:03:56, Evan Stade wrote: > I am the wrong reviewer. Trying a couple other dudes. Sorry about that - The IRC channel was dead at the time, so I looked at the blame for platform_util.h and download_file.cc and saw you there in a few places ... In retrospect I should've cross-checked your history here as well, but I'm a bit new at this :)
I am looking into this now. I like your approach but the use of TLS feels funny. Let me consider this for a few hours.
On 2009/10/19 20:58:48, cpu wrote: > I am looking into this now. > > I like your approach but the use of TLS feels funny. Let me consider this for a > few hours. Hi, The use of TLS came about after I considered and rejected a few other designs: * Periodic polling - a single function that would reset the sleep timer, called periodically from the download code. This works fine on win32, but I suspect Linux might have problems with this, as I believe the dbus suspend-inhibit stuff is more of a register-your-process approach. * Process-wide counter - this requires on win32 for a single thread to manage the SetThreadExecutionState. However this introduces additional complexity, and since this is going in platform_util in chrome/common, I didn't know if I could use chrome_thread in chrome/browser. So I ended up going with TLS. That said, I'm beginning to wonder whether platform_util is the right place to put it - on Linux with dbus, won't it need access to chrome_thread to give whatever thread deals with dbus a kick? Or would it be good enough to give each thread its own connection to dbus? And if platform_util isn't a good place for this, what is...?
Thanks for your patience.
I can see you have considered several approaches, I also spent some time today
how could it be hooked more cleanly.
What bothers me is that reminds me of other patterns in windows, such as
ShowCursor()
The OS already has a counter for the action.
What I propose is a simple RAII class
class AGoodName {
AGoodName() {
SetThreadExecutionState(..)
}
~AGoodName() {
SetThreadExecutionState(..)
}
}
Attach this as a member of DownloadFile and off we go.
It might need a method to trigger instead of doing the call at ctor.
On 2009/10/20 02:53:41, cpu wrote:
> Thanks for your patience.
>
> I can see you have considered several approaches, I also spent some time today
> how could it be hooked more cleanly.
>
> What bothers me is that reminds me of other patterns in windows, such as
> ShowCursor()
>
> The OS already has a counter for the action.
>
> What I propose is a simple RAII class
>
> class AGoodName {
> AGoodName() {
> SetThreadExecutionState(..)
> }
>
> ~AGoodName() {
> SetThreadExecutionState(..)
> }
>
> }
>
> Attach this as a member of DownloadFile and off we go.
>
> It might need a method to trigger instead of doing the call at ctor.
Okay. I'll have to have the main UI thread take care of the actual call to
SetThreadExecutionState then, I suppose. I'll try to spin a patch tomorrow or
wednesday and update this codereview entry when it's done.
Drive-by question... Will this interfere with the user's ability to manually put a laptop in standby/sleep? On Mon, Oct 19, 2009 at 19:53, <cpu@chromium.org> wrote: > > Thanks for your patience. > > I can see you have considered several approaches, I also spent some time > today > how could it be hooked more cleanly. > > What bothers me is that reminds me of other patterns in windows, such as > ShowCursor() > > The OS already has a counter for the action. > > What I propose is a simple RAII class > > class AGoodName { > AGoodName() { > SetThreadExecutionState(..) > } > > ~AGoodName() { > SetThreadExecutionState(..) > } > > } > > Attach this as a member of DownloadFile and off we go. > > It might need a method to trigger instead of doing the call at ctor. > > > > http://codereview.chromium.org/287017 >
On 2009/10/20 02:59:54, Finnur wrote: > Drive-by question... > Will this interfere with the user's ability to manually put a laptop in > standby/sleep? No. Microsoft does not even expose an API to do so :) This only affects the automatic inactivity timers - it's very annoying to have a long download fail just because your laptop went to sleep after 15 minutes after all! It also does not affect screen blanking timeouts - just actual CPU suspend (although there's another unrelated bug that prevents screen blanking while a chrome window is selected...)
On 2009/10/20 02:53:41, cpu wrote: > Thanks for your patience. > > I can see you have considered several approaches, I also spent some time today > how could it be hooked more cleanly. > > What bothers me is that reminds me of other patterns in windows, such as > ShowCursor() > > The OS already has a counter for the action. > > What I propose is a simple RAII class Patch updated per cpu's suggestions. I'm not entirely sure I've added the new source files correctly for !windows, so a build test on linux or mac would be appreciated.
And updated again to fix lint errors. Just now found out about 'gcl lint' :) http://codereview.chromium.org/287017/diff/17001/18005 File chrome/browser/power_save_blocker.h (right): http://codereview.chromium.org/287017/diff/17001/18005#newcode31 Line 31: #endif // CHROME_BROWSER_POWER_SAVE_BLOCKER_H_ The codereview site lint reports a missing newline here, but it's present on my local copy - it seems to have been lost somewhere :/
Ok after more consideration I agree to the post to the UI thread approach. The only issue remaining in my mind is the need for the counter. I don't see the need for it. Can you explain? Other than that there are various issues of code style, as I hope you become a regular contributor I would like to give a look at http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml Sorry for the slowness.
On 2009/10/20 18:36:23, cpu wrote: > Ok after more consideration I agree to the post to the UI thread approach. > > The only issue remaining in my mind is the need for the counter. I don't see the > need for it. Can you explain? If two instances of PowerSaveBlocker exist at the same time (which will happen with two concurrent downloads), we don't want sleep to be re-enabled when just one of them completes. So there needs to be a counter to track how many remain in existence. > > Other than that there are various issues of code style, as I hope you become a > regular contributor I would like to give a look at > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml Ok, I'll take a look at that and submit a cleaned-up version.
"The system maintains a count of applications that have called SetThreadExecutionState. The system tracks each thread that calls SetThreadExecutionState and adjusts the counter accordingly. If this counter reaches zero and there has not been any user input, the system enters sleep." http://msdn.microsoft.com/en-us/library/aa373233(VS.85).aspx
On 2009/10/20 21:57:44, cpu wrote: > "The system maintains a count of applications that have called > SetThreadExecutionState. The system tracks each thread that calls > SetThreadExecutionState and adjusts the counter accordingly. If this counter > reaches zero and there has not been any user input, the system enters sleep." Yes, but a single thread only contributes one to that count. ES_CONTINUOUS just sets a persistent state - calling again resets that state. It doesn't add to the counter.
Nice job. You should familiarize yourself with our style guide when you have a chance. http://codereview.chromium.org/287017/diff/17001/18005 File chrome/browser/power_save_blocker.h (right): http://codereview.chromium.org/287017/diff/17001/18005#newcode12 Line 12: bool enabled_; Methods before data. http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar... http://codereview.chromium.org/287017/diff/17001/18005#newcode16 Line 16: static void apply_block(bool blocked); MethodsAreNamedThisWay() and_not_this_way(). http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi... http://codereview.chromium.org/287017/diff/17001/18005#newcode21 Line 21: static int blocker_count; Static data members still count as data members. blocker_count_. http://codereview.chromium.org/287017/diff/17001/18005#newcode22 Line 22: public: Public before private. http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar... http://codereview.chromium.org/287017/diff/17001/18005#newcode23 Line 23: explicit PowerSaveBlocker(bool enabled = false); Default arguments forbidden. http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Defaul... http://codereview.chromium.org/287017/diff/17001/18005#newcode24 Line 24: ~PowerSaveBlocker(void); void doesn't add anything. http://codereview.chromium.org/287017/diff/17001/18005#newcode26 Line 26: bool enabled() const { return enabled_; } This is allowed to be called enabled() because it's a trivial inline accessor, but set_enabled is not inline and not trivial and therefore shouldn't be called set_enabled. http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi... http://codereview.chromium.org/287017/diff/17001/18005#newcode29 Line 29: }; DISALLOW_COPY_AND_ASSIGN? http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Copy_C... http://codereview.chromium.org/287017/diff/17001/18005#newcode31 Line 31: #endif // CHROME_BROWSER_POWER_SAVE_BLOCKER_H_ bdonlan wrote: > The codereview site lint reports a missing newline here, but it's present on my > local copy - it seems to have been lost somewhere :/ codereview lint is broken in this way, just ignore it. http://codereview.chromium.org/287017/diff/17001/18003 File chrome/browser/power_save_blocker_common.cc (right): http://codereview.chromium.org/287017/diff/17001/18003#newcode12 Line 12: enabled_ = false; For constructors, initializer lists are preferred. http://codereview.chromium.org/287017/diff/17001/18003#newcode33 Line 33: NewRunnableFunction(&PowerSaveBlocker::adjust, delta)); 4-space indent (not 2) or lining up with something that makes sense from the preceding line is generally preferred. http://codereview.chromium.org/287017/diff/17001/18002 File chrome/browser/power_save_blocker_stub.cc (right): http://codereview.chromium.org/287017/diff/17001/18002#newcode10 Line 10: // STUB NOTIMPLEMENTED(); ? (from base/logging.h) http://codereview.chromium.org/287017/diff/17001/18004 File chrome/browser/power_save_blocker_win.cc (right): http://codereview.chromium.org/287017/diff/17001/18004#newcode11 Line 11: static int sleep_blocker_count = 0; Unused. http://codereview.chromium.org/287017/diff/17001/18007 File chrome/chrome.gyp (right): http://codereview.chromium.org/287017/diff/17001/18007#newcode1840 Line 1840: 'browser/power_save_blocker_stub.cc', Put power_save_blocker_win.cc here too. _win.cc files are automatically excluded in non-Windows builds.
Are bugs filed for the Mac and Linux implementation?
On 2009/10/21 15:39:58, pink wrote: > Are bugs filed for the Mac and Linux implementation? Actually issue 18895 is for Linux. This patch is for Windows - issue 25420 Mac - issue 25419
Here's an updated patch - hopefully the style's a bit better now :) @cpu - "ES_CONTINUOUS - Informs the system that the state being set should remain in effect until the next call that uses ES_CONTINUOUS and one of the other state flags is cleared." Note that it refers to a 'state', not a 'counter' here... This behavior can be confirmed with this simple test program: http://chromium.pastebin.com/f7be7a7bc (tested on windows 7) @Lei - IMO those ought to be split up a bit more, into download suppression of PM and video suppression of both PM and screensaver.
http://codereview.chromium.org/287017/diff/17001/18005 File chrome/browser/power_save_blocker.h (right): http://codereview.chromium.org/287017/diff/17001/18005#newcode12 Line 12: bool enabled_; On 2009/10/21 04:49:16, Mark Mentovai wrote: > Methods before data. > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar... Done. http://codereview.chromium.org/287017/diff/17001/18005#newcode16 Line 16: static void apply_block(bool blocked); On 2009/10/21 04:49:16, Mark Mentovai wrote: > MethodsAreNamedThisWay() and_not_this_way(). > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi... Done. http://codereview.chromium.org/287017/diff/17001/18005#newcode21 Line 21: static int blocker_count; On 2009/10/21 04:49:16, Mark Mentovai wrote: > Static data members still count as data members. blocker_count_. Done. http://codereview.chromium.org/287017/diff/17001/18005#newcode22 Line 22: public: On 2009/10/21 04:49:16, Mark Mentovai wrote: > Public before private. > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Declar... Done. http://codereview.chromium.org/287017/diff/17001/18005#newcode23 Line 23: explicit PowerSaveBlocker(bool enabled = false); On 2009/10/21 04:49:16, Mark Mentovai wrote: > Default arguments forbidden. > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Defaul... Done. http://codereview.chromium.org/287017/diff/17001/18005#newcode24 Line 24: ~PowerSaveBlocker(void); On 2009/10/21 04:49:16, Mark Mentovai wrote: > void doesn't add anything. Done. http://codereview.chromium.org/287017/diff/17001/18005#newcode26 Line 26: bool enabled() const { return enabled_; } On 2009/10/21 04:49:16, Mark Mentovai wrote: > This is allowed to be called enabled() because it's a trivial inline accessor, > but set_enabled is not inline and not trivial and therefore shouldn't be called > set_enabled. > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi... Replaced with an Enable() and Disable() http://codereview.chromium.org/287017/diff/17001/18005#newcode29 Line 29: }; On 2009/10/21 04:49:16, Mark Mentovai wrote: > DISALLOW_COPY_AND_ASSIGN? > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Copy_C... Done. http://codereview.chromium.org/287017/diff/17001/18003 File chrome/browser/power_save_blocker_common.cc (right): http://codereview.chromium.org/287017/diff/17001/18003#newcode12 Line 12: enabled_ = false; On 2009/10/21 04:49:16, Mark Mentovai wrote: > For constructors, initializer lists are preferred. Done. http://codereview.chromium.org/287017/diff/17001/18003#newcode33 Line 33: NewRunnableFunction(&PowerSaveBlocker::adjust, delta)); On 2009/10/21 04:49:16, Mark Mentovai wrote: > 4-space indent (not 2) or lining up with something that makes sense from the > preceding line is generally preferred. Done. http://codereview.chromium.org/287017/diff/17001/18002 File chrome/browser/power_save_blocker_stub.cc (right): http://codereview.chromium.org/287017/diff/17001/18002#newcode10 Line 10: // STUB On 2009/10/21 04:49:16, Mark Mentovai wrote: > NOTIMPLEMENTED(); ? > > (from base/logging.h) Done. http://codereview.chromium.org/287017/diff/17001/18004 File chrome/browser/power_save_blocker_win.cc (right): http://codereview.chromium.org/287017/diff/17001/18004#newcode11 Line 11: static int sleep_blocker_count = 0; On 2009/10/21 04:49:16, Mark Mentovai wrote: > Unused. Oops, left over from an earlier implementation I discarded. Removed :) http://codereview.chromium.org/287017/diff/17001/18007 File chrome/chrome.gyp (right): http://codereview.chromium.org/287017/diff/17001/18007#newcode1840 Line 1840: 'browser/power_save_blocker_stub.cc', On 2009/10/21 04:49:16, Mark Mentovai wrote: > Put power_save_blocker_win.cc here too. _win.cc files are automatically > excluded in non-Windows builds. Done. For future reference, does this apply to _mac and _linux as well?
Regarding the count question. I see now the need. What its left is some style nits. You are almost there. http://codereview.chromium.org/287017/diff/19002/20005 File chrome/browser/power_save_blocker.h (right): http://codereview.chromium.org/287017/diff/19002/20005#newcode13 Line 13: public: please specify the default ctor behavior, is it on or off? http://codereview.chromium.org/287017/diff/19002/20005#newcode20 Line 20: void Enable(); please add comments on Enable() and disable. Don't need to be too long. http://codereview.chromium.org/287017/diff/19002/20005#newcode24 Line 24: remove empty line in 24 http://codereview.chromium.org/287017/diff/19002/20005#newcode29 Line 29: // Called only from UI thread period at the end of comments, same on line 32 and elsewhere. don't put them in the #endif ones http://codereview.chromium.org/287017/diff/19002/20004 File chrome/browser/power_save_blocker_win.cc (right): http://codereview.chromium.org/287017/diff/19002/20004#newcode11 Line 11: // Runs on UI thread only period
http://codereview.chromium.org/287017/diff/19002/20006 File chrome/browser/download/download_file.h (right): http://codereview.chromium.org/287017/diff/19002/20006#newcode55 Line 55: #include "chrome/browser/power_save_blocker.h" Sort: c comes between b and g. http://codereview.chromium.org/287017/diff/19002/20005 File chrome/browser/power_save_blocker.h (right): http://codereview.chromium.org/287017/diff/19002/20005#newcode14 Line 14: PowerSaveBlocker(); Do we need a default constructor at all? I don't remember anything using it. I think it should be omitted. http://codereview.chromium.org/287017/diff/19002/20005#newcode29 Line 29: // Called only from UI thread cpu wrote: > period at the end of comments, same on line 32 and elsewhere. don't put them in > the #endif ones Also download_file.h:157...
Updated per style reviews. http://codereview.chromium.org/287017/diff/19002/20006 File chrome/browser/download/download_file.h (right): http://codereview.chromium.org/287017/diff/19002/20006#newcode55 Line 55: #include "chrome/browser/power_save_blocker.h" On 2009/10/22 01:23:25, Mark Mentovai wrote: > Sort: c comes between b and g. Done. http://codereview.chromium.org/287017/diff/19002/20005 File chrome/browser/power_save_blocker.h (right): http://codereview.chromium.org/287017/diff/19002/20005#newcode14 Line 14: PowerSaveBlocker(); On 2009/10/22 01:23:25, Mark Mentovai wrote: > Do we need a default constructor at all? I don't remember anything using it. I > think it should be omitted. Good point, removed. We can always add one later if it turns out to be convenient, after all. http://codereview.chromium.org/287017/diff/19002/20005#newcode20 Line 20: void Enable(); On 2009/10/22 01:05:25, cpu wrote: > please add comments on Enable() and disable. Don't need to be too long. Done. http://codereview.chromium.org/287017/diff/19002/20005#newcode24 Line 24: On 2009/10/22 01:05:25, cpu wrote: > remove empty line in 24 Done. http://codereview.chromium.org/287017/diff/19002/20005#newcode29 Line 29: // Called only from UI thread On 2009/10/22 01:05:25, cpu wrote: > period at the end of comments, same on line 32 and elsewhere. don't put them in > the #endif ones Done. http://codereview.chromium.org/287017/diff/19002/20005#newcode29 Line 29: // Called only from UI thread On 2009/10/22 01:23:25, Mark Mentovai wrote: > cpu wrote: > > period at the end of comments, same on line 32 and elsewhere. don't put them > in > > the #endif ones > > Also download_file.h:157... Done. http://codereview.chromium.org/287017/diff/19002/20004 File chrome/browser/power_save_blocker_win.cc (right): http://codereview.chromium.org/287017/diff/19002/20004#newcode11 Line 11: // Runs on UI thread only On 2009/10/22 01:05:25, cpu wrote: > period Done.
LGTM
On 2009/10/22 18:21:26, cpu wrote: > LGTM Good to hear :) What do I need to do next to get this committed? Also, I just noticed, but http://dev.chromium.org/developers/contributing-code mentions I must add myself to AUTHORS - I didn't include it in this patch; will whoever commits add my entry for me? It should be: Bryan Donlan <bdonlan@gmail.com>
Patch updated with AUTHORS entry :)
Sent to the try servers for testing.
On 2009/10/22 20:15:14, Mark Mentovai wrote: > Sent to the try servers for testing. It seems I forgot to rename apply_block() in _stub.cc - but also, for some reason the entire content of _common.cc and _stub.cc was pasted twice in the file, when sent to the tryservers. This can be seen in the patch here: http://build.chromium.org/buildbot/try-server/builders/mac/builds/4600/steps/... This isn't in the codereview patch, how'd that happen? I'll send out a revision with _stub.cc fixed shortly.
The files that repeated themselves were my fault. I'll do better on the next try job. Mark On Thu, Oct 22, 2009 at 4:44 PM, <bdonlan@gmail.com> wrote: > On 2009/10/22 20:15:14, Mark Mentovai wrote: >> >> Sent to the try servers for testing. > > It seems I forgot to rename apply_block() in _stub.cc - but also, for some > reason the entire content of _common.cc and _stub.cc was pasted twice in the > file, when sent to the tryservers. This can be seen in the patch here: > http://build.chromium.org/buildbot/try-server/builders/mac/builds/4600/steps/... > > This isn't in the codereview patch, how'd that happen? > > I'll send out a revision with _stub.cc fixed shortly. > > http://codereview.chromium.org/287017 >
New try jobs launched.
Okay. BTW, I tried attaching a comment to my patch with gcl upload -m this time, but it seems to have been cut off after the first word - does gcl not like that on windows? (or do single quotes not work with gcl on windows?) The exact command I used was: gcl upload download-no-suspend -m 'Fix ApplyBlock() naming in power_save_blocker_stub.cc; add newline to new line in AUTHORS' --no_try --send_mail On Thu, Oct 22, 2009 at 4:46 PM, Mark Mentovai <mark@chromium.org> wrote: > The files that repeated themselves were my fault. =A0I'll do better on > the next try job. > > Mark > > On Thu, Oct 22, 2009 at 4:44 PM, =A0<bdonlan@gmail.com> wrote: >> On 2009/10/22 20:15:14, Mark Mentovai wrote: >>> >>> Sent to the try servers for testing. >> >> It seems I forgot to rename apply_block() in _stub.cc - but also, for so= me >> reason the entire content of _common.cc and _stub.cc was pasted twice in= the >> file, when sent to the tryservers. This can be seen in the patch here: >> http://build.chromium.org/buildbot/try-server/builders/mac/builds/4600/s= teps/gclient/logs/patch >> >> This isn't in the codereview patch, how'd that happen? >> >> I'll send out a revision with _stub.cc fixed shortly. >> >> http://codereview.chromium.org/287017 >> >
On 2009/10/22 20:54:26, Mark Mentovai wrote: > New try jobs launched. Okay, now what's going on? It's failing in the middle of webcore on windows and I'm fairly sure I shouldn't have touched anything that could affect that... See: http://build.chromium.org/buildbot/try-server/builders/win/builds/4887/steps/...
r29873 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
