Change tests to prepare for turning on the permission bubble flag.

Change tests to prepare for turning on the permission bubble flag.

[Greg Billock, 2014-07-22 23:09:49]

Please take a look -- these tests aren't on quite yet, but they basically
translate tests to the permission bubble format in preparation for turning on
the flag.

For geolocation, I think there is still a problem: I'm not sure how the
Off-the-record leak prevention is supposed to be enforced, but I'm suspicious
that it doesn't happen for bubbles, and I'm not sure why. Tim, would you mind
explaining that in a bit more detail? I suspect there's a profile check
somewhere that ought to be moved somewhere else.

[leng, 2014-07-23 16:29:11]

lgtm

Thanks for doing the conversions!

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:42:
PermissionBubbleManager::CreateForWebContents(web_contents());
Is this a no-op for !PermissionBubbleManager::Enabled()?

[timvolodine, 2014-07-24 11:25:54]

I am not very familiar with bubble leak prevention, maybe Michael knows more..
+mvanouwerkerk@

[Miguel Garcia, 2014-07-25 12:17:35]

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:111: if
(!PermissionBubbleManager::Enabled()) {
Can you create a Respond(bool value)  method that does infobars or bubbles and
then just use it in both tests in stead of having one if per test?

[Michael van Ouwerkerk, 2014-07-25 18:34:27]

What kind of leak prevention do you mean? Forgetting a permission when the
incognito profile is closed? The GeolocationPermissionContext is not aware of
whether or not the current profile is a OffTheRecordProfile. It just uses its
HostContentSettingsMap to call SetContentSetting on it.

[Greg Billock, 2014-07-29 16:37:05]

On 2014/07/25 18:34:27, Michael van Ouwerkerk wrote:
> What kind of leak prevention do you mean? Forgetting a permission when the
> incognito profile is closed? The GeolocationPermissionContext is not aware of
> whether or not the current profile is a OffTheRecordProfile. It just uses its
> HostContentSettingsMap to call SetContentSetting on it.

That makes sense, but the test is failing and I can't figure out why -- as
nearly as I can tell the bubble does exactly the same thing the infobar does.
Something's different, but I'm not sure if its a hole in the implementation or
something missing in the test.

[Greg Billock, 2014-07-29 16:44:14]

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:42:
PermissionBubbleManager::CreateForWebContents(web_contents());
On 2014/07/23 16:29:10, leng wrote:
> Is this a no-op for !PermissionBubbleManager::Enabled()?

Not quite a no-op, but shouldn't interfere.

https://codereview.chromium.org/411503005/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:111: if
(!PermissionBubbleManager::Enabled()) {
On 2014/07/25 12:17:35, Miguel Garcia wrote:
> Can you create a Respond(bool value)  method that does infobars or bubbles and
> then just use it in both tests in stead of having one if per test?

Done.

[Michael van Ouwerkerk, 2014-08-01 15:43:41]

On 2014/07/29 16:37:05, Greg Billock wrote:
> On 2014/07/25 18:34:27, Michael van Ouwerkerk wrote:
> > What kind of leak prevention do you mean? Forgetting a permission when the
> > incognito profile is closed? The GeolocationPermissionContext is not aware
of
> > whether or not the current profile is a OffTheRecordProfile. It just uses
its
> > HostContentSettingsMap to call SetContentSetting on it.
> 
> That makes sense, but the test is failing and I can't figure out why -- as
> nearly as I can tell the bubble does exactly the same thing the infobar does.
> Something's different, but I'm not sure if its a hole in the implementation or
> something missing in the test.

I'm not quite sure what you are asking here. Can you link to the unexplained
failure?

[Greg Billock, 2014-08-01 20:15:58]

On 2014/08/01 15:43:41, Michael van Ouwerkerk wrote:
> On 2014/07/29 16:37:05, Greg Billock wrote:
> > On 2014/07/25 18:34:27, Michael van Ouwerkerk wrote:
> > > What kind of leak prevention do you mean? Forgetting a permission when the
> > > incognito profile is closed? The GeolocationPermissionContext is not aware
> of
> > > whether or not the current profile is a OffTheRecordProfile. It just uses
> its
> > > HostContentSettingsMap to call SetContentSetting on it.
> > 
> > That makes sense, but the test is failing and I can't figure out why -- as
> > nearly as I can tell the bubble does exactly the same thing the infobar
does.
> > Something's different, but I'm not sure if its a hole in the implementation
or
> > something missing in the test.
> 
> I'm not quite sure what you are asking here. Can you link to the unexplained
> failure?

Actually now after syncing it isn't failing any more. Not sure what happened --
I suspect it may be flaky and need some adjustment.

[Greg Billock, 2014-08-07 21:38:07]

On 2014/08/01 20:15:58, Greg Billock wrote:
> On 2014/08/01 15:43:41, Michael van Ouwerkerk wrote:
> > On 2014/07/29 16:37:05, Greg Billock wrote:
> > > On 2014/07/25 18:34:27, Michael van Ouwerkerk wrote:
> > > > What kind of leak prevention do you mean? Forgetting a permission when
the
> > > > incognito profile is closed? The GeolocationPermissionContext is not
aware
> > of
> > > > whether or not the current profile is a OffTheRecordProfile. It just
uses
> > its
> > > > HostContentSettingsMap to call SetContentSetting on it.
> > > 
> > > That makes sense, but the test is failing and I can't figure out why -- as
> > > nearly as I can tell the bubble does exactly the same thing the infobar
> does.
> > > Something's different, but I'm not sure if its a hole in the
implementation
> or
> > > something missing in the test.
> > 
> > I'm not quite sure what you are asking here. Can you link to the unexplained
> > failure?
> 
> Actually now after syncing it isn't failing any more. Not sure what happened
--
> I suspect it may be flaky and need some adjustment.

ping

[Michael van Ouwerkerk, 2014-08-07 21:58:59]

lgtm

[Greg Billock, 2014-08-13 21:24:45]

The CQ bit was checked by gbillock@chromium.org

[Greg Billock, 2014-08-13 21:26:31]

On 2014/08/13 21:24:45, Greg Billock wrote:
> The CQ bit was checked by mailto:gbillock@chromium.org

Added bauerb and hajdan for owners. Can you have a look?

[commit-bot: I haz the power, 2014-08-13 21:27:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gbillock@chromium.org/411503005/80001

[Bernhard Bauer, 2014-08-13 21:30:31]

content settings and WebUI LGTM

https://codereview.chromium.org/411503005/diff/80001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://codereview.chromium.org/411503005/diff/80001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:33: id, url,
url, accept, accept);
You could return here to remove a level of indentation below.

https://codereview.chromium.org/411503005/diff/80001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:148:
RespondToPermission(false);
Missing parameters?

[Greg Billock, 2014-08-13 22:12:23]

https://codereview.chromium.org/411503005/diff/80001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context_base_unittest.cc
(right):

https://codereview.chromium.org/411503005/diff/80001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:33: id, url,
url, accept, accept);
On 2014/08/13 21:30:31, Bernhard Bauer wrote:
> You could return here to remove a level of indentation below.

Done.

https://codereview.chromium.org/411503005/diff/80001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context_base_unittest.cc:148:
RespondToPermission(false);
On 2014/08/13 21:30:31, Bernhard Bauer wrote:
> Missing parameters?

yeah, I thought I compiled this, but obviously not -- I'm trying to peel CLs off
a working set of code that addressed many tests...

[Greg Billock, 2014-08-13 22:12:30]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-08-13 22:14:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gbillock@chromium.org/411503005/100001

[commit-bot: I haz the power, 2014-08-14 04:21:34]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-14 04:29:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-14 04:29:46]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Paweł Hajdan Jr., 2014-08-14 11:28:57]

