Linux: Change ShellIntegration code to use GetXDGDirectory().

Linux: Change ShellIntegration code to use GetXDGDirectory().

Do some misc cleanup and adjust unit tests accordingly.

Committed: https://crrev.com/d2b1fcfa150c2e1cbc56eda72fffeb761e87d291
Cr-Commit-Position: refs/heads/master@{#312476}

[Lei Zhang, 2015-01-16 21:54:26]

thestig@chromium.org changed reviewers:
+ mgiuca@chromium.org

[Matt Giuca, 2015-01-18 23:52:11]

OK seems like an appropriate simplification. My main concern is that previously
the code was more robust if the user has no $XDG_HOME_DIR or $HOME, and now it
will give weird results. But since base::nix assumes that there is a $HOME, I
guess this is not a case we care about, so its fine.

Just one thing I'd prefer to change about the tests. (lgtm)

https://codereview.chromium.org/827013004/diff/1/chrome/browser/shell_integra...
File chrome/browser/shell_integration_linux_unittest.cc (right):

https://codereview.chromium.org/827013004/diff/1/chrome/browser/shell_integra...
chrome/browser/shell_integration_linux_unittest.cc:166: ASSERT_EQ(3U,
results.size());
I'm not sure why you changed this from a nice ElementsAre to a more awkward
length-and-individual-elements test. This new test is also not testing the
order. Is it because the output changed so now the first element of the result
is ""?

That's kind of unfortunate (part of the point of this test is to check that if
there is no $HOME, that GetDataSearchLocations just ignores it). But as it is
unlikely that there will be no $HOME, I think it's OK to change the behaviour.

But I do think the test should properly assert the full output of
GetDataSearchLocations. So:

EXPECT_THAT(
    ...,
    ElementsAre("",
                "/system/path/1",
                "/system/path/2"));

is my preference.

[Lei Zhang, 2015-01-20 23:13:24]

On 2015/01/18 23:52:11, Matt Giuca wrote:
> OK seems like an appropriate simplification. My main concern is that
previously
> the code was more robust if the user has no $XDG_HOME_DIR or $HOME, and now it
> will give weird results. But since base::nix assumes that there is a $HOME, I
> guess this is not a case we care about, so its fine.

GetHomeDir() in base/ will always return a valid home directory, even if it ends
up returning /tmp or c:\. If $HOME somehow disappears, then Chrome and every
other program will fail miserably. We don't need to worry about that happening
here.

> Just one thing I'd prefer to change about the tests. (lgtm)
> 
>
https://codereview.chromium.org/827013004/diff/1/chrome/browser/shell_integra...
> File chrome/browser/shell_integration_linux_unittest.cc (right):
> 
>
https://codereview.chromium.org/827013004/diff/1/chrome/browser/shell_integra...
> chrome/browser/shell_integration_linux_unittest.cc:166: ASSERT_EQ(3U,
> results.size());
> I'm not sure why you changed this from a nice ElementsAre to a more awkward
> length-and-individual-elements test. This new test is also not testing the
> order. Is it because the output changed so now the first element of the result
> is ""?

The first result is $HOME, which could be anything. Why don't I test that:
- size is 3
- element 0 is not empty
- element 1 is /system/path/1
- element 2 is /system/path/2

[Lei Zhang, 2015-01-21 21:42:14]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-01-21 21:43:20]

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

[commit-bot: I haz the power, 2015-01-21 22:12:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-21 22:13:50]

Patchset 2 (id:??) landed as
https://crrev.com/d2b1fcfa150c2e1cbc56eda72fffeb761e87d291
Cr-Commit-Position: refs/heads/master@{#312476}