Move system.display tests to extensions/

Move system.display tests to extensions/

Also copy a few helper functions from extension_function_test_utils.h to
api_test_utils.h so we don't need to depend on chrome/. These helper
functions are needed by tests other than the display test and will help with
the migration.

BUG=392842
Committed: https://crrev.com/5c855a28156ed8f4d3f191ea7a1f9ee3dc777ee7
Cr-Commit-Position: refs/heads/master@{#309514}

[tmpsantos, 2014-12-05 12:44:15]

Patchset #1 (id:1) has been deleted

[tmpsantos, 2014-12-05 15:40:55]

thiago.santos@intel.com changed reviewers:
+ jamescook@chromium.org, thestig@chromium.org, yoz@chromium.org

[tmpsantos, 2014-12-05 15:41:41]

thiago.santos@intel.com changed reviewers:
+ hongbo.min@intel.com

[tmpsantos, 2014-12-05 15:41:41]

hongbo.min@intel.com: Please review changes in

[tmpsantos, 2014-12-05 15:43:23]

Please review.

app_shell_browsertests will now depend on ash/. I made a new DEPS file very
restrict to make sure that the display test is the exception.

[James Cook, 2014-12-05 16:44:16]

No. Do not add dependencies on ash/ in extensions/, even for tests.

There must be a way to test this function without an ash/ dependency, as the
implementation itself does not use ash.

You need to find some other way to get the Screen object you need.  It's
possible this may need some refactoring of how screen works. If you get stuck
oshima@ might have some advice.

[tmpsantos, 2014-12-18 16:22:49]

On 2014/12/05 16:44:16, James Cook wrote:
> No. Do not add dependencies on ash/ in extensions/, even for tests.
> 
> There must be a way to test this function without an ash/ dependency, as the
> implementation itself does not use ash.
> 
> You need to find some other way to get the Screen object you need.  It's
> possible this may need some refactoring of how screen works. If you get stuck
> oshima@ might have some advice.

Thanks for reviewing James. I updated the patch removing the ash:: dependency.
I'm still trying to figure it out why the dependency was there in the first
place.

[tmpsantos, 2014-12-18 16:25:50]

Patchset #2 (id:40001) has been deleted

[tmpsantos, 2014-12-18 16:26:07]

Patchset #2 (id:40001) has been deleted

[James Cook, 2014-12-19 18:07:03]

LGTM but please try to reduce the code duplication with
extension_function_test_utils.cc

Thanks for taking care of this! The code looks cleaner.

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api/s...
File extensions/browser/api/system_display/system_display_apitest.cc (right):

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api/s...
extensions/browser/api/system_display/system_display_apitest.cc:67: };
It's great you were able to get rid of this #ifdef chromeos block!

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api_t...
File extensions/browser/api_test_utils.cc (right):

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api_t...
extensions/browser/api_test_utils.cc:81: scoped_refptr<extensions::Extension>
CreateExtension(
Can you move this out of the anonymous namespace and make the code in
chrome/browser/extensions/extension_function_utils.cc call this version? Then
there will be less duplicated code.

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api_t...
extensions/browser/api_test_utils.cc:103: base::DictionaryValue*
ParseDictionary(const std::string& data) {
Likewise, can the extension_function_utils.cc versions just call these versions?

That's a strong signal to engineers touching //chrome that they really ought to
use these versions.

[Lei Zhang, 2014-12-19 21:19:39]

chrome/ lgtm

[Yoyo Zhou, 2014-12-19 22:07:22]

LGTM with James's comments addressed

[tmpsantos, 2014-12-22 23:04:40]

Patchset #3 (id:80001) has been deleted

[tmpsantos, 2014-12-23 01:47:59]

The CQ bit was checked by thiago.santos@intel.com

[commit-bot: I haz the power, 2014-12-23 01:49:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/779083002/100001

[commit-bot: I haz the power, 2014-12-23 02:01:18]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2014-12-23 02:02:12]

Patchset 3 (id:??) landed as
https://crrev.com/5c855a28156ed8f4d3f191ea7a1f9ee3dc777ee7
Cr-Commit-Position: refs/heads/master@{#309514}

[tmpsantos, 2014-12-23 02:09:38]

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api_t...
File extensions/browser/api_test_utils.cc (right):

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api_t...
extensions/browser/api_test_utils.cc:81: scoped_refptr<extensions::Extension>
CreateExtension(
On 2014/12/19 18:07:03, James Cook (OOO until Jan 5) wrote:
> Can you move this out of the anonymous namespace and make the code in
> chrome/browser/extensions/extension_function_utils.cc call this version? Then
> there will be less duplicated code.

Acknowledged.

https://codereview.chromium.org/779083002/diff/60001/extensions/browser/api_t...
extensions/browser/api_test_utils.cc:103: base::DictionaryValue*
ParseDictionary(const std::string& data) {
On 2014/12/19 18:07:03, James Cook (OOO until Jan 5) wrote:
> Likewise, can the extension_function_utils.cc versions just call these
versions?
> 
> That's a strong signal to engineers touching //chrome that they really ought
to
> use these versions.

Acknowledged.