permissions: remove duplicates from request multiple at the Blink level

permissions: remove duplicates from request multiple at the Blink level

Propogating this information all the way to PermissionManager so it can be handled there
seems to be a waste of time and memory. It also adds a bunch of complexity to
all PermissionManager implementations.

Instead of this, handle all duplication at the Blink level so that chrome
can focus on handling requests.

BUG=516626
Committed: https://crrev.com/b3d2e697dc79112379f96f0f34cf26b83227a737
Cr-Commit-Position: refs/heads/master@{#352072}

[Lalit Maganti, 2015-09-28 12:47:59]

lalitm@google.com changed reviewers:
+ mlamouri@chromium.org

[Lalit Maganti, 2015-09-28 12:47:59]

Mounir: Now that the dedupe CL is on it's way, this CL should be good to review
:D

[mlamouri (slow - plz ping), 2015-09-29 13:25:35]

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp
(right):

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp:21: void
PermissionsCallback::onSuccess(WebPassOwnPtr<WebVector<WebPermissionStatus>>
result)
Could you keep the |permissionStatus| as a name here?

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp:27:
Vector<PermissionStatus*> finalResult(m_indexToCannonicalIndex->size());
Rename this |result|.

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.h
(right):

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.h:38:
OwnPtr<Vector<int>> m_indexToCannonicalIndex;
The name is confusing. Could you use "caller index" and "internal index"?

[mlamouri (slow - plz ping), 2015-09-29 13:25:57]

Oh, also, could you add more multiple request permissions with duplicates?

[Lalit Maganti, 2015-09-29 14:51:50]

Do you mean in the tests? The tests got reverted. I need to track down the fix
but no access to a windows 7 computer makes that hard :(

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp
(right):

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp:21: void
PermissionsCallback::onSuccess(WebPassOwnPtr<WebVector<WebPermissionStatus>>
result)
On 2015/09/29 13:25:35, Mounir Lamouri wrote:
> Could you keep the |permissionStatus| as a name here?

Done.

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp:27:
Vector<PermissionStatus*> finalResult(m_indexToCannonicalIndex->size());
On 2015/09/29 13:25:35, Mounir Lamouri wrote:
> Rename this |result|.

Done.

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.h
(right):

https://codereview.chromium.org/1360023005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.h:38:
OwnPtr<Vector<int>> m_indexToCannonicalIndex;
On 2015/09/29 13:25:35, Mounir Lamouri wrote:
> The name is confusing. Could you use "caller index" and "internal index"?

Done.

[mlamouri (slow - plz ping), 2015-09-29 18:00:07]

lgtm but needs a bunch of comments... and tests!

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/Permissions.cpp (right):

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/Permissions.cpp:160: int
internalIndex;
nit: add a comment explaining what you are doing here (ie. resolving duplicates)

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp
(right):

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp:27:
Vector<PermissionStatus*> result(m_callerIndexToInternalIndex->size());
nit: need comments

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.h
(right):

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.h:38:
OwnPtr<Vector<int>> m_callerIndexToInternalIndex;
nit: those two things need comments.

[Lalit Maganti, 2015-10-02 13:57:25]

Considering how hard it was for me to write the comments probably a very good
idea to get me to do so lol

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/Permissions.cpp (right):

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/Permissions.cpp:160: int
internalIndex;
On 2015/09/29 18:00:07, Mounir Lamouri wrote:
> nit: add a comment explaining what you are doing here (ie. resolving
duplicates)

Done.

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp
(right):

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.cpp:27:
Vector<PermissionStatus*> result(m_callerIndexToInternalIndex->size());
On 2015/09/29 18:00:07, Mounir Lamouri wrote:
> nit: need comments

Done.

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/permissions/PermissionsCallback.h
(right):

https://codereview.chromium.org/1360023005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/permissions/PermissionsCallback.h:38:
OwnPtr<Vector<int>> m_callerIndexToInternalIndex;
On 2015/09/29 18:00:07, Mounir Lamouri wrote:
> nit: those two things need comments.

Done.

[mlamouri (slow - plz ping), 2015-10-02 14:28:08]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-10-02 14:28:08]

lgtm

[commit-bot: I haz the power, 2015-10-02 14:28:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360023005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360023005/60001

[commit-bot: I haz the power, 2015-10-02 17:54:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-02 17:55:11]

Patchset 4 (id:??) landed as
https://crrev.com/b3d2e697dc79112379f96f0f34cf26b83227a737
Cr-Commit-Position: refs/heads/master@{#352072}

[dmurph, 2015-10-02 20:16:25]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1381353002/ by dmurph@chromium.org.

The reason for reverting is: BUG=538717.

[dmurph, 2015-10-02 20:17:14]

On 2015/10/02 at 20:16:25, dmurph wrote:
> A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1381353002/ by dmurph@chromium.org.
> 
> The reason for reverting is: BUG=538717.

This revert is needed before https://codereview.chromium.org/1381023003, which
was failing on Win7 bots.