Fix crash of chrome.fileSystem.chooseEntry on Linux.

Fix crash of chrome.fileSystem.chooseEntry on Linux.

I misunderstood FileSelected() is used only from FileSelectedWithExtraInfo(),
but it was wrong. There are situations it is used by itself,
hence NOTREACHED() does not hold at all.

BUG=224196
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190915

[kinaba, 2013-03-27 08:28:02]

I'm sorry I was dumb.
Ben, could you take a look?

[benwells, 2013-03-27 09:05:50]

no need to be sorry. gotta break some eggs to make an omelette...
but, sorry i missed it in the review :)

lgtm

[kinaba, 2013-03-27 09:07:49]

On 2013/03/27 09:05:50, benwells wrote:
> no need to be sorry. gotta break some eggs to make an omelette...
> but, sorry i missed it in the review :)
> 
> lgtm

Thanks. Putting this in and will request merge after verified it safely landed
and fixed the crash.

[commit-bot: I haz the power, 2013-03-27 09:09:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/13042004/1

[commit-bot: I haz the power, 2013-03-27 12:24:38]

Change committed as 190915

[Matt Giuca, 2013-03-27 23:02:33]

On 2013/03/27 12:24:38, I haz the power (commit-bot) wrote:
> Change committed as 190915

Thanks for a quick response, Kazuhiro. It looks like this will fix (compiling
now).

But I'm confused that you've rolled back the point of your original change.
You've still got the FileSelectedWithExtraInfo method there, but you don't
appear to call it at all any more. Shouldn't you either fully revert the
original change (including removing FileSelectedWithExtraInfo and the two new
#include lines), or keep the call to FileSelectedWithExtraInfo from FilePicker?

Lastly, as I said in the bug, I think this patch will need to be merged into the
M27 branch, because otherwise this bug is going to make its way into Chrome 27
stable. (But I'm still confused about the Chrome release process so I'm not sure
if this is true.)

[kinaba, 2013-03-27 23:12:39]

Thanks for you to find the bug very quickly.

On 2013/03/27 23:02:33, Matt Giuca wrote:
> You've still got the FileSelectedWithExtraInfo method there, but you don't
> appear to call it at all any more.

It is implementing an interface for receiving results from the file chooser
dialog,
and is called from the OS-dependent dialog implementation
(say, from
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...).

> Lastly, as I said in the bug, I think this patch will need to be merged into
the
> M27 branch,

True. I'll start the merging procedure once I verified it safely worked in
canary/dev channel.

[benwells, 2013-03-27 23:13:42]

On 2013/03/27 23:02:33, Matt Giuca wrote:
> On 2013/03/27 12:24:38, I haz the power (commit-bot) wrote:
> > Change committed as 190915
> 
> Thanks for a quick response, Kazuhiro. It looks like this will fix (compiling
> now).
> 
> But I'm confused that you've rolled back the point of your original change.
> You've still got the FileSelectedWithExtraInfo method there, but you don't
> appear to call it at all any more. Shouldn't you either fully revert the
> original change (including removing FileSelectedWithExtraInfo and the two new
> #include lines), or keep the call to FileSelectedWithExtraInfo from
FilePicker?
> 
> Lastly, as I said in the bug, I think this patch will need to be merged into
the
> M27 branch, because otherwise this bug is going to make its way into Chrome 27
> stable. (But I'm still confused about the Chrome release process so I'm not
sure
> if this is true.)

The call from FilePicker is only for tests, it normally called by the file
selection dialog implementation. In the basic file selection dialog, it calls
FileSelectedWithExtraInfo. The default implementation of this is to call
FileSelected, but it can be overridden to do something special.

The file selection dialog has different implementations (or overridden
implementations, not sure) on other platform however.