Add an iOS implementation for base::PathService::Get() with path key DIR_APP_DATA.

Add an iOS implementation for base::PathService::Get() with path key DIR_APP_DATA.

This implementation creates a DIR_APP_DATA directory if it does not exist. This is needed for iOS because on iOS NSApplicationSupportDirectory directory does not exist unless it is created explicitly. 

BUG=NONE
TEST=NONE

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149331

[rohitrao (ping after 24h), 2012-07-18 16:01:34]

https://chromiumcodereview.appspot.com/10795010/diff/2001/base/base_paths_mac.mm
File base/base_paths_mac.mm (right):

https://chromiumcodereview.appspot.com/10795010/diff/2001/base/base_paths_mac...
base/base_paths_mac.mm:63: #if defined(OS_IOS)
bool success = base::mac::GetUserDirectory(NSApplicationSupportDirectory,
                                                    result);
#if defined(OS_IOS)
if (success && !file_util::PathExists(*result))
  success = file_util::CreateDirectory(*result);
#endif
return success;


I wonder if we can drop the ifdef and push this through for desktop as well.

[chenyu, 2012-07-18 16:43:32]

https://chromiumcodereview.appspot.com/10795010/diff/2001/base/base_paths_mac.mm
File base/base_paths_mac.mm (right):

https://chromiumcodereview.appspot.com/10795010/diff/2001/base/base_paths_mac...
base/base_paths_mac.mm:63: #if defined(OS_IOS)
Done. I keep the if-def for now and will remove it if it is recommended.

(actually base::mac::GetUserDirectory() looks confusing to me. The comment gives
the impression that it returns true and fill result only when the directory is
found, but it calls NSSearchPathForDirectoriesInDomains(), which has no
guarantee that the returned directories exist) 

On 2012/07/18 16:01:34, rohitrao wrote:
> bool success = base::mac::GetUserDirectory(NSApplicationSupportDirectory,
>                                                     result);
> #if defined(OS_IOS)
> if (success && !file_util::PathExists(*result))
>   success = file_util::CreateDirectory(*result);
> #endif
> return success;
> 
> 
> I wonder if we can drop the ifdef and push this through for desktop as well.

[rohitrao (ping after 24h), 2012-07-25 14:45:51]

Can you add a section to the CL description talking about the change to create
the directory if it doesn't exist, and why that's important on iOS?

https://chromiumcodereview.appspot.com/10795010/diff/5001/base/base_paths_mac.mm
File base/base_paths_mac.mm (right):

https://chromiumcodereview.appspot.com/10795010/diff/5001/base/base_paths_mac...
base/base_paths_mac.mm:63: // On IOS, this directory does not exist unless it is
created explicitly.
This comment should move down inside the OS_IOS block.

[Chen Yu, 2012-07-30 08:51:55]

Done
On 2012/07/25 14:45:51, rohitrao wrote:
> Can you add a section to the CL description talking about the change to create
> the directory if it doesn't exist, and why that's important on iOS?
> 
>
https://chromiumcodereview.appspot.com/10795010/diff/5001/base/base_paths_mac.mm
> File base/base_paths_mac.mm (right):
> 
>
https://chromiumcodereview.appspot.com/10795010/diff/5001/base/base_paths_mac...
> base/base_paths_mac.mm:63: // On IOS, this directory does not exist unless it
is
> created explicitly.
> This comment should move down inside the OS_IOS block.

[rohitrao (ping after 24h), 2012-07-31 14:41:47]

+Mark

Mark, the Application Support directory doesn't exist inside the iOS sandbox by
default, so we have to add code to create it if needed.  On Mac, this has never
been a problem, but I'm not sure if the Application Support directory is always
guaranteed to exist or not.

We've #ifdefed the directory creation code to be iOS-specific, but we could just
as easily keep it on Mac as well.  What would you prefer?

[Mark Mentovai, 2012-07-31 23:04:13]

LGTM

