Merge ChromeMainDelegateStagingAndroid into ChromeMainDelegateAndroid.

Merge ChromeMainDelegateStagingAndroid into ChromeMainDelegateAndroid.

A follow-up CL will delete the remnants of
ChromeMainDelegateStagingAndroid once downstream dependencies have been
updated.

BUG=526335
Committed: https://crrev.com/84219eb9637c487211b30bc18499a8d71ab60a53
Cr-Commit-Position: refs/heads/master@{#347823}

[newt (away), 2015-08-29 00:49:07]

newt@chromium.org changed reviewers:
+ yfriedman@chromium.org

[newt (away), 2015-08-29 00:49:07]

PTAL

[Yaron, 2015-08-31 13:56:51]

lgtm

[newt (away), 2015-08-31 22:56:26]

https://codereview.chromium.org/1320253002/diff/1/chrome/app/android/chrome_m...
File chrome/app/android/chrome_main_delegate_android.cc (right):

https://codereview.chromium.org/1320253002/diff/1/chrome/app/android/chrome_m...
chrome/app/android/chrome_main_delegate_android.cc:17: #include
"components/policy/core/browser/android/android_combined_policy_provider.h"
presubmit complained about this include being "illegal". Should I update
chrome/app/android/DEPS to allow including components/policy, or do something
else?

[Yaron, 2015-09-01 15:06:19]

https://codereview.chromium.org/1320253002/diff/1/chrome/app/android/chrome_m...
File chrome/app/android/chrome_main_delegate_android.cc (right):

https://codereview.chromium.org/1320253002/diff/1/chrome/app/android/chrome_m...
chrome/app/android/chrome_main_delegate_android.cc:17: #include
"components/policy/core/browser/android/android_combined_policy_provider.h"
On 2015/08/31 22:56:26, newt wrote:
> presubmit complained about this include being "illegal". Should I update
> chrome/app/android/DEPS to allow including components/policy, or do something
> else?

Why was it ok in the previous location? This is just a move. I don't see a
whitelisting of it. I'd just add it to chrome/app/DEPS

[newt (away), 2015-09-01 21:52:25]

https://codereview.chromium.org/1320253002/diff/1/chrome/app/android/chrome_m...
File chrome/app/android/chrome_main_delegate_android.cc (right):

https://codereview.chromium.org/1320253002/diff/1/chrome/app/android/chrome_m...
chrome/app/android/chrome_main_delegate_android.cc:17: #include
"components/policy/core/browser/android/android_combined_policy_provider.h"
On 2015/09/01 15:06:19, Yaron wrote:
> On 2015/08/31 22:56:26, newt wrote:
> > presubmit complained about this include being "illegal". Should I update
> > chrome/app/android/DEPS to allow including components/policy, or do
something
> > else?
> 
> Why was it ok in the previous location? This is just a move. I don't see a
> whitelisting of it. I'd just add it to chrome/app/DEPS

This code used to be in chrome_main_delegate_staging_android.h, which was in
chrome/browser, and chrome/browser/DEPS has a rule to allow components/policy.

[newt (away), 2015-09-01 21:57:07]

newt@chromium.org changed reviewers:
+ pneubeck@chromium.org

[newt (away), 2015-09-01 21:57:08]

+pneubeck: Is it OK to add a dependency from chrome/app on components/policy?
I'm moving code from chrome/browser into chrome/app and it depends on
components/policy/core/browser/android/android_combined_policy_provider.h

[pneubeck (no reviews), 2015-09-02 15:17:01]

pneubeck@chromium.org changed reviewers:
+ bartfab@chromium.org

[pneubeck (no reviews), 2015-09-02 15:17:01]

I'm not familiar with the policy integration on android.
Handing over to Bartosz.

[pneubeck (no reviews), 2015-09-02 15:17:17]

pneubeck@chromium.org changed reviewers:
- pneubeck@chromium.org

[bartfab (slow), 2015-09-03 09:51:57]

I know about as much about our policy handling code on Android as Philipp :).
However, this is a mechanical change that is easy to review.

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
File chrome/app/android/chrome_main_delegate_android.cc (right):

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:72: bool ok =
PathService::Get(base::DIR_ANDROID_APP_DATA, &data_path);
Nit: #include "base/base_paths_android.h"

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:78: else
Nit: Curly braces are required for multi-line conditionals.

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:86: LOG(ERROR) << "Failed to
set permission of " << data_path.value().c_str();
Nit: #include <string> to be able to call c_str() or simply remove that part.
LOG(ERROR) is able to handle std::string directly anyway.

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:108:
SafeBrowsingApiHandler::SetInstance(NULL);
Nit: s/NULL/nullptr/

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:115: return NULL;
Nit: s/NULL/nullptr/

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
File chrome/app/android/chrome_main_delegate_android.h (right):

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.h:37:
scoped_ptr<SafeBrowsingApiHandler> safe_browsing_api_handler_;
Nit: #include "base/memory/scoped_ptr.h"

