Extracted MockManifestPermission to its own file

Extracted MockManifestPermission to its own file

BUG=715638
Review-Url: https://codereview.chromium.org/2850493002
Cr-Commit-Position: refs/heads/master@{#468612}
Committed: https://chromium.googlesource.com/chromium/src/+/556c3fd74233cb76a64ddbdbc0f1bd61edc0895e

[Ivan Šandrk, 2017-04-27 13:38:29]

The CQ bit was checked by isandrk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 13:38:57]

Dry run: 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-04-27 13:52:12]

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

[commit-bot: I haz the power, 2017-04-27 13:52:13]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ivan Šandrk, 2017-04-27 14:23:39]

isandrk@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Ivan Šandrk, 2017-04-27 14:23:39]

In file included from
../../extensions/common/permissions/mock_manifest_permission.cc:8:
../../testing/gtest/include/gtest/gtest.h:58:10: fatal error:
'gtest/internal/gtest-internal.h' file not found


Hey Devlin, at this point I'm unsure what to do. I've talked to a guy (beverloo)
on #chromium IRC channel, and he said I need to add the mock to "test_support".
I see 4 options now:

- use extensions/common/BUILD.gn, and add the new section there for it
- use extensions/test/BUILD.gn, put section there
- remove those 3 EXPECT_EQ's from mock_manifest_permission.cc
- forget about this refactor.

What are your thoughts?

[Devlin, 2017-04-28 16:14:57]

On 2017/04/27 14:23:39, Ivan Šandrk wrote:
> In file included from
> ../../extensions/common/permissions/mock_manifest_permission.cc:8:
> ../../testing/gtest/include/gtest/gtest.h:58:10: fatal error:
> 'gtest/internal/gtest-internal.h' file not found
> 
> 
> Hey Devlin, at this point I'm unsure what to do. I've talked to a guy
(beverloo)
> on #chromium IRC channel, and he said I need to add the mock to
"test_support".
> I see 4 options now:
> 
> - use extensions/common/BUILD.gn, and add the new section there for it
> - use extensions/test/BUILD.gn, put section there
> - remove those 3 EXPECT_EQ's from mock_manifest_permission.cc
> - forget about this refactor.
> 
> What are your thoughts?

Try putting this in the test_support section of extensions/BUILD.gn here:
https://chromium.googlesource.com/chromium/src/+/638b48e2f217dd8fe276e91906d7...

Does that work?

[Ivan Šandrk, 2017-04-28 16:32:37]

The CQ bit was checked by isandrk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-28 16:33:12]

Dry run: CQ is trying da patch.

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

[Ivan Šandrk, 2017-04-28 16:33:28]

On 2017/04/28 16:14:57, Devlin wrote:
> On 2017/04/27 14:23:39, Ivan Šandrk wrote:
> > In file included from
> > ../../extensions/common/permissions/mock_manifest_permission.cc:8:
> > ../../testing/gtest/include/gtest/gtest.h:58:10: fatal error:
> > 'gtest/internal/gtest-internal.h' file not found
> > 
> > 
> > Hey Devlin, at this point I'm unsure what to do. I've talked to a guy
> (beverloo)
> > on #chromium IRC channel, and he said I need to add the mock to
> "test_support".
> > I see 4 options now:
> > 
> > - use extensions/common/BUILD.gn, and add the new section there for it
> > - use extensions/test/BUILD.gn, put section there
> > - remove those 3 EXPECT_EQ's from mock_manifest_permission.cc
> > - forget about this refactor.
> > 
> > What are your thoughts?
> 
> Try putting this in the test_support section of extensions/BUILD.gn here:
>
https://chromium.googlesource.com/chromium/src/+/638b48e2f217dd8fe276e91906d7...
> 
> Does that work?

Perfect!

[Devlin, 2017-04-28 17:09:54]

lgtm; thanks for cleaning this up. :)

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
File extensions/common/permissions/mock_manifest_permission.h (right):

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:11: #include
"base/values.h"
I think a forward declaration is sufficient here.

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:12: #include
"extensions/common/permissions/api_permission_set.h"
needed?

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:17: class
MockManifestPermission : public ManifestPermission {
Add a brief class comment

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:34: std::string name_;
DISALLOW_COPY_AND_ASSIGN

[commit-bot: I haz the power, 2017-04-28 18:56:45]

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

[commit-bot: I haz the power, 2017-04-28 18:56:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ivan Šandrk, 2017-05-02 12:11:34]

The CQ bit was checked by isandrk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 12:11:52]

Dry run: CQ is trying da patch.

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

[Ivan Šandrk, 2017-05-02 12:12:57]

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
File extensions/common/permissions/mock_manifest_permission.h (right):

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:11: #include
"base/values.h"
On 2017/04/28 17:09:54, Devlin wrote:
> I think a forward declaration is sufficient here.

Done.

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:12: #include
"extensions/common/permissions/api_permission_set.h"
On 2017/04/28 17:09:54, Devlin wrote:
> needed?

Done.

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:17: class
MockManifestPermission : public ManifestPermission {
On 2017/04/28 17:09:54, Devlin wrote:
> Add a brief class comment

Done.

https://codereview.chromium.org/2850493002/diff/20001/extensions/common/permi...
extensions/common/permissions/mock_manifest_permission.h:34: std::string name_;
On 2017/04/28 17:09:54, Devlin wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

[Ivan Šandrk, 2017-05-02 12:13:02]

The CQ bit was unchecked by isandrk@chromium.org

[Ivan Šandrk, 2017-05-02 12:13:04]

The CQ bit was checked by isandrk@chromium.org

[Ivan Šandrk, 2017-05-02 12:13:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2850493002/#ps40001
(title: "Nits")

[Ivan Šandrk, 2017-05-02 12:13:07]

The CQ bit was checked by isandrk@chromium.org

[commit-bot: I haz the power, 2017-05-02 12:13:16]

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-05-02 13:07:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493727187997440,
"parent_rev": "eb6696a1bfb1c32c6d2d1baf9a0dabcc632119e7", "commit_rev":
"556c3fd74233cb76a64ddbdbc0f1bd61edc0895e"}

[commit-bot: I haz the power, 2017-05-02 13:07:28]

Description was changed from

==========
Extracted MockManifestPermission to its own file

BUG=715638
==========

to

==========
Extracted MockManifestPermission to its own file

BUG=715638

Review-Url: https://codereview.chromium.org/2850493002
Cr-Commit-Position: refs/heads/master@{#468612}
Committed:
https://chromium.googlesource.com/chromium/src/+/556c3fd74233cb76a64ddbdbc0f1...
==========

[commit-bot: I haz the power, 2017-05-02 13:07:29]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/556c3fd74233cb76a64ddbdbc0f1...