`extension.isAllowed???Access` samples

`extension.isAllowed???Access` samples

Sample extension for the `extension.isAllowed???Access` APIs, and adding
the functionality to the proxy sample extension.

BUG=76494, 74694
TEST=None

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

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

[Mike West, 2011-03-17 11:58:38]

Hey Arne, I've put together a tiny API sample for the new
`extension.isAllowed???Access` bits (see http://crbug.com/74694), and tweaked
the proxy_configuration extension to test for incognito mode permissions before
displaying the incognito options.

WDYT?

[kurrik.chromium, 2011-03-21 22:12:44]

Yeah, this is cool.  Just a few suggestions on ways I think it could be
improved.  Also, if you change code in an existing sample, I'd prefer that you
bump the version up in the manifest as well.

~Arne


On 2011/03/17 11:58:38, Mike West wrote:
> Hey Arne, I've put together a tiny API sample for the new
> `extension.isAllowed???Access` bits (see http://crbug.com/74694), and tweaked
> the proxy_configuration extension to test for incognito mode permissions
before
> displaying the incognito options.
> 
> WDYT?

[kurrik.chromium, 2011-03-21 22:13:02]

Oops, didn't publish comments.

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
File
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html
(right):

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html:16:
<li><p><span>1</span> chrome.extension.isAllowedFileSchemeAccess: <code
id="file">unknown</code> (unpacked extensions always have file scheme access,
you'll need to install this as a packed extension to toggle it
properly)</p></li>
I didn't even know this.  Interesting.

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html:22:
document.getElementById('file').textContent = state ? 'true': 'false';
Maybe toggle a class name here and style true/false as green/red instead of both
as yellow?  Just to make things a bit more obvious.

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
File
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js
(right):

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js:685:
button.style.display = this.isAllowedIncognitoAccess_ ? 'block' : 'none';
This button now wraps to another line.  I think I preferred it inline.

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js:686:

I think you could also add some UI indicating how to enable the extension's
incognito functionality.  Seems to be in the spirit of this function.

[Mike West, 2011-03-22 08:44:56]

I've made some changes based on your comments, and updated the manifest.  WDYT?

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
File
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html
(right):

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html:22:
document.getElementById('file').textContent = state ? 'true': 'false';
On 2011/03/21 22:13:02, kurrik.chromium wrote:
> Maybe toggle a class name here and style true/false as green/red instead of
both
> as yellow?  Just to make things a bit more obvious.

Done.

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
File
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js
(right):

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js:685:
button.style.display = this.isAllowedIncognitoAccess_ ? 'block' : 'none';
On 2011/03/21 22:13:02, kurrik.chromium wrote:
> This button now wraps to another line.  I think I preferred it inline.

Done.

http://codereview.chromium.org/6677108/diff/1/chrome/common/extensions/docs/e...
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js:686:

Fair enough.  I've reworked this section entirely; I think you're right, it's
better with a message.

[kurrik.chromium, 2011-03-22 18:22:13]

LGTM.  I'd really prefer not to use innerHTML in samples unless absolutely
necessary, but honestly if someone is reading this much code and coming away
with that habit out of everything, then there's probably not much hope anyway,
huh.

http://codereview.chromium.org/6677108/diff/8001/chrome/common/extensions/doc...
File
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js
(right):

http://codereview.chromium.org/6677108/diff/8001/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js:664:
var msg = "<p>I'm sorry, Dave, I'm afraid<br>I can't do that.</p>" +
To be honest, not my favorite approach in a sample.  Since you're not putting in
dynamic/untrusted input though I won't block on it though.

[Mike West, 2011-03-22 19:58:51]

Fair point.  I'll change it.

http://codereview.chromium.org/6677108/diff/8001/chrome/common/extensions/doc...
File
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js
(right):

http://codereview.chromium.org/6677108/diff/8001/chrome/common/extensions/doc...
chrome/common/extensions/docs/examples/extensions/proxy_configuration/proxy_form_controller.js:664:
var msg = "<p>I'm sorry, Dave, I'm afraid<br>I can't do that.</p>" +
On 2011/03/22 18:22:13, kurrik.chromium wrote:
> To be honest, not my favorite approach in a sample.  Since you're not putting
in
> dynamic/untrusted input though I won't block on it though.

No, that's a very fair point.  I only used it because I completely control the
incoming text, but it's a bad habit.  I'll rework this bit.

[Mike West, 2011-03-23 08:44:46]

I've dropped `innerHTML` and regenerated the documentation after rebasing on top
of origin/trunk.  If you're still happy with the code, I'd appreciate you
committing it for me.

Thanks!

-Mike

[kurrik.chromium, 2011-03-29 20:41:30]

Can you update the issue with the following whitespace fix?  Patch doesn't apply
cleanly with git cl patch, and I don't get zips when I download the patch
manually.

http://codereview.chromium.org/6677108/diff/13003/chrome/common/extensions/do...
File
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html
(right):

http://codereview.chromium.org/6677108/diff/13003/chrome/common/extensions/do...
chrome/common/extensions/docs/examples/api/extension/isAllowedAccess/popup.html:17:
<span>1</span> chrome.extension.isAllowedFileSchemeAccess:
Trailing whitespace on this line breaks cl patch

[Mike West, 2011-03-29 21:07:05]

On 2011/03/29 20:41:30, kurrik.chromium wrote:
> Can you update the issue with the following whitespace fix?  Patch doesn't
apply
> cleanly with git cl patch, and I don't get zips when I download the patch
> manually.

Fixed the whitespace, and regenerated the zips/samples.json after rebasing onto
trunk.

`git cl patch` is broken, as far as I know.  It doesn't pull down binary files
period; I think you'd be missing the zip and png files.  Last I heard,
Marc-Antonie knew about the bug, and was "still writing code to take care of
that."

[kurrik.chromium, 2011-04-01 00:29:58]

Hm, can you just send me a zip of modified files, then?

You should also go in for provisional committer status after this ;)

~Arne


On Tue, Mar 29, 2011 at 2:07 PM, <mkwst@chromium.org> wrote:

> On 2011/03/29 20:41:30, kurrik.chromium wrote:
>
>> Can you update the issue with the following whitespace fix?  Patch doesn't
>>
> apply
>
>> cleanly with git cl patch, and I don't get zips when I download the patch
>> manually.
>>
>
> Fixed the whitespace, and regenerated the zips/samples.json after rebasing
> onto
> trunk.
>
> `git cl patch` is broken, as far as I know.  It doesn't pull down binary
> files
> period; I think you'd be missing the zip and png files.  Last I heard,
> Marc-Antonie knew about the bug, and was "still writing code to take care
> of
> that."
>
>
> http://codereview.chromium.org/6677108/
>

[Mike West, 2011-04-04 08:57:37]

On 2011/04/01 00:29:58, kurrik.chromium wrote:

> You should also go in for provisional committer status after this ;)