https://codereview.chromium.org/1320253002/diff/20001/chrome/browser/android/...
File chrome/browser/android/chrome_main_delegate_staging_android.h (right):

https://codereview.chromium.org/1320253002/diff/20001/chrome/browser/android/...
chrome/browser/android/chrome_main_delegate_staging_android.h:10: class
SafeBrowsingApiHandler;
Nit: No longer used.

https://codereview.chromium.org/1320253002/diff/20001/chrome/browser/android/...
chrome/browser/android/chrome_main_delegate_staging_android.h:11: class
DataReductionProxyResourceThrottleFactory;
Nit: No longer used.

[newt (away), 2015-09-03 18:35:24]

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
File chrome/app/android/chrome_main_delegate_android.cc (right):

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:72: bool ok =
PathService::Get(base::DIR_ANDROID_APP_DATA, &data_path);
On 2015/09/03 09:51:56, bartfab wrote:
> Nit: #include "base/base_paths_android.h"

Done.

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:78: else
On 2015/09/03 09:51:57, bartfab wrote:
> Nit: Curly braces are required for multi-line conditionals.

Done.

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.cc:86: LOG(ERROR) << "Failed to
set permission of " << data_path.value().c_str();
On 2015/09/03 09:51:57, bartfab wrote:
> Nit: #include <string> to be able to call c_str() or simply remove that part.
> LOG(ERROR) is able to handle std::string directly anyway.

Just removed c_str()

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
File chrome/app/android/chrome_main_delegate_android.h (right):

https://codereview.chromium.org/1320253002/diff/20001/chrome/app/android/chro...
chrome/app/android/chrome_main_delegate_android.h:37:
scoped_ptr<SafeBrowsingApiHandler> safe_browsing_api_handler_;
On 2015/09/03 09:51:57, bartfab wrote:
> Nit: #include "base/memory/scoped_ptr.h"

Done.

https://codereview.chromium.org/1320253002/diff/20001/chrome/browser/android/...
File chrome/browser/android/chrome_main_delegate_staging_android.h (right):

https://codereview.chromium.org/1320253002/diff/20001/chrome/browser/android/...
chrome/browser/android/chrome_main_delegate_staging_android.h:10: class
SafeBrowsingApiHandler;
On 2015/09/03 09:51:57, bartfab wrote:
> Nit: No longer used.

This is actually used downstream. I'm deleting this entire file in a follow up
CL, so it's simplest just to leave this forward declaration here for now.

https://codereview.chromium.org/1320253002/diff/20001/chrome/browser/android/...
chrome/browser/android/chrome_main_delegate_staging_android.h:11: class
DataReductionProxyResourceThrottleFactory;
On 2015/09/03 09:51:57, bartfab wrote:
> Nit: No longer used.

ditto

[bartfab (slow), 2015-09-04 09:24:22]

lgtm

[newt (away), 2015-09-04 15:34:56]

newt@chromium.org changed reviewers:
+ cpu@chromium.org

[newt (away), 2015-09-04 15:34:56]

+cpu for chrome/app/DEPS

[newt (away), 2015-09-08 17:26:34]

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

[commit-bot: I haz the power, 2015-09-08 17:27:18]

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

[commit-bot: I haz the power, 2015-09-08 18:09:31]

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

[commit-bot: I haz the power, 2015-09-08 18:09:32]

Dry run: This issue passed the CQ dry run.

[cpu_(ooo_6.6-7.5), 2015-09-08 18:28:04]

Is there a way to segment deps by platform? The app in stuff on windows is meant
to be very tin, because the exe just hosts the sandbox and loads the chrome dll.

[newt (away), 2015-09-08 18:49:24]

On 2015/09/08 18:28:04, cpu wrote:
> Is there a way to segment deps by platform? The app in stuff on windows is
meant
> to be very tin, because the exe just hosts the sandbox and loads the chrome
dll.

There doesn't seem to be. I could add a more specific DEPS rule, though:
"+components/policy/core/browser/android" instead of "+components/policy". Does
that sound OK?

[cpu_(ooo_6.6-7.5), 2015-09-08 19:31:12]

yes lets do that. with that lgtm

[newt (away), 2015-09-08 22:40:13]

The CQ bit was checked by newt@chromium.org

[newt (away), 2015-09-08 22:40:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1320253002/#ps60001
(title: "addressed bartfab's comments")

[newt (away), 2015-09-08 22:40:16]

The CQ bit was unchecked by newt@chromium.org

[newt (away), 2015-09-08 22:40:53]

The CQ bit was checked by newt@chromium.org

[newt (away), 2015-09-08 22:40:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
bartfab@chromium.org, cpu@chromium.org, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1320253002/#ps80001
(title: "used more specific DEPS rule")

[commit-bot: I haz the power, 2015-09-08 22:41:00]

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

[commit-bot: I haz the power, 2015-09-08 23:22:03]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-09-08 23:22:47]

Patchset 5 (id:??) landed as
https://crrev.com/84219eb9637c487211b30bc18499a8d71ab60a53
Cr-Commit-Position: refs/heads/master@{#347823}