Patchwork [meta-browser,1/2] chromium: Add PACKAGECONFIG for disabling API keys info bar

login
register
mail settings
Submitter Carlos Rafael Giani
Date July 29, 2014, 10:22 a.m.
Message ID <1406629327-29169-1-git-send-email-dv@pseudoterminal.org>
Download mbox | patch
Permalink /patch/76831/
State New, archived
Headers show

Comments

Carlos Rafael Giani - July 29, 2014, 10:22 a.m.
Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>
---
 .../chromium/0002-Disable-API-keys-info-bar.patch  | 33 ++++++++++++++++++++++
 recipes-browser/chromium/chromium_35.0.1916.114.bb |  1 +
 2 files changed, 34 insertions(+)
 create mode 100644 recipes-browser/chromium/chromium/0002-Disable-API-keys-info-bar.patch
Khem Raj - July 29, 2014, 8:35 p.m.
On Tue, Jul 29, 2014 at 3:22 AM, Carlos Rafael Giani
<dv@pseudoterminal.org> wrote:
> +Upstream-Status: Inappropriate [other]
> +The info bar is not a bug; it is just undesirable in some use cases.
> +A clean approach - which requires discussion with upstream - is to
> +disable it by using a command line option.
> +
> +Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>

I think it would be better to get a suggested solution which is
acceptable upstream.
Otavio Salvador - July 29, 2014, 9:51 p.m.
On Tue, Jul 29, 2014 at 5:35 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Tue, Jul 29, 2014 at 3:22 AM, Carlos Rafael Giani
> <dv@pseudoterminal.org> wrote:
>> +Upstream-Status: Inappropriate [other]
>> +The info bar is not a bug; it is just undesirable in some use cases.
>> +A clean approach - which requires discussion with upstream - is to
>> +disable it by using a command line option.
>> +
>> +Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>
>
> I think it would be better to get a suggested solution which is
> acceptable upstream.

There are customers who don't need to use the Google APIs so they are
not willing to make an API key and instead they prefer to drop the
warning and keep using the Chromium as an application platform. We've
been involved in a couple of projects lately where this happened.

Chromium's upstream advice is to generate an API and use it to deploy;
I think in the meta-browser's case this is sub-optimal as we'll end
having several products and customers using our key and not making
their own ones.
Khem Raj - July 30, 2014, 2:28 a.m.
On Tue, Jul 29, 2014 at 2:51 PM, Otavio Salvador
<otavio@ossystems.com.br> wrote:
> There are customers who don't need to use the Google APIs so they are
> not willing to make an API key and instead they prefer to drop the
> warning and keep using the Chromium as an application platform. We've
> been involved in a couple of projects lately where this happened.
>
> Chromium's upstream advice is to generate an API and use it to deploy;
> I think in the meta-browser's case this is sub-optimal as we'll end
> having several products and customers using our key and not making
> their own ones.

OK then disabling the API key is in essence suppressing the problem. I
am fine if thats
the general usecase but I doubt that
Otavio Salvador - July 30, 2014, 1:02 p.m.
On Tue, Jul 29, 2014 at 11:28 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Tue, Jul 29, 2014 at 2:51 PM, Otavio Salvador
> <otavio@ossystems.com.br> wrote:
>> There are customers who don't need to use the Google APIs so they are
>> not willing to make an API key and instead they prefer to drop the
>> warning and keep using the Chromium as an application platform. We've
>> been involved in a couple of projects lately where this happened.
>>
>> Chromium's upstream advice is to generate an API and use it to deploy;
>> I think in the meta-browser's case this is sub-optimal as we'll end
>> having several products and customers using our key and not making
>> their own ones.
>
> OK then disabling the API key is in essence suppressing the problem. I
> am fine if thats the general usecase but I doubt that

As I explained, this is the requirement for some projects we've been
involved with. The idea here is to share it in a 'reusable' way and
avoid this patch to be kept private for no good reason.

I think it is fine as is.
Khem Raj - July 30, 2014, 5:01 p.m.
On Wed, Jul 30, 2014 at 6:02 AM, Otavio Salvador
<otavio@ossystems.com.br> wrote:
> As I explained, this is the requirement for some projects we've been
> involved with. The idea here is to share it in a 'reusable' way and
> avoid this patch to be kept private for no good reason.
>

than have it in bbappends would solve it.

> I think it is fine as is.

For you yes I understand, do you have say of other folks who use this layer ?
Otavio Salvador - July 30, 2014, 5:20 p.m.
On Wed, Jul 30, 2014 at 2:01 PM, Khem Raj <raj.khem@gmail.com> wrote:
> On Wed, Jul 30, 2014 at 6:02 AM, Otavio Salvador
> <otavio@ossystems.com.br> wrote:
>> As I explained, this is the requirement for some projects we've been
>> involved with. The idea here is to share it in a 'reusable' way and
>> avoid this patch to be kept private for no good reason.
>
> than have it in bbappends would solve it.