Adding Randy and Asanka to review the downloads changes. They are not obvious to
me.

[asanka, 2014-08-14 15:43:49]

Can you add a bug annotation?

[asanka, 2014-08-14 15:49:31]

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
File chrome/browser/download/download_permission_request.cc (right):

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
chrome/browser/download/download_permission_request.cc:35: return true;
Why is it necessary to invert this? Downloads that are initiated due to an
explicit user requests ('Save as' etc...) don't make it here. The ones that are
throttled are those that are considered "content initiated." I.e. the decision
to download was made by the content rather than the browser. Even if there was a
user gesture associated with it, it's not clear whether the gesture was
unambiguously indicating consent to download.

[Greg Billock, 2014-08-18 22:28:12]

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
File chrome/browser/download/download_permission_request.cc (right):

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
chrome/browser/download/download_permission_request.cc:35: return true;
On 2014/08/14 15:49:31, asanka wrote:
> Why is it necessary to invert this? Downloads that are initiated due to an
> explicit user requests ('Save as' etc...) don't make it here. The ones that
are
> throttled are those that are considered "content initiated." I.e. the decision
> to download was made by the content rather than the browser. Even if there was
a
> user gesture associated with it, it's not clear whether the gesture was
> unambiguously indicating consent to download.

It isn't clear what we want to do here. I think that's more your department to
figure out. There's no simple way to propagate the signal, so it's something
that if you want policy coherency on, will have to be done upstream. "return
false" is conservative, so that's what I put when I mocked out the original
client code. In practice, now that its going to turn on, it needs to return true
so things actually work.

