Removing file_thread_task_runner parameter from SerialIoHandler

Removed the use of file_thread_task_runner across
SerialIoHandler and BattOrAgent. 

BUG=728448
Review-Url: https://codereview.chromium.org/2914173002
Cr-Commit-Position: refs/heads/master@{#481529}
Committed: https://chromium.googlesource.com/chromium/src/+/1f83b935b39059fb1f407857b0da6f4614e16e71

[sujith, 2017-06-01 11:21:51]

Description was changed from

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448

Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

to

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448
R=SamsungPeerReview

Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

[sujith, 2017-06-01 11:21:51]

sujiths.s@samsung.com changed reviewers:
+ shanmuga.m@samsung.com

[sujith, 2017-06-01 11:22:03]

PTAL

[sujith, 2017-06-01 11:43:22]

Description was changed from

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448
R=SamsungPeerReview

Signed-off-by: sujiths.s <sujiths.s@samsung.com>
==========

to

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448
R=SamsungPeerReview
==========

[Shanmuga Pandi, 2017-06-01 11:53:37]

peer l-g-t-m.

[sujith, 2017-06-01 11:56:14]

Description was changed from

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448
R=SamsungPeerReview
==========

to

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448
==========

[sujith, 2017-06-01 12:05:51]

sujiths.s@samsung.com changed reviewers:
+ alexmos@chromium.org, dpranke@chromium.org, nick@chromium.org,
piman@chromium.org, primiano@chromium.org, reillyg@chromium.org,
rockot@chromium.org, scottmg@chromium.org, thakis@chromium.org

[sujith, 2017-06-01 12:05:52]

Hi All,

PTAL

[Reilly Grant (use Gerrit), 2017-06-01 13:48:21]

reillyg@chromium.org changed reviewers:
+ charliea@chromium.org
- alexmos@chromium.org, dpranke@chromium.org, nick@chromium.org,
piman@chromium.org, rockot@chromium.org, scottmg@chromium.org,
shanmuga.m@samsung.com, thakis@chromium.org

[Reilly Grant (use Gerrit), 2017-06-01 13:48:21]

Thank you for your contribution. Please see my comments below.

In addition, I have removed most of the reviewers on this patch and added
charliea@ for //tools/battor_agent. When picking reviewers only choose one from
the OWNERS file and prefer OWNERS for the specific directory in which you are
making a change.

https://codereview.chromium.org/2914173002/diff/20001/device/serial/serial_io...
File device/serial/serial_io_handler.cc (left):

https://codereview.chromium.org/2914173002/diff/20001/device/serial/serial_io...
device/serial/serial_io_handler.cc:68: base::ThreadTaskRunnerHandle::Get()));
file_thread_task_runner_ is clearly used here so the original issue was wrong to
say it was completely unused. However we can replace this statement with,

base::PostTaskWithTraits(
    FROM_HERE,
    {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
    base::Bind(&SerialIoHandler::StartOpen, this, port,	
               base::ThreadTaskRunnerHandle::Get()));

https://codereview.chromium.org/2914173002/diff/20001/device/serial/serial_io...
device/serial/serial_io_handler.cc:174: file_thread_task_runner_->PostTask(
Same here. We can't just remove the Close method. Please keep this method and
use base::PostTaskWithTraits instead.

[sujith, 2017-06-01 14:57:31]

changes done as suggested.

PTAL

https://codereview.chromium.org/2914173002/diff/20001/device/serial/serial_io...
File device/serial/serial_io_handler.cc (left):

https://codereview.chromium.org/2914173002/diff/20001/device/serial/serial_io...
device/serial/serial_io_handler.cc:68: base::ThreadTaskRunnerHandle::Get()));
On 2017/06/01 13:48:21, Reilly Grant wrote:
> file_thread_task_runner_ is clearly used here so the original issue was wrong
to
> say it was completely unused. However we can replace this statement with,
> 
> base::PostTaskWithTraits(
>     FROM_HERE,
>     {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
>     base::Bind(&SerialIoHandler::StartOpen, this, port,	
>                base::ThreadTaskRunnerHandle::Get()));

Done.

https://codereview.chromium.org/2914173002/diff/20001/device/serial/serial_io...
device/serial/serial_io_handler.cc:174: file_thread_task_runner_->PostTask(
On 2017/06/01 13:48:21, Reilly Grant wrote:
> Same here. We can't just remove the Close method. Please keep this method and
> use base::PostTaskWithTraits instead.

Done.

[Reilly Grant (use Gerrit), 2017-06-06 19:47:59]

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

[commit-bot: I haz the power, 2017-06-06 19:48:44]

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-06-06 20:18:50]

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

[commit-bot: I haz the power, 2017-06-06 20:18:52]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Reilly Grant (use Gerrit), 2017-06-06 21:10:06]

Please fix the compilation issues reported by the CQ dry run.

[charliea (OOO until 10-5), 2017-06-07 15:23:30]

I defer to Reilly whether the changes to SerialIoHandler::Open() and ::Close()
are correct, but I think that the CL description is wrong. It's clear that
file_thread_task_runner_ was indeed used by these two methods on non-CrOS
systems, although it looks like there might be a way to avoid using it now? I
think the CL description should be updated accordingly.

Also, it looks like you updated the constructor of BattOrAgent to no longer
require the file_thread_task_runner, but never updating the callers of that
constructor.

[sujith, 2017-06-07 15:37:55]

Description was changed from

==========
Removing file_thread_task_runner parameter from SerialIoHandler

This parameter is saved in a member variable and it is unused.
So removed it.

BUG=728448
==========

to

==========
Removed the use of file_thread_task_runner across
different SerialIoHandler and BattOrAgent. 
Instead used PostTaskWithTraits.