Yes; it does. But duplicated among all users doing it (in our case 2 already).

>> I think it is fine as is.
>
> For you yes I understand, do you have say of other folks who use this layer ?

No; only you.
Martin Jansa - July 30, 2014, 5:23 p.m.
On Wed, Jul 30, 2014 at 10:01:42AM -0700, Khem Raj wrote:
> On Wed, Jul 30, 2014 at 6:02 AM, Otavio Salvador
> <otavio@ossystems.com.br> wrote:
> > As I explained, this is the requirement for some projects we've been
> > involved with. The idea here is to share it in a 'reusable' way and
> > avoid this patch to be kept private for no good reason.
> >
> 
> than have it in bbappends would solve it.

I agree with Otavio, that having just PACKAGECONFIG switch in .bbappend
or distro config is simpler than reinventing this .patch in each layer
which needs that.

> 
> > I think it is fine as is.
> 
> For you yes I understand, do you have say of other folks who use this layer ?

He only adds the option, it even isn't enabled by default, so FWIW I'm
fine with this.
Khem Raj - July 30, 2014, 5:29 p.m.
On Wed, Jul 30, 2014 at 10:23 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
>> For you yes I understand, do you have say of other folks who use this layer ?
>
> He only adds the option, it even isn't enabled by default, so FWIW I'm
> fine with this.
>

oh its not default. That was an oversight from me. In this case its ok.
Otavio Salvador - Aug. 1, 2014, 12:56 p.m.
On Tue, Jul 29, 2014 at 7:22 AM, Carlos Rafael Giani
<dv@pseudoterminal.org> wrote:
> Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>

Applied.

Patch

diff --git a/recipes-browser/chromium/chromium/0002-Disable-API-keys-info-bar.patch b/recipes-browser/chromium/chromium/0002-Disable-API-keys-info-bar.patch
new file mode 100644
index 0000000..5b5962a
--- /dev/null
+++ b/recipes-browser/chromium/chromium/0002-Disable-API-keys-info-bar.patch
@@ -0,0 +1,33 @@ 
+From e3a694a5324b9fab58875f2a1bd4cbe48fe6e151 Mon Sep 17 00:00:00 2001
+From: Carlos Rafael Giani <dv@pseudoterminal.org>
+Date: Mon, 9 Jun 2014 15:05:11 +0200
+Subject: [PATCH] Disable API keys info bar
+
+Upstream-Status: Inappropriate [other]
+The info bar is not a bug; it is just undesirable in some use cases.
+A clean approach - which requires discussion with upstream - is to
+disable it by using a command line option.
+
+Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>
+---
+ chrome/browser/ui/startup/google_api_keys_infobar_delegate.cc | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/chrome/browser/ui/startup/google_api_keys_infobar_delegate.cc b/chrome/browser/ui/startup/google_api_keys_infobar_delegate.cc
+index 5014219..bb2a39d 100644
+--- a/chrome/browser/ui/startup/google_api_keys_infobar_delegate.cc
++++ b/chrome/browser/ui/startup/google_api_keys_infobar_delegate.cc
+@@ -18,8 +18,10 @@ void GoogleApiKeysInfoBarDelegate::Create(InfoBarService* infobar_service) {
+   if (google_apis::HasKeysConfigured())
+     return;
+ 
++#if 0
+   infobar_service->AddInfoBar(ConfirmInfoBarDelegate::CreateInfoBar(
+       scoped_ptr<ConfirmInfoBarDelegate>(new GoogleApiKeysInfoBarDelegate())));
++#endif
+ }
+ 
+ GoogleApiKeysInfoBarDelegate::GoogleApiKeysInfoBarDelegate()
+-- 
+1.8.3.2
+
diff --git a/recipes-browser/chromium/chromium_35.0.1916.114.bb b/recipes-browser/chromium/chromium_35.0.1916.114.bb
index cab3782..c1021c0 100644
--- a/recipes-browser/chromium/chromium_35.0.1916.114.bb
+++ b/recipes-browser/chromium/chromium_35.0.1916.114.bb
@@ -8,6 +8,7 @@  SRC_URI = "\
         file://oe-defaults.gypi \
         ${@bb.utils.contains('PACKAGECONFIG', 'component-build', 'file://component-build.gypi', '', d)} \
         ${@bb.utils.contains('PACKAGECONFIG', 'ignore-lost-context', 'file://remove-linux-accel-canvas-from-blacklist.patch', '', d)} \
+        ${@bb.utils.contains('PACKAGECONFIG', 'disable-api-keys-info-bar', 'file://0002-Disable-API-keys-info-bar.patch', '', d)} \
         file://unistd-2.patch \
         file://fix-glib-deprecated-warning.patch \
         file://google-chrome \