New issue
Advanced search Search tips

Issue 648276 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 648275



Sign in to add a comment

Remove ExtensionFunction::DelegateForTests

Project Member Reported by rdevlin....@chromium.org, Sep 19 2016

Issue description

We shouldn't need DelegateForTests when we can assign a custom callback.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/187edaa9952ac1c26f40b034cdf99cf3aae7e91f

commit 187edaa9952ac1c26f40b034cdf99cf3aae7e91f
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Mon Sep 19 21:34:02 2016

[Extensions] Remove UIThreadExtensionFunction::DelegateForTests

The ExtensionFunction test delegate basically just served as a way of
getting the results from the function, but we already have that by simply
setting the response callback in the function. Remove the
DelegateForTests class and update uses of it to use the callback.

Also add a response() field on ExtensionFunction for use in determining
if the function succeeded or failed. Previously, code would rely on
whether or not results_ were set, but this was wrong for a few reasons:
- The absence of results doesn't always indicate failure
- The presence of results doesn't always indicate success
- In practice, we always send back an empty list value (even if
  results weren't previously set).
Update code to use the response() value instead.

BUG= 648276 
TBR=benwells@chromium.org (apps browsertest change)

Review-Url: https://codereview.chromium.org/2348723002
Cr-Commit-Position: refs/heads/master@{#419560}

[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/chrome/browser/apps/app_browsertest_util.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_api_unittest.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/chrome/browser/extensions/api/identity/identity_apitest.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/chrome/browser/extensions/extension_function_test_utils.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/extensions/browser/api/bluetooth/bluetooth_apitest.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/extensions/browser/api/cast_channel/cast_channel_apitest.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/extensions/browser/api_test_utils.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/extensions/browser/api_test_utils.h
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/extensions/browser/extension_function.cc
[modify] https://crrev.com/187edaa9952ac1c26f40b034cdf99cf3aae7e91f/extensions/browser/extension_function.h

Status: Fixed (was: Assigned)

Sign in to add a comment