[asanka, 2014-08-22 20:56:48]

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
File chrome/browser/download/download_permission_request.cc (right):

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
chrome/browser/download/download_permission_request.cc:35: return true;
On 2014/08/18 22:28:12, Greg Billock wrote:
> On 2014/08/14 15:49:31, asanka wrote:
> > Why is it necessary to invert this? Downloads that are initiated due to an
> > explicit user requests ('Save as' etc...) don't make it here. The ones that
> are
> > throttled are those that are considered "content initiated." I.e. the
decision
> > to download was made by the content rather than the browser. Even if there
was
> a
> > user gesture associated with it, it's not clear whether the gesture was
> > unambiguously indicating consent to download.
> 
> It isn't clear what we want to do here. I think that's more your department to
> figure out. There's no simple way to propagate the signal, so it's something
> that if you want policy coherency on, will have to be done upstream. "return
> false" is conservative, so that's what I put when I mocked out the original
> client code. In practice, now that its going to turn on, it needs to return
true
> so things actually work.

Depends on how it's used. It seems that if any permission request has a user
action then the permission bubble is displayed?

Plumbing the actual user gesture flag up here isn't too hard. You can query it
on the IO thread at the time DownloadRequestLimiter::CanDownloadOnIOThread() is
called using content::ResourceRequestInfo::HasUserGesture().

[Greg Billock, 2014-08-26 21:02:26]

On 2014/08/22 20:56:48, asanka wrote:
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> File chrome/browser/download/download_permission_request.cc (right):
> 
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> chrome/browser/download/download_permission_request.cc:35: return true;
> On 2014/08/18 22:28:12, Greg Billock wrote:
> > On 2014/08/14 15:49:31, asanka wrote:
> > > Why is it necessary to invert this? Downloads that are initiated due to an
> > > explicit user requests ('Save as' etc...) don't make it here. The ones
that
> > are
> > > throttled are those that are considered "content initiated." I.e. the
> decision
> > > to download was made by the content rather than the browser. Even if there
> was
> > a
> > > user gesture associated with it, it's not clear whether the gesture was
> > > unambiguously indicating consent to download.
> > 
> > It isn't clear what we want to do here. I think that's more your department
to
> > figure out. There's no simple way to propagate the signal, so it's something
> > that if you want policy coherency on, will have to be done upstream. "return
> > false" is conservative, so that's what I put when I mocked out the original
> > client code. In practice, now that its going to turn on, it needs to return
> true
> > so things actually work.
> 
> Depends on how it's used. It seems that if any permission request has a user
> action then the permission bubble is displayed?

That's right. Meaning that requests stack up, but if one has a user gesture,
then they'll all be coalesced.

> Plumbing the actual user gesture flag up here isn't too hard. You can query it
> on the IO thread at the time DownloadRequestLimiter::CanDownloadOnIOThread()
is
> called using content::ResourceRequestInfo::HasUserGesture().

That definitely sounds like the best plan. Will that be accurate for multiple
downloads? If so, shall I do that here or leave for a later bug/cl if you want
to do it?

[asanka, 2014-08-26 22:50:01]

