Componentize shortcuts_database_unittest

Componentize shortcuts_database_unittest

Creates components/test/data/omnibox.
Move related test data files from c/t/data/history to components/test/data/omnibox/.
Move the shortcuts_database_unittest to components/omnibox/browser

BUG=568434
Committed: https://crrev.com/0dee20c6ca4156722669bcfd837462eb5e0fbee5
Cr-Commit-Position: refs/heads/master@{#364884}

[Abhishek, 2015-12-10 18:42:04]

Patchset #1 (id:1) has been deleted

[Abhishek, 2015-12-10 18:46:42]

Description was changed from

==========
Componentize shortcuts_database_unittest

BUG=None
==========

to

==========
Componentize shortcuts_database_unittest

Creates components/test/data/omnibox.
Move related test data files from c/t/data/history to
components/test/data/omnibox/.
Move the shortcuts_database_unittest to components/omnibox/browser

BUG=568434
==========

[Abhishek, 2015-12-10 18:46:42]

abhishek.a21@samsung.com changed reviewers:
+ droger@chromium.org

[Abhishek, 2015-12-10 18:50:29]

PTAL!

[Abhishek, 2015-12-10 19:02:29]

Patchset #2 (id:40001) has been deleted

[Abhishek, 2015-12-10 19:07:19]

Patchset #2 (id:60001) has been deleted

[Abhishek, 2015-12-10 19:17:25]

Patchset #2 (id:80001) has been deleted

[Abhishek, 2015-12-10 19:22:59]

Patchset #2 (id:100001) has been deleted

[droger, 2015-12-11 12:08:23]

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
File components/omnibox/browser/shortcuts_database_unittest.cc (right):

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:81: return dir;
I don't think the static local is useful here.

I would suggest simply:

base::FilePath path;
PathService::Get(base::DIR_SOURCE_ROOT, &path);
return path.AppendASCII("components/test/data/omnibox");

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:101: base::MessageLoop
message_loop_;
Is this even needed?

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:108: base::FilePath
db_path(temp_dir.path().AppendASCII(kShortcutsDatabaseName));
This does not compile on windows, can you try using Append() instead of
AppendASCII()?

[Abhishek, 2015-12-11 15:54:11]

PTAL!

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
File components/omnibox/browser/shortcuts_database_unittest.cc (right):

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:81: return dir;
On 2015/12/11 12:08:23, droger wrote:
> I don't think the static local is useful here.
> 
> I would suggest simply:
> 
> base::FilePath path;
> PathService::Get(base::DIR_SOURCE_ROOT, &path);
> return path.AppendASCII("components/test/data/omnibox");

Done.

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:101: base::MessageLoop
message_loop_;
On 2015/12/11 12:08:23, droger wrote:
> Is this even needed?

Yeah! We don't require it now.
I've removed it.

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:108: base::FilePath
db_path(temp_dir.path().AppendASCII(kShortcutsDatabaseName));
On 2015/12/11 12:08:23, droger wrote:
> This does not compile on windows, can you try using Append() instead of
> AppendASCII()?

Done.

[Abhishek, 2015-12-11 15:54:11]

PTAL!

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
File components/omnibox/browser/shortcuts_database_unittest.cc (right):

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:81: return dir;
On 2015/12/11 12:08:23, droger wrote:
> I don't think the static local is useful here.
> 
> I would suggest simply:
> 
> base::FilePath path;
> PathService::Get(base::DIR_SOURCE_ROOT, &path);
> return path.AppendASCII("components/test/data/omnibox");

Done.

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:101: base::MessageLoop
message_loop_;
On 2015/12/11 12:08:23, droger wrote:
> Is this even needed?

Yeah! We don't require it now.
I've removed it.

https://codereview.chromium.org/1515853002/diff/120001/components/omnibox/bro...
components/omnibox/browser/shortcuts_database_unittest.cc:108: base::FilePath
db_path(temp_dir.path().AppendASCII(kShortcutsDatabaseName));
On 2015/12/11 12:08:23, droger wrote:
> This does not compile on windows, can you try using Append() instead of
> AppendASCII()?

Done.

[droger, 2015-12-11 16:00:49]

droger@chromium.org changed reviewers:
+ pkasting@chromium.org

[droger, 2015-12-11 16:00:50]

LGTM

+pkasting as OWNER

[Peter Kasting, 2015-12-11 18:48:45]

LGTM

[Abhishek, 2015-12-12 02:31:36]

The CQ bit was checked by abhishek.a21@samsung.com

[commit-bot: I haz the power, 2015-12-12 02:35:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1515853002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1515853002/140001

[commit-bot: I haz the power, 2015-12-12 03:43:28]

Description was changed from

==========
Componentize shortcuts_database_unittest

Creates components/test/data/omnibox.
Move related test data files from c/t/data/history to
components/test/data/omnibox/.
Move the shortcuts_database_unittest to components/omnibox/browser

BUG=568434
==========

to

==========
Componentize shortcuts_database_unittest

Creates components/test/data/omnibox.
Move related test data files from c/t/data/history to
components/test/data/omnibox/.
Move the shortcuts_database_unittest to components/omnibox/browser

BUG=568434
==========

[commit-bot: I haz the power, 2015-12-12 03:43:28]

Committed patchset #3 (id:140001)

[commit-bot: I haz the power, 2015-12-12 03:45:07]

Description was changed from

==========
Componentize shortcuts_database_unittest

Creates components/test/data/omnibox.
Move related test data files from c/t/data/history to
components/test/data/omnibox/.
Move the shortcuts_database_unittest to components/omnibox/browser

BUG=568434
==========

to

==========
Componentize shortcuts_database_unittest

Creates components/test/data/omnibox.
Move related test data files from c/t/data/history to
components/test/data/omnibox/.
Move the shortcuts_database_unittest to components/omnibox/browser

BUG=568434

Committed: https://crrev.com/0dee20c6ca4156722669bcfd837462eb5e0fbee5
Cr-Commit-Position: refs/heads/master@{#364884}
==========

[commit-bot: I haz the power, 2015-12-12 03:45:08]

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