That'll be an OKR.

Regardless, this is becoming a pain in the ass for you. I'm going to try to
commit it via the commit queue, which I've just discovered. I'll try to get an
LGTM quickly from an engineer here so that I don't have to regenerate
documentation _again_ in case something changes overnight. :)

[commit-bot: I haz the power, 2011-04-04 12:01:48]

Can't process patch for file
chrome/common/extensions/docs/examples/extensions/proxy_configuration.zip.
File's status is None, patchset upload is incomplete

[commit-bot: I haz the power, 2011-04-04 12:02:35]

Can't process patch for file
chrome/common/extensions/docs/examples/extensions/proxy_configuration.zip.
File's status is None, patchset upload is incomplete

[Mike West, 2011-04-04 12:12:31]

On 2011/04/04 12:01:48, commit-bot wrote:
> Can't process patch for file
> chrome/common/extensions/docs/examples/extensions/proxy_configuration.zip.
> File's status is None, patchset upload is incomplete

Gah.  Ok, so the commit-bot can't handle binary files either. This is a somewhat
broken system.

Mailing you a zip, Arne.

[kurrik.chromium, 2011-04-04 17:00:57]

Thanks, Mike.  Sorry for the hassle.

~Arne

On Mon, Apr 4, 2011 at 5:12 AM, <mkwst@chromium.org> wrote:

> On 2011/04/04 12:01:48, commit-bot wrote:
>
>> Can't process patch for file
>> chrome/common/extensions/docs/examples/extensions/proxy_configuration.zip.
>> File's status is None, patchset upload is incomplete
>>
>
> Gah.  Ok, so the commit-bot can't handle binary files either. This is a
> somewhat
> broken system.
>
> Mailing you a zip, Arne.
>
>
> http://codereview.chromium.org/6677108/
>