Open ChromeOS clock settings on the UI thread instead of File.

Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
TEST=Manual, change system clock, go to https://expired.badssl.com and launch clock settings.

Committed: https://crrev.com/c14f57a9d1d19685e64b692ee7259dd4eabd01f7
Cr-Commit-Position: refs/heads/master@{#429182}

[meacer, 2016-10-31 18:18:17]

meacer@chromium.org changed reviewers:
+ estark@chromium.org

[meacer, 2016-10-31 18:18:18]

estark: PTAL?

[meacer, 2016-10-31 18:19:19]

Description was changed from

==========
Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
==========

to

==========
Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
TEST=Manual, change system clock, go to https://expired.badssl.com and launch
clock settings.
==========

[estark, 2016-10-31 19:02:49]

lgtm

https://codereview.chromium.org/2459193003/diff/1/chrome/browser/interstitial...
File chrome/browser/interstitials/chrome_controller_client.cc (right):

https://codereview.chromium.org/2459193003/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/chrome_controller_client.cc:50:
chrome::android::OpenDateAndTimeSettings();
I wonder if this should be on the UI thread as well. I can't figure out if
sending an intent is a blocking operation or not... do you know?

[meacer, 2016-10-31 21:38:22]

https://codereview.chromium.org/2459193003/diff/1/chrome/browser/interstitial...
File chrome/browser/interstitials/chrome_controller_client.cc (right):

https://codereview.chromium.org/2459193003/diff/1/chrome/browser/interstitial...
chrome/browser/interstitials/chrome_controller_client.cc:50:
chrome::android::OpenDateAndTimeSettings();
On 2016/10/31 19:02:49, estark wrote:
> I wonder if this should be on the UI thread as well. I can't figure out if
> sending an intent is a blocking operation or not... do you know?

Yeah, I was wondering the same thing. 

I don't know about this specific intent, but there is another one in that file
called SendMail. SendMail is called from the UI thread:
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/net_export_ui.cc...

I can't tell which one is the correct one. How about I land this and keep the
bug open to investigate?

[estark, 2016-10-31 21:39:05]

On 2016/10/31 21:38:22, Mustafa Emre Acer wrote:
>
https://codereview.chromium.org/2459193003/diff/1/chrome/browser/interstitial...
> File chrome/browser/interstitials/chrome_controller_client.cc (right):
> 
>
https://codereview.chromium.org/2459193003/diff/1/chrome/browser/interstitial...
> chrome/browser/interstitials/chrome_controller_client.cc:50:
> chrome::android::OpenDateAndTimeSettings();
> On 2016/10/31 19:02:49, estark wrote:
> > I wonder if this should be on the UI thread as well. I can't figure out if
> > sending an intent is a blocking operation or not... do you know?
> 
> Yeah, I was wondering the same thing. 
> 
> I don't know about this specific intent, but there is another one in that file
> called SendMail. SendMail is called from the UI thread:
>
https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/net_export_ui.cc...
> 
> I can't tell which one is the correct one. How about I land this and keep the
> bug open to investigate?

SGTM, thanks.

[meacer, 2016-10-31 21:51:33]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2016-10-31 21:52:13]

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, 2016-10-31 21:54:26]

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

[commit-bot: I haz the power, 2016-10-31 21:54:27]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[meacer, 2016-11-01 22:30:47]

Rebased, trying again.

[meacer, 2016-11-01 22:30:51]

The CQ bit was checked by meacer@chromium.org

[meacer, 2016-11-01 22:30:51]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2459193003/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2016-11-01 22:32:06]

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, 2016-11-01 22:54:56]

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

[commit-bot: I haz the power, 2016-11-01 22:54:58]

Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[meacer, 2016-11-01 23:34:24]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2016-11-01 23:34:48]

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, 2016-11-02 00:18:20]

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

[commit-bot: I haz the power, 2016-11-02 00:18:21]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[meacer, 2016-11-02 00:51:30]

The CQ bit was checked by meacer@chromium.org

[meacer, 2016-11-02 00:51:30]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org
Link to the patchset: https://codereview.chromium.org/2459193003/#ps40001
(title: "Exclude code in ChromeOS")

[commit-bot: I haz the power, 2016-11-02 00:51:54]

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, 2016-11-02 01:48:02]

Description was changed from

==========
Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
TEST=Manual, change system clock, go to https://expired.badssl.com and launch
clock settings.
==========

to

==========
Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
TEST=Manual, change system clock, go to https://expired.badssl.com and launch
clock settings.
==========

[commit-bot: I haz the power, 2016-11-02 01:48:03]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-02 01:49:42]

Description was changed from

==========
Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
TEST=Manual, change system clock, go to https://expired.badssl.com and launch
clock settings.
==========

to

==========
Open ChromeOS clock settings on the UI thread instead of File.

ChromeOS clock settings code accesses BrowserProcessImpl::profile_manager()
which should be accessed on the UI thread. Since this code is simply opening
a new window, there is no need to run it on a different thread.

BUG=657704
TEST=Manual, change system clock, go to https://expired.badssl.com and launch
clock settings.

Committed: https://crrev.com/c14f57a9d1d19685e64b692ee7259dd4eabd01f7
Cr-Commit-Position: refs/heads/master@{#429182}
==========

[commit-bot: I haz the power, 2016-11-02 01:49:43]

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