Prefer ScopedNestableTaskAllower over manual save/restore

chrome/browser/printing/print_job.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/printing/print_job.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/message_loop.h"
 #include "base/threading/thread_restrictions.h"
@@ skipping 180 matching lines @@
 
 bool PrintJob::FlushJob(int timeout_ms) {
   // Make sure the object outlive this message loop.
   scoped_refptr<PrintJob> handle(this);
 
   // Stop() will eventually be called, which will get out of the inner message
   // loop. But, don't take it for granted and set a timer in case something goes
   // wrong.
   base::OneShotTimer<MessageLoop> quit_task;
   if (timeout_ms) {
     quit_task.Start(FROM_HERE, TimeDelta::FromMilliseconds(timeout_ms),

[jar (doing other things), 2012/02/11 03:24:38]

This is pretty evil/problematic... though not related to this CL change.  This
pushes a quite message into the pending message queue. If *any* other user makes
yet another recursive  (nested) message loop run, then *they* can be terminated
by this task.

The pattern of posting a quite task this way is more than fraught with peril,
IMO.

[dhollowa, 2012/02/13 17:44:26]

Looks like this was added by Darin.  I'm not sure of the history.

[jar (doing other things), 2012/02/13 18:03:02]

Darin,

This code looks pretty risky... especially since there are other folks (like in
UI) that are using nested tasks (which the Quit() can terminate).

Can you justify why this is reasonable?

Thanks,

Jim

On 2012/02/13 17:44:26, dhollowa wrote:

                     MessageLoop::current(), &MessageLoop::Quit);
   }
 
-  bool old_state = MessageLoop::current()->NestableTasksAllowed();
-  MessageLoop::current()->SetNestableTasksAllowed(true);
+  MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current());
   MessageLoop::current()->Run();
-  // Restore task state.
-  MessageLoop::current()->SetNestableTasksAllowed(old_state);
 
   return true;
 }
 
 void PrintJob::DisconnectSource() {
   source_ = NULL;
   if (document_.get())
     document_->DisconnectSource();
 }
 
@@ skipping 145 matching lines @@
 
 PrintedDocument* JobEventDetails::document() const {
   return document_;
 }
 
 PrintedPage* JobEventDetails::page() const {
   return page_;
 }
 
 }  // namespace printing