Chrome Alarm Clear and ClearAll Updates

chrome/browser/extensions/api/alarms/alarms_api.cc

 // Copyright (c) 2012 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/extensions/api/alarms/alarms_api.h"
 
 #include "base/strings/string_number_conversions.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "base/values.h"
@@ skipping 171 matching lines @@
   std::string name = params->name.get() ? *params->name : kDefaultAlarmName;
   AlarmManager::Get(GetProfile())
       ->RemoveAlarm(extension_id(),
                     name,
                     base::Bind(&AlarmsClearFunction::Callback, this, name));
 
   return true;
 }
 
 void AlarmsClearFunction::Callback(const std::string& name, bool success) {
-  if (!success)
-    error_ = ErrorUtils::FormatErrorMessage(kAlarmNotFound, name);
-
-  SendResponse(success);
+  SetResult(new base::FundamentalValue(success));
+  SendResponse(true);
 }
 
 bool AlarmsClearAllFunction::RunImpl() {
   AlarmManager::Get(GetProfile())->RemoveAllAlarms(
       extension_id(), base::Bind(&AlarmsClearAllFunction::Callback, this));
   return true;
 }
 
 void AlarmsClearAllFunction::Callback() {
+  SetResult(new base::FundamentalValue(true));

[not at google - send to devlin, 2014/03/20 10:52:21]

seems more correct to set this to (num_alarms_cleared > 0) if you can plumb that
through. seems trivial.

[Matt Perry, 2014/03/20 18:53:42]

hmm.. I think generally removeAll() on the empty set would result in success.

[robliao, 2014/03/20 19:01:05]

There are two philosophies to take here:
1) We treat this as a success indicator. This means it returns true.
2) We treat this as an indicator work was done. This means it return false.

I'm not sure which is more useful, so let's go with consistency for APIs that
clear everything. chrome.storage.clear uses success. The other ones are
non-obvious at first glance.

On 2014/03/20 18:53:42, Matt Perry wrote:

[Matt Perry, 2014/03/20 21:07:00]

Yeah, #1 makes more sense IMO. I think it should only return false if
num_alarms_remaining != 0.

On 2014/03/20 19:01:05, robliao wrote:

[robliao, 2014/03/20 21:20:30]

We do a hard CHECK if that happens, so we should be good to go for checkin as
is.

If there are no other objections, I'll commit this later today.
On 2014/03/20 21:07:00, Matt Perry wrote:

   SendResponse(true);
 }
 
 }  // namespace extensions