https://chromiumcodereview.appspot.com/10795010/diff/9001/base/base_paths_mac.mm
File base/base_paths_mac.mm (right):

https://chromiumcodereview.appspot.com/10795010/diff/9001/base/base_paths_mac...
base/base_paths_mac.mm:65: #if defined(OS_IOS)
I would say that you don’t have to #ifdef this. It “probably” exists, but
there’s no guarantee in that if someone removes it, I don’t think the OS
automatically recreates it.

But DIR_CACHE doesn’t get the same treatment, and I’d think it’s subject to the
same rules, so I’m inclined to just leave this alone.

[rohitrao (ping after 24h), 2012-07-31 23:08:18]

https://chromiumcodereview.appspot.com/10795010/diff/9001/base/base_paths_mac.mm
File base/base_paths_mac.mm (right):

https://chromiumcodereview.appspot.com/10795010/diff/9001/base/base_paths_mac...
base/base_paths_mac.mm:65: #if defined(OS_IOS)
On 2012/07/31 23:04:13, Mark Mentovai wrote:
> But DIR_CACHE doesn’t get the same treatment, and I’d think it’s subject to
the
> same rules, so I’m inclined to just leave this alone.

Ok, will leave this as-is.  It's a two-line CL if we decide otherwise later, but
I don't expect the current code to ever cause any problems in practice.

[commit-bot: I haz the power, 2012-07-31 23:10:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/chenyu@chromium.org/10795010/9001

[commit-bot: I haz the power, 2012-08-01 00:29:17]

Change committed as 149331

[2005daga_gmail.com, 2017-02-02 23:40:36]

El lunes, 30 de julio de 2012, 3:51:00 (UTC-5), che...@chromium.org 
escribió:
>
> Reviewers: rohitrao, 
>
> Description: 
> Add an iOS implementation for base::PathService::Get() with path key 
> DIR_APP_DATA. 
>
> This implementation creates a DIR_APP_DATA directory if it does not exist. 
>   
> This 
> is needed for IOS because on iOS NSApplicationSupportDirectory directory   
> does 
> not exist unless it is created explicitly. 
>
> BUG=NONE 
> TEST=NONE 
>
> Please review this at https://chromiumcodereview.appspot.com/10795010/ 
>
> SVN Base: http://git.chromium.org/chromium/src.git@master 
>
> Affected files: 
>    M base/base_paths_mac.mm 
>
>
> Index: base/base_paths_mac.mm 
> diff --git a/base/base_paths_mac.mm b/base/base_paths_mac.mm 
> index   
>
56f3ecb9a91e9be5c8888e2f9e53fc04a4cbf64a..ee6c03f6d10ad74bd61eab338e3b3890b319d176

>   
> 100644 
> --- a/base/base_paths_mac.mm 
> +++ b/base/base_paths_mac.mm 
> @@ -59,8 +59,16 @@ bool PathProviderMac(int key, FilePath* result) { 
>             reinterpret_cast<const void*>(&base::PathProviderMac)); 
>       case base::DIR_CACHE: 
>         return base::mac::GetUserDirectory(NSCachesDirectory, result); 
> -    case base::DIR_APP_DATA: 
> -      return base::mac::GetUserDirectory(NSApplicationSupportDirectory,   
> result); 
> +    case base::DIR_APP_DATA: { 
> +      bool success =   
> base::mac::GetUserDirectory(NSApplicationSupportDirectory, 
> +                                                 result); 
> +#if defined(OS_IOS) 
> +      // On IOS, this directory does not exist unless it is created   
> explicitly. 
> +      if (success && !file_util::PathExists(*result)) 
> +        success = file_util::CreateDirectory(*result); 
> +#endif  // defined(OS_IOS) 
> +      return success; 
> +    } 
>       case base::DIR_SOURCE_ROOT: { 
>         // Go through PathService to catch overrides. 
>         if (!PathService::Get(base::FILE_EXE, result)) 
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.