On 2014/08/26 21:02:26, Greg Billock wrote:
> On 2014/08/22 20:56:48, asanka wrote:
> >
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> > File chrome/browser/download/download_permission_request.cc (right):
> > 
> >
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> > chrome/browser/download/download_permission_request.cc:35: return true;
> > On 2014/08/18 22:28:12, Greg Billock wrote:
> > > On 2014/08/14 15:49:31, asanka wrote:
> > > > Why is it necessary to invert this? Downloads that are initiated due to
an
> > > > explicit user requests ('Save as' etc...) don't make it here. The ones
> that
> > > are
> > > > throttled are those that are considered "content initiated." I.e. the
> > decision
> > > > to download was made by the content rather than the browser. Even if
there
> > was
> > > a
> > > > user gesture associated with it, it's not clear whether the gesture was
> > > > unambiguously indicating consent to download.
> > > 
> > > It isn't clear what we want to do here. I think that's more your
department
> to
> > > figure out. There's no simple way to propagate the signal, so it's
something
> > > that if you want policy coherency on, will have to be done upstream.
"return
> > > false" is conservative, so that's what I put when I mocked out the
original
> > > client code. In practice, now that its going to turn on, it needs to
return
> > true
> > > so things actually work.
> > 
> > Depends on how it's used. It seems that if any permission request has a user
> > action then the permission bubble is displayed?
> 
> That's right. Meaning that requests stack up, but if one has a user gesture,
> then they'll all be coalesced.
> 
> > Plumbing the actual user gesture flag up here isn't too hard. You can query
it
> > on the IO thread at the time DownloadRequestLimiter::CanDownloadOnIOThread()
> is
> > called using content::ResourceRequestInfo::HasUserGesture().
> 
> That definitely sounds like the best plan. Will that be accurate for multiple
> downloads? If so, shall I do that here or leave for a later bug/cl if you want
> to do it?

DownloadRequestLimiter is per-download. Each instance will have a somewhat
accurate flag indicating whether there's an associated user gesture.

It would be great if you could do the plumbing here since there's a good chance
it won't happen if we leave it for later.

[Greg Billock, 2014-09-05 17:41:05]

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
File chrome/browser/download/download_permission_request.cc (right):

https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
chrome/browser/download/download_permission_request.cc:35: return true;
On 2014/08/22 20:56:47, asanka wrote:
> On 2014/08/18 22:28:12, Greg Billock wrote:
> > On 2014/08/14 15:49:31, asanka wrote:
> > > Why is it necessary to invert this? Downloads that are initiated due to an
> > > explicit user requests ('Save as' etc...) don't make it here. The ones
that
> > are
> > > throttled are those that are considered "content initiated." I.e. the
> decision
> > > to download was made by the content rather than the browser. Even if there
> was
> > a
> > > user gesture associated with it, it's not clear whether the gesture was
> > > unambiguously indicating consent to download.
> > 
> > It isn't clear what we want to do here. I think that's more your department
to
> > figure out. There's no simple way to propagate the signal, so it's something
> > that if you want policy coherency on, will have to be done upstream. "return
> > false" is conservative, so that's what I put when I mocked out the original
> > client code. In practice, now that its going to turn on, it needs to return
> true
> > so things actually work.
> 
> Depends on how it's used. It seems that if any permission request has a user
> action then the permission bubble is displayed?
> 
> Plumbing the actual user gesture flag up here isn't too hard. You can query it
> on the IO thread at the time DownloadRequestLimiter::CanDownloadOnIOThread()
is
> called using content::ResourceRequestInfo::HasUserGesture().

I looked into this, and I can't connect the dots from CanDownloadOnIOThread to a
ResourceRequestInfo object, or anything I think I have that connects to it.

But if this is only handling non-user-gestured requests, then the answer is
always false and I shouldn't change it. I'll just figure out another way to
drive the test.

[Greg Billock, 2014-09-17 21:10:44]

On 2014/09/05 17:41:05, Greg Billock wrote:
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> File chrome/browser/download/download_permission_request.cc (right):
> 
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> chrome/browser/download/download_permission_request.cc:35: return true;
> On 2014/08/22 20:56:47, asanka wrote:
> > On 2014/08/18 22:28:12, Greg Billock wrote:
> > > On 2014/08/14 15:49:31, asanka wrote:
> > > > Why is it necessary to invert this? Downloads that are initiated due to
an
> > > > explicit user requests ('Save as' etc...) don't make it here. The ones
> that
> > > are
> > > > throttled are those that are considered "content initiated." I.e. the
> > decision
> > > > to download was made by the content rather than the browser. Even if
there
> > was
> > > a
> > > > user gesture associated with it, it's not clear whether the gesture was
> > > > unambiguously indicating consent to download.
> > > 
> > > It isn't clear what we want to do here. I think that's more your
department
> to
> > > figure out. There's no simple way to propagate the signal, so it's
something
> > > that if you want policy coherency on, will have to be done upstream.
"return
> > > false" is conservative, so that's what I put when I mocked out the
original
> > > client code. In practice, now that its going to turn on, it needs to
return
> > true
> > > so things actually work.
> > 
> > Depends on how it's used. It seems that if any permission request has a user
> > action then the permission bubble is displayed?
> > 
> > Plumbing the actual user gesture flag up here isn't too hard. You can query
it
> > on the IO thread at the time DownloadRequestLimiter::CanDownloadOnIOThread()
> is
> > called using content::ResourceRequestInfo::HasUserGesture().
> 
> I looked into this, and I can't connect the dots from CanDownloadOnIOThread to
a
> ResourceRequestInfo object, or anything I think I have that connects to it.
> 
> But if this is only handling non-user-gestured requests, then the answer is
> always false and I shouldn't change it. I'll just figure out another way to
> drive the test.

