Chromium Code Reviews| Index: chrome/browser/android/banners/app_banner_infobar_delegate_android.cc |
| diff --git a/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc b/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc |
| index 1e2e973cbef929860fbd4ce586fd014ffe1bb2ee..cc0f100b0d8dafeab4345fa263423ccbca26dafa 100644 |
| --- a/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc |
| +++ b/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc |
| @@ -37,13 +37,21 @@ using base::android::ConvertUTF16ToJavaString; |
| using base::android::JavaParamRef; |
| using base::android::ScopedJavaLocalRef; |
| +namespace { |
| + |
| +bool isShortcutInfoEmpty(ShortcutInfo* info) { |
|
dominickn
2016/09/01 05:22:00
Nit: IsInfoEmpty(), and it should take a const Sho
Xi Han
2016/09/01 18:44:14
Done.
|
| + return !info || info->url.is_empty(); |
| +} |
| + |
| +} // anonymous namespace |
| + |
| namespace banners { |
| AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid( |
| base::WeakPtr<AppBannerManager> weak_manager, |
| const base::string16& app_title, |
| const GURL& manifest_url, |
| - const content::Manifest& manifest, |
| + std::unique_ptr<ShortcutInfo> info, |
|
dominickn
2016/09/01 05:22:00
Don't change the signature of this method. Instead
Xi Han
2016/09/01 18:44:13
I change the signature because AddToHonmeScreenMan
dominickn
2016/09/02 01:00:58
Acknowledged - nice delegation of constructors. Ho
Xi Han
2016/09/02 13:58:23
I am confused, since that is what I did before. I
dominickn
2016/09/02 14:30:48
Oh, wow, sorry for the water-in-the-brain reviewin
Xi Han
2016/09/02 15:01:50
No worries, just reverted it:)
|
| const GURL& icon_url, |
| std::unique_ptr<SkBitmap> icon, |
| int event_request_id, |
| @@ -51,14 +59,14 @@ AppBannerInfoBarDelegateAndroid::AppBannerInfoBarDelegateAndroid( |
| : weak_manager_(weak_manager), |
| app_title_(app_title), |
| manifest_url_(manifest_url), |
| - manifest_(manifest), |
| + info_(std::move(info)), |
| icon_url_(icon_url), |
| icon_(std::move(icon)), |
| event_request_id_(event_request_id), |
| has_user_interaction_(false), |
| is_webapk_(is_webapk), |
| weak_ptr_factory_(this) { |
| - DCHECK(!manifest.IsEmpty()); |
| + DCHECK(!isShortcutInfoEmpty(info_.get())); |
| CreateJavaDelegate(); |
| } |
| @@ -87,7 +95,7 @@ AppBannerInfoBarDelegateAndroid::~AppBannerInfoBarDelegateAndroid() { |
| if (!has_user_interaction_) { |
| if (!native_app_data_.is_null()) |
| TrackUserResponse(USER_RESPONSE_NATIVE_APP_IGNORED); |
| - else if (!manifest_.IsEmpty()) |
| + else if (!isShortcutInfoEmpty(info_.get())) |
| TrackUserResponse(USER_RESPONSE_WEB_APP_IGNORED); |
| } |
| @@ -154,6 +162,16 @@ void AppBannerInfoBarDelegateAndroid::OnInstallFinished( |
| } |
| } |
| +void AppBannerInfoBarDelegateAndroid::InstallWebApk( |
| + content::WebContents* web_contents) { |
| + if (!web_contents) { |
| + LOG(ERROR) << "Failed to create infobar to install the WebAPK: " |
| + << "the associated WebContents is null."; |
| + return; |
| + } |
| + AcceptWebApk(web_contents); |
| +} |
| + |
| void AppBannerInfoBarDelegateAndroid::CreateJavaDelegate() { |
| JNIEnv* env = base::android::AttachCurrentThread(); |
| java_delegate_.Reset(Java_AppBannerInfoBarDelegateAndroid_create( |
| @@ -197,10 +215,10 @@ void AppBannerInfoBarDelegateAndroid::InfoBarDismissed() { |
| TrackUserResponse(USER_RESPONSE_NATIVE_APP_DISMISSED); |
| AppBannerSettingsHelper::RecordBannerDismissEvent( |
| web_contents, native_app_package_, AppBannerSettingsHelper::NATIVE); |
| - } else if (!manifest_.IsEmpty()) { |
| + } else if (!isShortcutInfoEmpty(info_.get())) { |
| TrackUserResponse(USER_RESPONSE_WEB_APP_DISMISSED); |
| AppBannerSettingsHelper::RecordBannerDismissEvent( |
| - web_contents, manifest_.start_url.spec(), |
| + web_contents, info_->url.spec(), |
| AppBannerSettingsHelper::WEB); |
| } |
| } |
| @@ -260,24 +278,18 @@ bool AppBannerInfoBarDelegateAndroid::AcceptNativeApp( |
| bool AppBannerInfoBarDelegateAndroid::AcceptWebApp( |
| content::WebContents* web_contents) { |
| - if (manifest_.IsEmpty()) |
| + if (isShortcutInfoEmpty(info_.get())) |
| return true; |
| TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED); |
| AppBannerSettingsHelper::RecordBannerInstallEvent( |
| - web_contents, manifest_.start_url.spec(), |
| + web_contents, info_->url.spec(), |
| AppBannerSettingsHelper::WEB); |
| if (weak_manager_) { |
| - ShortcutInfo info(GURL::EmptyGURL()); |
| - info.UpdateFromManifest(manifest_); |
| - info.manifest_url = manifest_url_; |
| - info.icon_url = icon_url_; |
| - info.UpdateSource(ShortcutInfo::SOURCE_APP_BANNER); |
| - |
| const std::string& uid = base::GenerateGUID(); |
| ShortcutHelper::AddToLauncherWithSkBitmap( |
| - web_contents->GetBrowserContext(), info, uid, *icon_.get(), |
| + web_contents->GetBrowserContext(), *info_.get(), uid, *icon_.get(), |
| weak_manager_->FetchWebappSplashScreenImageCallback(uid)); |
| } |
| @@ -287,7 +299,7 @@ bool AppBannerInfoBarDelegateAndroid::AcceptWebApp( |
| bool AppBannerInfoBarDelegateAndroid::AcceptWebApk( |
| content::WebContents* web_contents) { |
| - if (manifest_.IsEmpty()) |
| + if (isShortcutInfoEmpty(info_.get())) |
| return true; |
| JNIEnv* env = base::android::AttachCurrentThread(); |
| @@ -300,15 +312,9 @@ bool AppBannerInfoBarDelegateAndroid::AcceptWebApk( |
| TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED); |
| AppBannerSettingsHelper::RecordBannerInstallEvent( |
| - web_contents, manifest_.start_url.spec(), |
| + web_contents, info_->url.spec(), |
| AppBannerSettingsHelper::WEB); |
| - ShortcutInfo info(GURL::EmptyGURL()); |
| - info.UpdateFromManifest(manifest_); |
| - info.manifest_url = manifest_url_; |
| - info.icon_url = icon_url_; |
| - info.UpdateSource(ShortcutInfo::SOURCE_APP_BANNER); |
| - |
| Java_AppBannerInfoBarDelegateAndroid_setWebApkInstallingState( |
| env, java_delegate_, true); |
| UpdateInstallState(env, nullptr); |
| @@ -318,7 +324,8 @@ bool AppBannerInfoBarDelegateAndroid::AcceptWebApk( |
| weak_ptr_factory_.GetWeakPtr()); |
| DVLOG(1) << "Trigger the installation of the WebAPK."; |
| ShortcutHelper::InstallWebApkWithSkBitmap( |
| - web_contents->GetBrowserContext(), info, *icon_.get(), callback); |
| + web_contents->GetBrowserContext(), *info_.get(), |
| + *icon_.get(), callback); |
| SendBannerAccepted(web_contents, "web"); |
| // Returns false to prevent the infobar from disappearing. |