BUG=728448
==========

[sujith, 2017-06-07 15:37:55]

sujiths.s@samsung.com changed reviewers:
+ shanmuga.m@samsung.com

[sujith, 2017-06-07 15:39:10]

@charliea, @Reilly Grant,

Changes done as suggested. PTAL

[Reilly Grant (use Gerrit), 2017-06-07 15:48:56]

https://codereview.chromium.org/2914173002/diff/60001/tools/battor_agent/batt...
File tools/battor_agent/battor_agent_bin.cc (right):

https://codereview.chromium.org/2914173002/diff/60001/tools/battor_agent/batt...
tools/battor_agent/battor_agent_bin.cc:333: base::Thread file_thread_;
This field should be replaced with a call to
base::TaskScheduler::CreateAndStartWithDefaultParams("battor_agent") before
calling bin.Run() in main().

[Reilly Grant (use Gerrit), 2017-06-07 15:50:12]

Description was changed from

==========
Removed the use of file_thread_task_runner across
different SerialIoHandler and BattOrAgent. 
Instead used PostTaskWithTraits.

BUG=728448
==========

to

==========
Removing file_thread_task_runner parameter from SerialIoHandler

Removed the use of file_thread_task_runner across
different SerialIoHandler and BattOrAgent. 
Instead used PostTaskWithTraits.

BUG=728448
==========

[Reilly Grant (use Gerrit), 2017-06-07 15:50:36]

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

[commit-bot: I haz the power, 2017-06-07 15:50:56]

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-06-07 17:16:02]

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

[commit-bot: I haz the power, 2017-06-07 17:16:03]

Dry run: This issue passed the CQ dry run.

[sujith, 2017-06-08 06:34:53]

Description was changed from

==========
Removing file_thread_task_runner parameter from SerialIoHandler

Removed the use of file_thread_task_runner across
different SerialIoHandler and BattOrAgent. 
Instead used PostTaskWithTraits.

BUG=728448
==========

to

==========
Removed the use of file_thread_task_runner across
SerialIoHandler and BattOrAgent. 

BUG=728448
==========

[sujith, 2017-06-08 06:36:38]

Hi All,
PTAL

https://codereview.chromium.org/2914173002/diff/60001/tools/battor_agent/batt...
File tools/battor_agent/battor_agent_bin.cc (right):

https://codereview.chromium.org/2914173002/diff/60001/tools/battor_agent/batt...
tools/battor_agent/battor_agent_bin.cc:333: base::Thread file_thread_;
On 2017/06/07 15:48:56, Reilly Grant wrote:
> This field should be replaced with a call to
> base::TaskScheduler::CreateAndStartWithDefaultParams("battor_agent") before
> calling bin.Run() in main().

Done.

[charliea (OOO until 10-5), 2017-06-09 19:41:00]

lgtm for tools/battor_agent

[Reilly Grant (use Gerrit), 2017-06-09 19:44:02]

lgtm

[sujith, 2017-06-10 05:40:55]

The CQ bit was checked by sujiths.s@samsung.com

[commit-bot: I haz the power, 2017-06-10 05:41:03]

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-06-10 05:48:32]

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

[commit-bot: I haz the power, 2017-06-10 05:48:33]

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

[Shanmuga Pandi, 2017-06-14 05:01:39]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 05:01:58]

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-06-14 05:18:37]

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

[commit-bot: I haz the power, 2017-06-14 05:18:38]

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

[Shanmuga Pandi, 2017-06-14 11:14:44]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 11:14:56]

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-06-14 12:36:38]

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

[commit-bot: I haz the power, 2017-06-14 12:36:39]

Dry run: This issue passed the CQ dry run.

[sujith, 2017-06-14 14:54:02]

hi All,

PTAL

[sujith, 2017-06-21 06:44:13]

Hi All,
PTAL.

@primiano tucci, could you please check from content/browser/tracing side?

[Primiano Tucci (use gerrit), 2017-06-21 08:58:42]

RS-LGTM on content/browser/tracing/power_tracing_agent.cc, assuming charliea@ is
fine with this.

[sujith, 2017-06-22 07:20:59]

The CQ bit was checked by sujiths.s@samsung.com

[sujith, 2017-06-22 07:21:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
reillyg@chromium.org, charliea@chromium.org
Link to the patchset: https://codereview.chromium.org/2914173002/#ps120001
(title: "removed unused variable")

[commit-bot: I haz the power, 2017-06-22 07:21:12]

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-06-22 07:59:57]

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

[commit-bot: I haz the power, 2017-06-22 07:59:59]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sujith, 2017-06-22 13:42:48]

The CQ bit was checked by sujiths.s@samsung.com

[commit-bot: I haz the power, 2017-06-22 13:42:56]

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-06-22 14:37:05]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1498138968517270,
"parent_rev": "7ee677b04f3651014f498741c533c48003985b60", "commit_rev":
"1f83b935b39059fb1f407857b0da6f4614e16e71"}

[commit-bot: I haz the power, 2017-06-22 14:37:16]

Description was changed from

==========
Removed the use of file_thread_task_runner across
SerialIoHandler and BattOrAgent. 

BUG=728448
==========

to

==========
Removed the use of file_thread_task_runner across
SerialIoHandler and BattOrAgent. 

BUG=728448

Review-Url: https://codereview.chromium.org/2914173002
Cr-Commit-Position: refs/heads/master@{#481529}
Committed:
https://chromium.googlesource.com/chromium/src/+/1f83b935b39059fb1f407857b0da...
==========

[commit-bot: I haz the power, 2017-06-22 14:37:17]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/1f83b935b39059fb1f407857b0da...