Skip to content

Commit

Permalink
Allow PhotoKit to download photos from iCloud
Browse files Browse the repository at this point in the history 
Summary:
Resolves #7081 by allowing iCloud to download photos not stored on the device.

**Test plan (required)**

1. Verified existing photos stored on the device still display.
2. Deleted my iCloud photo library from my phone and verified the image downloads and displays.
Closes #9530

Differential Revision: D3838470

Pulled By: mkonicek

fbshipit-source-id: 810830a4246714b6e166e4411f3fa848b1f1b71c
  • Loading branch information
Ron Heft authored and Facebook Github Bot 7 committed Sep 8, 2016
1 parent 7398780 commit b0c13eb
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions Libraries/CameraRoll/RCTPhotoLibraryImageLoader.m
View file
Expand Up @@ -60,6 +60,9 @@ - (RCTImageLoaderCancellationBlock)loadImageForURL:(NSURL *)imageURL
PHAsset *asset = [results firstObject];
PHImageRequestOptions *imageOptions = [PHImageRequestOptions new];

// Allow PhotoKit to fetch images from iCloud
imageOptions.networkAccessAllowed = YES;

if (progressHandler) {
imageOptions.progressHandler = ^(double progress, NSError *error, BOOL *stop, NSDictionary<NSString *, id> *info) {
static const double multiplier = 1e6;
Expand Down

4 comments on commit b0c13eb

@worldlyjohn
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronaldheft this fix has been crashing for our users who have Shared Albums turned on. We upgraded from 0.33 (before this fix these albums were being ignored) to 0.35.1.

Specifically, it crashes here: https://github.com/facebook/react-native/blob/0.35-stable/Libraries/CameraRoll/RCTCameraRollManager.m#L193 saying image cannot be nil. Upon further inspection: filename=nil, width=0, height=0.

My guess is that photo isn't on phone so this data is empty, but addObject complains with nil image. Although I would think it should be displaying rest of metadata (which it has) and just skip image unless requested. I'm doing a getPhotos to read in library.

I may a CameraRoll module to use PHAsset instead of older ALAsset if nothing exists.

@worldlyjohn
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can share an album with you if you dont have any way to reproduce.
Test code below. You need to set groupName to a shared album on your device then run on device.

let testParams1 = {first: 200, groupTypes: 'All', groupName: "SHARED_ALBUM_NAME", assetType: 'Photos'}
CameraRoll.getPhotos(testParams1)
  .then((res) => {
      console.log('this will crash', res);
  })
  .catch((err)=> {
      console.error('Error: '+err+'........ using camParams: '+JSON.stringify(testParams1));
  });

@ronaldheft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@worldlyjohn From my own testing, the issue is fixed in iOS 10 but may persist in iOS 9/8. I suspect from your comment, it's not that this fix caused your app to crash, but not all cases of this bug have been resolved. If you test a release without this fix you will still experience the crashing.

Unfortunately, I have moved onto to other projects and can not offer support for this issue. I am not a Facebook employee or part of the React Native team, and was simply contributing a fix that addressed one of the cases of this bug. It apparently did not fix all cases of this bug.

Since RN is an open source project, you are welcome to debug this issue and contribute your own fix. The project I was working on currently has 80% iOS 10 adoption, so we decided not to continue investigating this issue for iOS 9/8 and have application-specific workarounds that prevent a hard crash for users.

Best of luck in your project.

@worldlyjohn
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronaldheft working on fix now. But FYI we experienced crash on ios10 when users had shared albums and we were doing a getPhotos with type All (that way the loop tries to read a shared album). Not just iOS 9 :/

Please sign in to comment.