chrome: Use base::MessageLoop. (Part 3)

chrome: Use base::MessageLoop. (Part 3)

This CL updates chrome/browser/* other than chrome/browser/ui which is in
another CL.

BUG=236029
TBR=brettw

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202664

[xhwang, 2013-04-27 05:18:22]

PTAL

[brettw, 2013-04-28 04:26:26]

You were a bit too aggressive reindenting things that didn't need to be changed.
In particular, there's no requirement that parameters for function calls go on
separate lines. You can do this in some cases if it helps readability, although
I almost never do. And in particular, I would never change it for existing code
unless it was clearly weird.

I got through about half the patch and marked the places I thought should be
reverted. I stopped at "chrome/browser/extensions/installed_loader.cc". Can you
check the rest for unnecessary reindenting and revert as necessary?

Thanks!

https://codereview.chromium.org/14113053/diff/1/chrome/browser/autocomplete/h...
File chrome/browser/autocomplete/history_quick_provider_unittest.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/autocomplete/h...
chrome/browser/autocomplete/history_quick_provider_unittest.cc:231:
AutocompleteInput input(text,
I wouldn't bother changing this (I would normally write the previous formatting
anyway.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/autocomplete/s...
File chrome/browser/autocomplete/shortcuts_provider_unittest.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/autocomplete/s...
chrome/browser/autocomplete/shortcuts_provider_unittest.cc:229:
AutocompleteInput input(text,
Ditto

https://codereview.chromium.org/14113053/diff/1/chrome/browser/chromeos/chrom...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/chromeos/chrom...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:132: virtual
base::EventStatus WillProcessEvent(const base::NativeEvent& event)
I preferred the old wrapping here (OVERRIDE on its own line looks weird to me).

https://codereview.chromium.org/14113053/diff/1/chrome/browser/chromeos/web_s...
File chrome/browser/chromeos/web_socket_proxy.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/chromeos/web_s...
chrome/browser/chromeos/web_socket_proxy.cc:716:
base::MessageLoopForIO::current()->WatchFileDescriptor(
I wouldn't have changed the wrapping for the cases in this file.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/common/cancela...
File chrome/browser/common/cancelable_request.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/common/cancela...
chrome/browser/common/cancelable_request.cc:117:
&CancelableRequestBase::ExecuteCallback, this, forwarded_call));
Old wrapping was fine

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/api...
File
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc
(right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc:243:
base::Bind(&EPKPChallengeUserKey::UserConsentCallback,
Old wrapping was fine.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc:292:
this,
Ditto

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/storage/settings_apitest.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/storage/settings_apitest.cc:136:
->GetBackendForSync(kModelType));
Either way is fine, so I wouldn't change this. Same below.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/eve...
File chrome/browser/extensions/event_router.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/eve...
chrome/browser/extensions/event_router.cc:647:
base::Bind(&DispatchOnInstalledEvent,
I'd keep the old way, same below.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_disabled_ui.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_disabled_ui.cc:268:
uninstall_dialog_->AsWeakPtr(),
Old way was probably better

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_process_manager.cc:432:
weak_ptr_factory_.GetWeakPtr(),
Old way was fine.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_service.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_service.cc:2705: extension_id),
This was fine on the previous line.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_service_unittest.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_service_unittest.cc:3808: : result_(false),
weak_factory_(base::MessageLoop::current()) {}
In other cases you were a bit too aggressive putting things on separate lines
for function calls, but initializer lists should be on separate lines unless
everything (including the function name) fits on one, so the old way was better.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_service_unittest.cc:3812: FROM_HERE,
Old way was fine, below also.

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extensions_quota_service.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extensions_quota_service.cc:27: this,
Ditto

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/external_policy_loader_unittest.cc (right):

https://codereview.chromium.org/14113053/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/external_policy_loader_unittest.cc:31:
ui_thread_(BrowserThread::UI, &loop_) {}
I much prefer the } on a separate line.

[xhwang, 2013-05-25 18:58:30]

brettw@: I rebased this CL and removed all (I believe) unnecessary changes.

+thestig for OWNERS review since brettw@ is not an owner of this folder anymore.

PTAL!

[Lei Zhang, 2013-05-28 05:21:15]

lgtm (whew)

https://codereview.chromium.org/14113053/diff/36001/chrome/browser/metrics/th...
File chrome/browser/metrics/thread_watcher.cc (right):

https://codereview.chromium.org/14113053/diff/36001/chrome/browser/metrics/th...
chrome/browser/metrics/thread_watcher.cc:790: g_watchdog_thread->message_loop()
== base::MessageLoop::current();
can you 4 space indent here?

[xhwang, 2013-05-28 18:51:52]

https://codereview.chromium.org/14113053/diff/36001/chrome/browser/metrics/th...
File chrome/browser/metrics/thread_watcher.cc (right):

https://codereview.chromium.org/14113053/diff/36001/chrome/browser/metrics/th...
chrome/browser/metrics/thread_watcher.cc:790: g_watchdog_thread->message_loop()
== base::MessageLoop::current();
On 2013/05/28 05:21:16, Lei Zhang wrote:
> can you 4 space indent here?

Done.

[commit-bot: I haz the power, 2013-05-28 18:59:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/14113053/46001

[commit-bot: I haz the power, 2013-05-28 19:02:22]

Failed to apply patch for
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
  Hunk #1 FAILED at 166.
  Hunk #2 succeeded at 181 (offset 1 line).
  Hunk #3 succeeded at 214 with fuzz 1 (offset 1 line).
  Hunk #4 succeeded at 377 (offset 1 line).
  Hunk #5 succeeded at 440 (offset 1 line).
  Hunk #6 succeeded at 466 (offset 1 line).
  Hunk #7 succeeded at 474 (offset 1 line).
  Hunk #8 succeeded at 495 (offset 1 line).
  Hunk #9 succeeded at 504 (offset 1 line).
  Hunk #10 succeeded at 528 (offset 1 line).
  Hunk #11 succeeded at 537 (offset 1 line).
  Hunk #12 succeeded at 566 (offset 1 line).
  Hunk #13 succeeded at 577 (offset 1 line).
  Hunk #14 succeeded at 608 (offset 1 line).
  Hunk #15 succeeded at 619 (offset 1 line).
  Hunk #16 succeeded at 628 (offset 1 line).
  Hunk #17 succeeded at 678 (offset 1 line).
  Hunk #18 succeeded at 701 (offset 1 line).
  Hunk #19 succeeded at 716 (offset 1 line).
  Hunk #20 succeeded at 731 (offset 1 line).
  Hunk #21 succeeded at 753 (offset 1 line).
  Hunk #22 succeeded at 770 (offset 1 line).
  1 out of 22 hunks FAILED -- saving rejects to file
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc.rej

Patch:      
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
Index: chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
diff --git
a/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
b/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
index
292ad2eca4b21a0a060764c206b3acbf087d3011..082ae793f74dbe71e392189b44897a66855e0380
100644
--- a/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
+++ b/chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
@@ -166,13 +166,13 @@ class NetworkPortalDetectorImplTest
         dbus::ObjectPath(service_path),
         flimflam::kStateProperty, base::StringValue(flimflam::kStatePortal),
         base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
-    MessageLoop::current()->RunUntilIdle();
+    base::MessageLoop::current()->RunUntilIdle();
   }
 
   void SetNetworkDeviceEnabled(const std::string& type, bool enabled) {
     NetworkStateHandler::Get()->SetTechnologyEnabled(
         type, enabled, network_handler::ErrorCallback());
-    MessageLoop::current()->RunUntilIdle();
+    base::MessageLoop::current()->RunUntilIdle();
   }
 
   void SetConnected(const std::string& service_path) {
@@ -180,12 +180,12 @@ class NetworkPortalDetectorImplTest
         dbus::ObjectPath(service_path),
         flimflam::kStateProperty, base::StringValue(flimflam::kStateOnline),
         base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction));
-    MessageLoop::current()->RunUntilIdle();
+    base::MessageLoop::current()->RunUntilIdle();
   }
 
  private:
   void SetupDefaultShillState() {
-    MessageLoop::current()->RunUntilIdle();
+    base::MessageLoop::current()->RunUntilIdle();
     ShillServiceClient::TestInterface* service_test =
         DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
     service_test->ClearServices();
@@ -213,7 +213,7 @@ class NetworkPortalDetectorImplTest
     NetworkStateHandler::Initialize();
   }
 
-  MessageLoop message_loop_;
+  base::MessageLoop message_loop_;
   scoped_ptr<TestingProfile> profile_;
   scoped_ptr<NetworkPortalDetectorImpl> network_portal_detector_;
 };
@@ -376,7 +376,7 @@ TEST_F(NetworkPortalDetectorImplTest,
PortalDetectionTimeout) {
   ASSERT_EQ(0, attempt_count());
 
   SetConnected(kStubWireless1);
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   // First portal detection timeouts, next portal detection is
   // scheduled.
@@ -439,7 +439,7 @@ TEST_F(NetworkPortalDetectorImplTest, FirstAttemptFailed) {
   ASSERT_EQ(base::TimeDelta::FromSeconds(0), next_attempt_delay());
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 204, NULL);
   ASSERT_TRUE(is_state_idle());
@@ -465,7 +465,7 @@ TEST_F(NetworkPortalDetectorImplTest, AllAttemptsFailed) {
   ASSERT_EQ(base::TimeDelta::FromSeconds(0), next_attempt_delay());
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 503, retry_after);
   ASSERT_TRUE(is_state_portal_detection_pending());
@@ -473,7 +473,7 @@ TEST_F(NetworkPortalDetectorImplTest, AllAttemptsFailed) {
   ASSERT_EQ(base::TimeDelta::FromSeconds(0), next_attempt_delay());
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 503, retry_after);
   ASSERT_TRUE(is_state_idle());
@@ -494,7 +494,7 @@ TEST_F(NetworkPortalDetectorImplTest, ProxyAuthRequired) {
                    kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 407, NULL);
   ASSERT_EQ(2, attempt_count());
@@ -503,7 +503,7 @@ TEST_F(NetworkPortalDetectorImplTest, ProxyAuthRequired) {
                    kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 407, NULL);
   ASSERT_EQ(3, attempt_count());
@@ -527,7 +527,7 @@ TEST_F(NetworkPortalDetectorImplTest,
NoResponseButBehindPortal) {
   ASSERT_TRUE(is_state_portal_detection_pending());
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::ERR_CONNECTION_CLOSED,
                    net::URLFetcher::RESPONSE_CODE_INVALID,
@@ -536,7 +536,7 @@ TEST_F(NetworkPortalDetectorImplTest,
NoResponseButBehindPortal) {
   ASSERT_TRUE(is_state_portal_detection_pending());
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::ERR_CONNECTION_CLOSED,
                    net::URLFetcher::RESPONSE_CODE_INVALID,
@@ -565,7 +565,7 @@ TEST_F(NetworkPortalDetectorImplTest,
LazyDetectionForOnlineNetwork) {
       kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 204, NULL);
 
@@ -576,7 +576,7 @@ TEST_F(NetworkPortalDetectorImplTest,
LazyDetectionForOnlineNetwork) {
       kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   disable_lazy_detection();
 
@@ -607,7 +607,7 @@ TEST_F(NetworkPortalDetectorImplTest,
LazyDetectionForPortalNetwork) {
                    kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::ERR_CONNECTION_CLOSED,
                    net::URLFetcher::RESPONSE_CODE_INVALID,
@@ -618,7 +618,7 @@ TEST_F(NetworkPortalDetectorImplTest,
LazyDetectionForPortalNetwork) {
                    kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   CompleteURLFetch(net::OK, 200, NULL);
   ASSERT_EQ(3, attempt_count());
@@ -627,7 +627,7 @@ TEST_F(NetworkPortalDetectorImplTest,
LazyDetectionForPortalNetwork) {
                    kStubWireless1);
 
   // To run CaptivePortalDetector::DetectCaptivePortal().
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
 
   disable_lazy_detection();
 
@@ -677,7 +677,7 @@ TEST_F(NetworkPortalDetectorImplTest, TestDetectionRestart)
{
   ASSERT_TRUE(is_state_portal_detection_pending());
   ASSERT_FALSE(start_detection_if_idle());
 
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
   ASSERT_TRUE(is_state_checking_for_portal());
   CompleteURLFetch(net::OK, 200, NULL);
 
@@ -700,13 +700,13 @@ TEST_F(NetworkPortalDetectorImplTest, RequestTimeouts) {
 
   // Second portal detection attempt for cellular1 uses 10sec timeout.
   ASSERT_TRUE(is_state_portal_detection_pending());
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
   CheckRequestTimeoutAndCompleteAttempt(2, 10, net::ERR_CONNECTION_CLOSED,
                                        
net::URLFetcher::RESPONSE_CODE_INVALID);
 
   // Third portal detection attempt for cellular1 uses 15sec timeout.
   ASSERT_TRUE(is_state_portal_detection_pending());
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
   CheckRequestTimeoutAndCompleteAttempt(3, 15, net::ERR_CONNECTION_CLOSED,
                                        
net::URLFetcher::RESPONSE_CODE_INVALID);
 
@@ -715,7 +715,7 @@ TEST_F(NetworkPortalDetectorImplTest, RequestTimeouts) {
   // Check that in lazy detection for cellular1 15sec timeout is used.
   enable_lazy_detection();
   ASSERT_TRUE(is_state_portal_detection_pending());
-  MessageLoop::current()->RunUntilIdle();
+  base::MessageLoop::current()->RunUntilIdle();
   disable_lazy_detection();
   CheckRequestTimeoutAndCompleteAttempt(3, 15, net::ERR_CONNECTION_CLOSED,
                                        
net::URLFetcher::RESPONSE_CODE_INVALID);
@@ -730,14 +730,14 @@ TEST_F(NetworkPortalDe…
(message too large)

[commit-bot: I haz the power, 2013-05-28 19:35:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/14113053/51001

[commit-bot: I haz the power, 2013-05-28 22:08:24]

Change committed as 202664