Anything more you think we should do here? Or can we get this in? I don't think
we'll get the experiment on for M39, so we'll have some time in the next cycle
to vet it.

[Greg Billock, 2014-10-09 16:51:41]

On 2014/09/17 21:10:44, Greg Billock wrote:
> On 2014/09/05 17:41:05, Greg Billock wrote:
> >
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> > File chrome/browser/download/download_permission_request.cc (right):
> > 
> >
>
https://codereview.chromium.org/411503005/diff/100001/chrome/browser/download...
> > chrome/browser/download/download_permission_request.cc:35: return true;
> > On 2014/08/22 20:56:47, asanka wrote:
> > > On 2014/08/18 22:28:12, Greg Billock wrote:
> > > > On 2014/08/14 15:49:31, asanka wrote:
> > > > > Why is it necessary to invert this? Downloads that are initiated due
to
> an
> > > > > explicit user requests ('Save as' etc...) don't make it here. The ones
> > that
> > > > are
> > > > > throttled are those that are considered "content initiated." I.e. the
> > > decision
> > > > > to download was made by the content rather than the browser. Even if
> there
> > > was
> > > > a
> > > > > user gesture associated with it, it's not clear whether the gesture
was
> > > > > unambiguously indicating consent to download.
> > > > 
> > > > It isn't clear what we want to do here. I think that's more your
> department
> > to
> > > > figure out. There's no simple way to propagate the signal, so it's
> something
> > > > that if you want policy coherency on, will have to be done upstream.
> "return
> > > > false" is conservative, so that's what I put when I mocked out the
> original
> > > > client code. In practice, now that its going to turn on, it needs to
> return
> > > true
> > > > so things actually work.
> > > 
> > > Depends on how it's used. It seems that if any permission request has a
user
> > > action then the permission bubble is displayed?
> > > 
> > > Plumbing the actual user gesture flag up here isn't too hard. You can
query
> it
> > > on the IO thread at the time
DownloadRequestLimiter::CanDownloadOnIOThread()
> > is
> > > called using content::ResourceRequestInfo::HasUserGesture().
> > 
> > I looked into this, and I can't connect the dots from CanDownloadOnIOThread
to
> a
> > ResourceRequestInfo object, or anything I think I have that connects to it.
> > 
> > But if this is only handling non-user-gestured requests, then the answer is
> > always false and I shouldn't change it. I'll just figure out another way to
> > drive the test.
> 
> Anything more you think we should do here? Or can we get this in? I don't
think
> we'll get the experiment on for M39, so we'll have some time in the next cycle
> to vet it.

ping

[asanka, 2014-10-10 21:07:03]

lgtm

[markusheintz_, 2014-10-16 16:08:18]

On 2014/10/10 21:07:03, asanka wrote:
> lgtm

LGTM

[felt, 2014-11-05 03:02:29]

On 2014/10/16 16:08:18, markusheintz_ wrote:
> On 2014/10/10 21:07:03, asanka wrote:
> > lgtm
> 
> LGTM

what remains on this bug besides fixing the compile errors?

[Greg Billock, 2014-11-05 05:11:04]

There's a new scheme for handling the permissions requests that Miguel
Garcia made. It may be easier to just integrate with that than to follow
client code around. I haven't solved that question yet though.

On Tue, Nov 4, 2014 at 7:02 PM, <felt@chromium.org> wrote:

