Make OutputDeveloperMessageCode an enum class

Make OutputDeveloperMessageCode an enum class

BUG=609122

[pkotwicz, 2016-06-08 21:30:52]

pkotwicz@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[pkotwicz, 2016-06-08 21:30:53]

dfalcantara@ can you please take a look?

A page is WebAPK capable only if
AppBannerDataFetcher::IsManifestValidForWebApp() returns true. This CL makes
OutputDeveloperMessageCode an enum class in preparation to pulling
IsManifestValidForWebApp() into a separate file and calling it from
add_to_homescreen_data_fetcher.cc

[pkotwicz, 2016-06-08 21:31:35]

dfalcantara@ can you please take a look?

A page is WebAPK capable only if
AppBannerDataFetcher::IsManifestValidForWebApp() returns true. This CL makes
OutputDeveloperMessageCode an enum class in preparation to pulling
IsManifestValidForWebApp() into a separate file and calling it from
add_to_homescreen_data_fetcher.cc In particular, I want to add kNone to the
OutputDeveloperMessageCode enum

[gone, 2016-06-08 21:32:11]

dfalcantara@chromium.org changed reviewers:
+ dominickn@chromium.org

[gone, 2016-06-08 21:32:12]

Dominick: do you have cycles for this review?  It's your code, IIRC.

[dominickn, 2016-06-08 23:20:44]

On 2016/06/08 21:30:53, pkotwicz wrote:
> dfalcantara@ can you please take a look?
> 
> A page is WebAPK capable only if
> AppBannerDataFetcher::IsManifestValidForWebApp() returns true. This CL makes
> OutputDeveloperMessageCode an enum class in preparation to pulling
> IsManifestValidForWebApp() into a separate file and calling it from
> add_to_homescreen_data_fetcher.cc

IsManifestValidForWebApp is already static, so is there an issue in calling it
directly right now?

Also, I am in the process of refactoring all of this so that the entire app
banner check happens in a separate class (see comment in
crbug.com/606430#c8). That way we separate the banner eligibility
check from the banner engagement requirements, so WebAPKs can
use the same check mechanism. My intention is that this will happen in
the next milestone. That will put IsManifestValidForWebApp in a separate
class anyway.

[pkotwicz, 2016-06-08 23:59:44]

The reason I am making OutputDeveloperMessageCode an enum class is because I
want to:
- add OutputDeveloperMessageCode::kNone (Using OutputDeveloperMessageCode::kNone
feels much more correct than kNone without the class scoping)
- Change AppBannerDataFetcher::IsManifestValidForWebApp(const content::Manifest&
manifest, content::WebContents* web_contents, bool is_debug_mode) to
AppBannerDataFetcher::IsManifestValidForWebApp(const content::Manifest&
manifest, OutputDeveloperMessageCode* code)

Currently, my implementation does not log any messages to the JS console from
add_to_homescreen_data_fetcher.cc We may want to log to the JS console why an
app is not WebAPK-eligible but it will require more work

[dominickn, 2016-06-09 00:15:08]

On 2016/06/08 23:59:44, pkotwicz wrote:
> The reason I am making OutputDeveloperMessageCode an enum class is because I
> want to:
> - add OutputDeveloperMessageCode::kNone (Using
OutputDeveloperMessageCode::kNone
> feels much more correct than kNone without the class scoping)
> - Change AppBannerDataFetcher::IsManifestValidForWebApp(const
content::Manifest&
> manifest, content::WebContents* web_contents, bool is_debug_mode) to
> AppBannerDataFetcher::IsManifestValidForWebApp(const content::Manifest&
> manifest, OutputDeveloperMessageCode* code)
> 
> Currently, my implementation does not log any messages to the JS console from
> add_to_homescreen_data_fetcher.cc We may want to log to the JS console why an
> app is not WebAPK-eligible but it will require more work

So the primary reason is to be able to pass out an error code for WebAPKs to
report
to console? I'd much rather make the underlying app banner system more generic
than keep extending it in ways it wasn't originally intended to handle. Part of
this is
making the error messages less app-banner specific. They'll read something like,
"Site is not installable: <reason>", which should be generic enough for both app
banners and WebAPKs. Like I said, I'm aiming to have this done for the
next milestone.

[pkotwicz, 2016-06-09 01:40:47]

Description was changed from

==========
Make OutputDeveloperMessageCode an enum class

BUG=524670
==========

to

==========
Make OutputDeveloperMessageCode an enum class

BUG=609122
==========