Get rid of FilePath::AppendAndResolveRelative()....

Get rid of FilePath::AppendAndResolveRelative().

To resolve the problem of '..' parent references as well as symbolic links on POSIX platforms, we can simply use the file_util::AbsolutePath() function.

This has the drawback of having a different behavior on Windows and POSIX platforms, in the way that it can return a canonical path that doesn't exists when ran on Windows, but it will return an empty path (or false) when run on a POSIX platform.

So we need to add an extra PathExists() call to unify the behavior.

BUG=25681, 25131
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30334

[MAD, 2009-10-28 00:02:14]

Could this me the one? :-)

Thanks again!

BYE
MAD

[Mark Mentovai, 2009-10-28 00:20:47]

LG if the try servers like it.  You might want to wait for an extensions dude to
chime in too.

http://codereview.chromium.org/343003/diff/2001/2008
File chrome/test/data/extensions/samples/gmail_browser_action/manifest.json
(left):

http://codereview.chromium.org/343003/diff/2001/2008#oldcode13
Line 13: "128": "icon_128.png"
What's up here, does this not exist?

[MAD, 2009-10-28 03:44:45]

Fixed aa's email which I goofed in the first try...

Answered Mark's question...

And will retry the linux bot, it times out while compiling Skia on the first
try...

Thanks!

BYE
MAD

http://codereview.chromium.org/343003/diff/2001/2008
File chrome/test/data/extensions/samples/gmail_browser_action/manifest.json
(left):

http://codereview.chromium.org/343003/diff/2001/2008#oldcode13
Line 13: "128": "icon_128.png"
On 2009/10/28 00:20:48, Mark Mentovai wrote:
> What's up here, does this not exist?

Yep, I cleaned it up as I tried it and it failed because the icon has been
removed from the repo, but the manifest file wasn't updated... I double checked
with Aaron :-)

[Aaron Boodman, 2009-10-28 06:08:18]

lgtm

http://codereview.chromium.org/343003/diff/2001/2008
File chrome/test/data/extensions/samples/gmail_browser_action/manifest.json
(left):

http://codereview.chromium.org/343003/diff/2001/2008#oldcode13
Line 13: "128": "icon_128.png"
On 2009/10/28 03:44:45, mad1 wrote:
> On 2009/10/28 00:20:48, Mark Mentovai wrote:
> > What's up here, does this not exist?
> 
> Yep, I cleaned it up as I tried it and it failed because the icon has been
> removed from the repo, but the manifest file wasn't updated... I double
checked
> with Aaron :-)

I've since added this file. Please undo this delete.

[MAD, 2009-10-28 12:40:54]

All green... committing...

Thanks all...

BE:)
MAD

http://codereview.chromium.org/343003/diff/2001/2008
File chrome/test/data/extensions/samples/gmail_browser_action/manifest.json
(left):

http://codereview.chromium.org/343003/diff/2001/2008#oldcode13
Line 13: "128": "icon_128.png"
On 2009/10/28 06:08:18, Aaron Boodman wrote:
> On 2009/10/28 03:44:45, mad1 wrote:
> > On 2009/10/28 00:20:48, Mark Mentovai wrote:
> > > What's up here, does this not exist?
> > 
> > Yep, I cleaned it up as I tried it and it failed because the icon has been
> > removed from the repo, but the manifest file wasn't updated... I double
> checked
> > with Aaron :-)
> 
> I've since added this file. Please undo this delete.

Done.

[brettw, 2009-10-28 16:56:22]

It looks like this function (even before this change) did a bunch of file I/O.

It also looks to me like this is called from the UI thread. Can an extension
person clarify how extension loading is done? We really don't want to block the
UI thread on this type of thing.

[MAD, 2009-10-28 17:05:56]

I'll let the extension folks confirm it, but from what I have seen, the
reason for existence of this object (ExtensionResouce) is to package the
extension root path and a resource relative path so that they can be sent to
the IO thread for getting the full path and fetching the resource...

For example, inchrome\browser\extensions\execute_code_in_tab_function.cc

>   scoped_refptr<FileReader> file_reader(new FileReader(

      resource_, NewCallback(this,
> &ExecuteCodeInTabFunction::DidLoadFile)));

  file_reader->Start();

using chrome\browser\extensions\file_reader.h

> void FileReader::Start() {

  ChromeThread::PostTask(

      ChromeThread::FILE, FROM_HERE,

      NewRunnableMethod(this, &FileReader::ReadFileOnBackgroundThread));

}


> void FileReader::ReadFileOnBackgroundThread() {

  std::string data;

  bool success = file_util::ReadFileToString(resource_.GetFilePath(),
> &data);

  origin_loop_->PostTask(FROM_HERE, NewRunnableMethod(

      this, &FileReader::RunCallback, success, data));

}



Do we have a way to assert/DCHECK that we are actually running in the
ChromeThread::FILE thread?

BYE
MAD

On Wed, Oct 28, 2009 at 12:56 PM, <brettw@chromium.org> wrote:

> It looks like this function (even before this change) did a bunch of file
> I/O.
>
> It also looks to me like this is called from the UI thread. Can an
> extension
> person clarify how extension loading is done? We really don't want to block
> the
> UI thread on this type of thing.
>
>
> http://codereview.chromium.org/343003
>

[MAD, 2009-10-28 17:07:21]

I replied too quickly, found it... I could add this in
ExtensionResource::GetFilePath() maybe?

  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));


BYE
MAD

On Wed, Oct 28, 2009 at 1:05 PM, Marc-Andre Decoste <mad@chromium.org>wrote:

> I'll let the extension folks confirm it, but from what I have seen, the
> reason for existence of this object (ExtensionResouce) is to package the
> extension root path and a resource relative path so that they can be sent to
> the IO thread for getting the full path and fetching the resource...
>
> For example, inchrome\browser\extensions\execute_code_in_tab_function.cc
>
>>   scoped_refptr<FileReader> file_reader(new FileReader(
>
>       resource_, NewCallback(this,
>> &ExecuteCodeInTabFunction::DidLoadFile)));
>
>   file_reader->Start();
>
> using chrome\browser\extensions\file_reader.h
>
>> void FileReader::Start() {
>
>   ChromeThread::PostTask(
>
>       ChromeThread::FILE, FROM_HERE,
>
>       NewRunnableMethod(this, &FileReader::ReadFileOnBackgroundThread));
>
> }
>
>
>> void FileReader::ReadFileOnBackgroundThread() {
>
>   std::string data;
>
>   bool success = file_util::ReadFileToString(resource_.GetFilePath(),
>> &data);
>
>   origin_loop_->PostTask(FROM_HERE, NewRunnableMethod(
>
>       this, &FileReader::RunCallback, success, data));
>
> }
>
>
>
> Do we have a way to assert/DCHECK that we are actually running in the
> ChromeThread::FILE thread?
>
> BYE
> MAD
>
>
> On Wed, Oct 28, 2009 at 12:56 PM, <brettw@chromium.org> wrote:
>
>> It looks like this function (even before this change) did a bunch of file
>> I/O.
>>
>> It also looks to me like this is called from the UI thread. Can an
>> extension
>> person clarify how extension loading is done? We really don't want to
>> block the
>> UI thread on this type of thing.
>>
>>
>> http://codereview.chromium.org/343003
>>
>
>