Allow inline install for user initiated fullscreen mode.

Allow inline install for user initiated fullscreen mode since this mode requires that the user initiate it and due to a nuance with mac windows where the green maximize button enters fullscreen.

BUG=690523
Review-Url: https://codereview.chromium.org/2693183002
Cr-Commit-Position: refs/heads/master@{#450753}
Committed: https://chromium.googlesource.com/chromium/src/+/e6d00f176148e0aa1067b237a2352e99ceff751c

[Ackerman, 2017-02-14 19:39:07]

Description was changed from

==========
Allow inline install for user initiatied fullscreen mode.

BUG=690523
==========

to

==========
Allow inline install for user initiated fullscreen mode.

BUG=690523
==========

[Ackerman, 2017-02-14 21:28:10]

ackermanb@chromium.org changed reviewers:
+ meacer@chromium.org, rdevlin.cronin@chromium.org

[meacer, 2017-02-14 22:15:37]

Lgtm. Might want to explain why we are doing this in the bug description.

[Devlin, 2017-02-14 23:19:53]

https://codereview.chromium.org/2693183002/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/webstore_inline_installer_browsertest.cc (left):

https://codereview.chromium.org/2693183002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:269:
BlockInlineInstallFromFullscreenForBrowser) {
Instead of deleting this, let's make it a test for
AllowInlineInstallationFromFullscreenForBrowser (and add a comment for why we
allow it).

[Ackerman, 2017-02-15 05:33:22]

PTAL

[Devlin, 2017-02-15 15:48:06]

lgtm

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/webstore_inline_installer_browsertest.cc (right):

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:286:
"ecglahbcnmdpdciemllbhojghbkagdje"));
huh.  b.repeat(32) wasn't the id?  Does that mean that these tests weren't
actually testing anything before? (Because the registry would never contain that
id?)  Should have caught that in the original review; my bad.

Looks like we have this string const'd as kTestExtensionId on line 43, prefer
that.

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:310:
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
In line with the above, seems like this should be kTestExtensionId instead of
bbbbb...

[Devlin, 2017-02-15 17:01:12]

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/webstore_inline_installer_browsertest.cc (right):

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:286:
"ecglahbcnmdpdciemllbhojghbkagdje"));
On 2017/02/15 15:48:06, Devlin wrote:
> huh.  b.repeat(32) wasn't the id?  Does that mean that these tests weren't
> actually testing anything before? (Because the registry would never contain
that
> id?)  Should have caught that in the original review; my bad.
> 
> Looks like we have this string const'd as kTestExtensionId on line 43, prefer
> that.

nvm, all cleared up (but still please use kTestExtensionId here)

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:310:
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
On 2017/02/15 15:48:05, Devlin wrote:
> In line with the above, seems like this should be kTestExtensionId instead of
> bbbbb...

ignore

[Ackerman, 2017-02-15 17:28:23]

Description was changed from

==========
Allow inline install for user initiated fullscreen mode.

BUG=690523
==========

to

==========
Allow inline install for user initiated fullscreen mode since this mode requires
that the user initiate it and due to a nuance with mac windows where the green
maximize button enters fullscreen.

BUG=690523
==========

[Ackerman, 2017-02-15 17:28:39]

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/webstore_inline_installer_browsertest.cc (right):

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:286:
"ecglahbcnmdpdciemllbhojghbkagdje"));
On 2017/02/15 17:01:12, Devlin wrote:
> On 2017/02/15 15:48:06, Devlin wrote:
> > huh.  b.repeat(32) wasn't the id?  Does that mean that these tests weren't
> > actually testing anything before? (Because the registry would never contain
> that
> > id?)  Should have caught that in the original review; my bad.
> > 
> > Looks like we have this string const'd as kTestExtensionId on line 43,
prefer
> > that.
> 
> nvm, all cleared up (but still please use kTestExtensionId here)

Done.

https://codereview.chromium.org/2693183002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/webstore_inline_installer_browsertest.cc:310:
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
On 2017/02/15 17:01:12, Devlin wrote:
> On 2017/02/15 15:48:05, Devlin wrote:
> > In line with the above, seems like this should be kTestExtensionId instead
of
> > bbbbb...
> 
> ignore

Updated to use the test extension id in install_from_fullscreen.html and updated
here to use kTestExtensionId.

[Ackerman, 2017-02-15 17:32:09]

The CQ bit was checked by ackermanb@chromium.org

[Ackerman, 2017-02-15 17:32:09]

The patchset sent to the CQ was uploaded after l-g-t-m from meacer@chromium.org,
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2693183002/#ps60001
(title: "Updating to use test extension id.")

[commit-bot: I haz the power, 2017-02-15 17:33:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 19:02:20]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487179929701950,
"parent_rev": "eba09b260b6cad9c8dcd3f8142af1df5df66a4c4", "commit_rev":
"e6d00f176148e0aa1067b237a2352e99ceff751c"}

[commit-bot: I haz the power, 2017-02-15 19:06:44]

Description was changed from

==========
Allow inline install for user initiated fullscreen mode since this mode requires
that the user initiate it and due to a nuance with mac windows where the green
maximize button enters fullscreen.

BUG=690523
==========

to

==========
Allow inline install for user initiated fullscreen mode since this mode requires
that the user initiate it and due to a nuance with mac windows where the green
maximize button enters fullscreen.

BUG=690523

Review-Url: https://codereview.chromium.org/2693183002
Cr-Commit-Position: refs/heads/master@{#450753}
Committed:
https://chromium.googlesource.com/chromium/src/+/e6d00f176148e0aa1067b237a235...
==========

[commit-bot: I haz the power, 2017-02-15 19:06:45]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/e6d00f176148e0aa1067b237a235...