> On 2014/10/16 16:08:18, markusheintz_ wrote:
>
>> On 2014/10/10 21:07:03, asanka wrote:
>> > lgtm
>>
>
>  LGTM
>>
>
> what remains on this bug besides fixing the compile errors?
>
> https://codereview.chromium.org/411503005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Michael van Ouwerkerk, 2014-11-20 11:02:24]

mvanouwerkerk@chromium.org changed reviewers:
- mvanouwerkerk@chromium.org

[markusheintz_, 2015-04-14 08:53:48]

On 2014/11/05 05:11:04, Greg Billock wrote:
> There's a new scheme for handling the permissions requests that Miguel
> Garcia made. It may be easier to just integrate with that than to follow
> client code around. I haven't solved that question yet though.
> 
> On Tue, Nov 4, 2014 at 7:02 PM, <mailto:felt@chromium.org> wrote:
> 
> > On 2014/10/16 16:08:18, markusheintz_ wrote:
> >
> >> On 2014/10/10 21:07:03, asanka wrote:
> >> > lgtm
> >>
> >
> >  LGTM
> >>
> >
> > what remains on this bug besides fixing the compile errors?
> >
> > <crxref style="background-image:
url(chrome-extension://ppnlfijacbhfpjgkafbeiaklmgoljooo/res/type/codereview.png);">https://codereview.chromium.org/411503005/</crxref>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Do we still need this CL or can we close it?

[Greg Billock, 2015-04-14 15:45:15]

I think I handed this cl off, but I haven't tracked it through to know what
happened -- my guess is it is obsolete.


On Tue, Apr 14, 2015 at 1:53 AM, <markusheintz@chromium.org> wrote:

> On 2014/11/05 05:11:04, Greg Billock wrote:
>
>> There's a new scheme for handling the permissions requests that Miguel
>> Garcia made. It may be easier to just integrate with that than to follow
>> client code around. I haven't solved that question yet though.
>>
>
>  On Tue, Nov 4, 2014 at 7:02 PM, <mailto:felt@chromium.org> wrote:
>>
>
>  > On 2014/10/16 16:08:18, markusheintz_ wrote:
>> >
>> >> On 2014/10/10 21:07:03, asanka wrote:
>> >> > lgtm
>> >>
>> >
>> >  LGTM
>> >>
>> >
>> > what remains on this bug besides fixing the compile errors?
>> >
>> > <crxref style="background-image:
>>
> url(chrome-extension://ppnlfijacbhfpjgkafbeiaklmgoljo
> oo/res/type/codereview.png);">https://codereview.chromium.org/411503005/
> </crxref>
>
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> Do we still need this CL or can we close it?
>
> https://codereview.chromium.org/411503005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[felt, 2015-04-14 15:46:10]

On 2015/04/14 15:45:15, Greg Billock wrote:
> I think I handed this cl off, but I haven't tracked it through to know what
> happened -- my guess is it is obsolete.
> 
> 
> On Tue, Apr 14, 2015 at 1:53 AM, <mailto:markusheintz@chromium.org> wrote:
> 
> > On 2014/11/05 05:11:04, Greg Billock wrote:
> >
> >> There's a new scheme for handling the permissions requests that Miguel
> >> Garcia made. It may be easier to just integrate with that than to follow
> >> client code around. I haven't solved that question yet though.
> >>
> >
> >  On Tue, Nov 4, 2014 at 7:02 PM, <mailto:felt@chromium.org> wrote:
> >>
> >
> >  > On 2014/10/16 16:08:18, markusheintz_ wrote:
> >> >
> >> >> On 2014/10/10 21:07:03, asanka wrote:
> >> >> > lgtm
> >> >>
> >> >
> >> >  LGTM
> >> >>
> >> >
> >> > what remains on this bug besides fixing the compile errors?
> >> >
> >> > <crxref style="background-image:
> >>
> > url(chrome-extension://ppnlfijacbhfpjgkafbeiaklmgoljo
> > oo/res/type/codereview.png);">https://codereview.chromium.org/411503005/
> > </crxref>
> >
> >> >
> >>
> >
> >  To unsubscribe from this group and stop receiving emails from it, send an
> >>
> > email
> >
> >> to mailto:chromium-reviews+unsubscribe@chromium.org.
> >>
> >
> > Do we still need this CL or can we close it?
> >
> > https://codereview.chromium.org/411503005/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Yes, this CL is obsolete now -- tests were updated in subsequent CLs.