0
Commit Graph

16 Commits

Author SHA1 Message Date
danakj
f416ce9d3b Convert content/browser/ Callbacks to {Once,Repeating}Callback
Use OnceCallback where possible because the callback is only used once
and RepeatingCallback and BindRepeating otherwise.

R=avi@chromium.org

Bug: 1007760
Change-Id: I3d02fded4b40cdc843ac89e057a34bdf3766b1bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1962545
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723954}
2019-12-11 20:45:45 +00:00
Mark Pilgrim
b9350ee95f Migrate startup_complete_callback to OnceCallback
Bug: 714018
Change-Id: I9ecc53ebb303b1c08ca4f21f9b948fdbfa02598c
Reviewed-on: https://chromium-review.googlesource.com/1104387
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568198}
2018-06-18 22:25:22 +00:00
Tommy Nyquist
4b749d0e31 Reland "Add std::move() to local base::Callback instances in //content"
This reverts commit 9b4af06d02.

Reason for revert: This does not seem like the root cause.

Original change's description:
> Revert "Add std::move() to local base::Callback instances in //content"
> 
> This reverts commit c55a5a47ee.
> 
> Reason for revert: Speculative revert because of renderer crashes across
> the board on Android with:
> [FATAL:weak_ptr.cc(26)] Check failed: sequence_checker_.CalledOnValidSequence(). WeakPtrs must be checked on the same sequenced thread
> 
> Original change's description:
> > Add std::move() to local base::Callback instances in //content
> > 
> > This adds std::move() around base::Callback instances where it looks
> > relevant, by applying `base_bind_rewriters -rewriter=add_std_move`.
> > https://crrev.com/c/970143, plus manual fixes.
> > 
> > Example:
> >   // Before:
> >   void set_callback(base::Closure cb) { g_cb = cb; }
> >   void RunCallback(base::Callback<void(int)> cb) { cb.Run(42); }
> >   void Post() {
> >     base::Closure task = base::Bind(&Foo);
> >     PostTask(FROM_HERE, task);
> >   }
> > 
> >   // After:
> >   void set_callback(base::Closure cb) { g_cb = std::move(cb); }
> >   void RunCallback(base::Callback<void(int)> cb) { std::move(cb).Run(42); }
> >   void Post() {
> >     base::Closure task = base::Bind(&Foo);
> >     PostTask(FROM_HERE, std::move(task));
> >   }
> > 
> > Specifically, it inserts std::move() if:
> >  - it's a pass-by-value parameter or non-const local variable.
> >  - the occurrence is the latest in its control flow.
> >  - no pointer is taken for the variable.
> >  - no capturing lambda exists for the variable.
> > 
> > Change-Id: I53853f9b9c8604994e2065af66ed4607af9c12ed
> > Reviewed-on: https://chromium-review.googlesource.com/970056
> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> > Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#544356}
> 
> TBR=kinuko@chromium.org,tzik@chromium.org
> 
> Change-Id: Ie7392a2229e1ef0f740d8958f8fe43d99b0460e9
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/972321
> Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
> Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544527}

TBR=kinuko@chromium.org,nyquist@chromium.org,tzik@chromium.org

Change-Id: I0aabd6032a070a28d0e5a4f796f37fe18f1e5cd4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/972302
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544536}
2018-03-20 21:46:29 +00:00
Tommy Nyquist
9b4af06d02 Revert "Add std::move() to local base::Callback instances in //content"
This reverts commit c55a5a47ee.

Reason for revert: Speculative revert because of renderer crashes across
the board on Android with:
[FATAL:weak_ptr.cc(26)] Check failed: sequence_checker_.CalledOnValidSequence(). WeakPtrs must be checked on the same sequenced thread

Original change's description:
> Add std::move() to local base::Callback instances in //content
> 
> This adds std::move() around base::Callback instances where it looks
> relevant, by applying `base_bind_rewriters -rewriter=add_std_move`.
> https://crrev.com/c/970143, plus manual fixes.
> 
> Example:
>   // Before:
>   void set_callback(base::Closure cb) { g_cb = cb; }
>   void RunCallback(base::Callback<void(int)> cb) { cb.Run(42); }
>   void Post() {
>     base::Closure task = base::Bind(&Foo);
>     PostTask(FROM_HERE, task);
>   }
> 
>   // After:
>   void set_callback(base::Closure cb) { g_cb = std::move(cb); }
>   void RunCallback(base::Callback<void(int)> cb) { std::move(cb).Run(42); }
>   void Post() {
>     base::Closure task = base::Bind(&Foo);
>     PostTask(FROM_HERE, std::move(task));
>   }
> 
> Specifically, it inserts std::move() if:
>  - it's a pass-by-value parameter or non-const local variable.
>  - the occurrence is the latest in its control flow.
>  - no pointer is taken for the variable.
>  - no capturing lambda exists for the variable.
> 
> Change-Id: I53853f9b9c8604994e2065af66ed4607af9c12ed
> Reviewed-on: https://chromium-review.googlesource.com/970056
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544356}

TBR=kinuko@chromium.org,tzik@chromium.org

Change-Id: Ie7392a2229e1ef0f740d8958f8fe43d99b0460e9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/972321
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544527}
2018-03-20 21:37:17 +00:00
tzik
c55a5a47ee Add std::move() to local base::Callback instances in //content
This adds std::move() around base::Callback instances where it looks
relevant, by applying `base_bind_rewriters -rewriter=add_std_move`.
https://crrev.com/c/970143, plus manual fixes.

Example:
  // Before:
  void set_callback(base::Closure cb) { g_cb = cb; }
  void RunCallback(base::Callback<void(int)> cb) { cb.Run(42); }
  void Post() {
    base::Closure task = base::Bind(&Foo);
    PostTask(FROM_HERE, task);
  }

  // After:
  void set_callback(base::Closure cb) { g_cb = std::move(cb); }
  void RunCallback(base::Callback<void(int)> cb) { std::move(cb).Run(42); }
  void Post() {
    base::Closure task = base::Bind(&Foo);
    PostTask(FROM_HERE, std::move(task));
  }

Specifically, it inserts std::move() if:
 - it's a pass-by-value parameter or non-const local variable.
 - the occurrence is the latest in its control flow.
 - no pointer is taken for the variable.
 - no capturing lambda exists for the variable.

Change-Id: I53853f9b9c8604994e2065af66ed4607af9c12ed
Reviewed-on: https://chromium-review.googlesource.com/970056
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544356}
2018-03-20 14:35:44 +00:00
Brett Wilson
1c99002ae1 Replace tracked_objects::Location with base::Location.
Part 3/4 to replace these namespace qualifications.

TBR=dcheng@chromium.org

Bug: 763556
Change-Id: I052b146dd2ce9f5e3997ab5ae1e362b2422fb03e
Reviewed-on: https://chromium-review.googlesource.com/662140
Reviewed-by: Brett Wilson <brettw@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501376}
2017-09-12 20:11:15 +00:00
Gabriel Charette
5ff87ceb5f Automated IWYU fix for TaskRunner includes.
Pre-requisite to suppress message_loop.h from run_loop.h (issue 703346).

A similar include fix for message_loop.h was done in r471412 but some
targets are the removal as it looks like they properly didn't need
message_loop.h but were relying on it to get task runner includes...

The reasoning for this is the same as was once done in https://codereview.chromium.org/2443103003
(it was then done only on targets that failed to compile instead of via script):
scoped_refptr<Foo> requires full type of Foo to be defined not just fwd-declared.

Script used:

def Fix(file_path):
  content = refactor_lib.ReadFile(file_path)

  if not 'TaskRunner' in content:
    return False

  # Assume fwd-decls are correct in first pass.
  if 'class TaskRunner;' in content:
    return False
  if 'class SequencedTaskRunner;' in content:
    return False
  if 'class SingleThreadTaskRunner;' in content:
    return False

  # Using base:: prefix ensures we don't match fwd-decls and other things.
  # Will require a few fixups for missing includes in //base proper.
  # Complex prefix in regex attempts to skip comments.
  matches = re.compile(r'(private:|protected:|public:)|(\n *[^/\n][^/\n][^/\n]*base::(Sequenced|SingleThread)TaskRunner\b(>&|\*)?)', re.DOTALL).findall(content)

  if not matches:
    return False

  # Ignore instances in private sections (probably members or worst case methods
  # only used by impl which must include header already).
  in_private_section = False

  found_task_runner = False
  found_sequenced_task_runner = False
  found_single_thread_task_runner = False
  for match in matches:
    if match[0] == 'private:':
      in_private_section = True
      continue
    if match[0] == 'protected:':
      in_private_section = False
      continue
    if match[0] == 'public:':
      in_private_section = False
      continue

    # Otherwise match[0] was empty and we have a match[1] for the main thing.
    assert not match[0]

    # Only want to add the include if we don't have a match[3] (which indicates
    # this match is for a ref or a pointer).
    if match[3]:
      continue

    # Not a ref nor a pointer, count it if not in a private section, match[2]
    # tells which TaskRunner type it is.
    if not in_private_section:
      if not match[2]:
        found_task_runner = True
      elif match[2] == 'Sequenced':
        found_sequenced_task_runner = True
      elif match[2] == 'SingleThread':
        found_single_thread_task_runner = True
      else:
        assert False

  updated_content = content

  if found_task_runner:
    updated_content = refactor_lib.AddInclude(file_path, content, "base/task_runner.h")
  if found_sequenced_task_runner:
    updated_content = refactor_lib.AddInclude(file_path, content, "base/sequenced_task_runner.h")
  if found_single_thread_task_runner:
    updated_content = refactor_lib.AddInclude(file_path, content, "base/single_thread_task_runner.h")

  if updated_content == content:
    return False

  # Write updated file
  refactor_lib.WriteFile(file_path, updated_content)

  return True

TBR=gab@chromiu.org
BUG=703346
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2884763002 .
Cr-Commit-Position: refs/heads/master@{#472157}
2017-05-16 18:05:52 +00:00
peary2
3322df623f Introduce TaskRunner::RunsTasksInCurrentSequence() to replace
TaskRunner::RunsTasksOnCurrentThread().

Make TaskRunner::RunsTasksOnCurrentThread() non-virtual and define
it in terms of TaskRunner::RunsTasksInCurrentSequence().

It's the first step to rename RunsTasksOnCurrentThread()
to RunsTasksInCurrentSequence().

BUG=665062
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2823103003
Cr-Commit-Position: refs/heads/master@{#470173}
2017-05-09 03:55:48 +00:00
tzik
6e42784f92 Migrate base::TaskRunner from Closure to OnceClosure
After this CL, TaskRunner::PostTask and its family can take OnceClosure
in addition to Closure.

Most of the changes are mechanical replacement of Closure with OnceClosure
on TaskRunner family. Others are:
 - Limit CriticalClosure from Closure to OnceClosure as no caller call
   the resulting callback more than once
 - Add several PostTaskAndReplyWithResult overloads for old Callback
   version, for compatibility. (in base/task_scheduler/post_task.h)
 - Update SequencedWorkerPool implementation for OnceClosure.
 - Update task handling code in app_state.mm for OnceClosure, which is
   needed to bring OnceClosure into a ObjC block.

BUG=704027
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2637843002
Cr-Commit-Position: refs/heads/master@{#462023}
2017-04-05 10:13:21 +00:00
tzik
070c8ffb44 Pass Callback to TaskRunner by value and consume it on invocation
This is a preparation CL for http://crrev.com/2637843002, which replaces
the Callback parameter of TaskRunner::PostTask with OnceCallback.
This one replaces the passed-by-const-ref Callback parameter of
TaskRunner::PostTask() with pass-by-value.

With the pass-by-const-ref manner as the old code does, we can't avoid
leaving a reference to the callback object on the original thread. That
is, the callback object may be destroyed either on the target thread or
the original thread. That's problematic when a non-thread-safe object is
bound to the callback.

Pass-by-value and move() in this CL mitigate the nondeterminism: if the
caller of TaskRunner::PostTask() passes the callback object as rvalue,
TaskRunner::PostTask() leaves no reference on the original thread.
I.e. the reference is not left if the callback is passed directly from
Bind(), or passed with std::move() as below.

  task_runner->PostTask(FROM_HERE, base::Bind(&Foo));

  base::Closure cb = base::Bind(&Foo);
  task_runner->PostTask(FROM_HERE, std::move(cb));

Otherwise, if the caller passes the callback as lvalue, a reference to
the callback is left on the original thread as we do in the previous code.
I.e. a reference is left if the callback is passed from other non-temporary
variable.

  base::Closure cb = base::Bind(&Foo);
  task_runner->PostTask(FROM_HERE, cb);

This is less controversial part of http://crrev.com/2637843002. This CL
is mainly to land it incrementally.

TBR=shrike@chromium.org
BUG=704027
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2726523002
Cr-Commit-Position: refs/heads/master@{#460288}
2017-03-29 05:28:12 +00:00
dcheng
fa85b15991 Standardize usage of virtual/override/final specifiers.
The Google C++ style guide states:

  Explicitly annotate overrides of virtual functions or virtual
  destructors with an override or (less frequently) final specifier.
  Older (pre-C++11) code will use the virtual keyword as an inferior
  alternative annotation. For clarity, use exactly one of override,
  final, or virtual when declaring an override.

To better conform to these guidelines, the following constructs have
been rewritten:

- if a base class has a virtual destructor, then:
    virtual ~Foo();                   ->  ~Foo() override;
- virtual void Foo() override;        ->  void Foo() override;
- virtual void Foo() override final;  ->  void Foo() final;

This patch was automatically generated. The clang plugin can generate
fixit hints, which are suggested edits when it is 100% sure it knows how
to fix a problem. The hints from the clang plugin were applied to the
source tree using the tool in https://codereview.chromium.org/598073004.

BUG=417463
R=nasko@chromium.org

Review URL: https://codereview.chromium.org/678073006

Cr-Commit-Position: refs/heads/master@{#301534}
2014-10-28 01:13:59 +00:00
dcheng
c2282aa891 Standardize usage of virtual/override/final in content/browser/
This patch was automatically generated by applying clang fixit hints
generated by the plugin to the source tree.

BUG=417463
TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/667943003

Cr-Commit-Position: refs/heads/master@{#300469}
2014-10-21 12:08:25 +00:00
mohan.reddy
7fc3ac7d5a Replace FINAL and OVERRIDE with their C++11 counterparts in content
This step is a giant search and replace for OVERRIDE and FINAL to
replace them with their lowercase versions.

BUG=417463

Review URL: https://codereview.chromium.org/637183002

Cr-Commit-Position: refs/heads/master@{#298804}
2014-10-09 05:24:24 +00:00
aberent@chromium.org
b07c081d2c Empty startup task queue when tasks run synchronously
Also empty it when a task fails, and ensure that callback is only
called once. Extend the tests to check all this.

TEST=content_unittests --gtest_filter='StartupTaskRunner*'
BUG=304108

Review URL: https://codereview.chromium.org/25348004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@228010 0039d316-1c4b-4281-b951-d872f2087c98
2013-10-10 21:39:15 +00:00
aberent@chromium.org
232e09d18f Allow overlapping sync and async startup requests
On Android we can get a second request to start the browser while
the an asynchronous request is in progress. Since the second
request may be synchronous, we may have switch to completing
initialization synchronously. This patch handles this by tracking
which initialization tasks have been run, and running the remaining
initialization tasks.

BUG=260574

Review URL: https://chromiumcodereview.appspot.com/22691002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@219795 0039d316-1c4b-4281-b951-d872f2087c98
2013-08-27 15:29:56 +00:00
aberent@chromium.org
57624abcbe Run the later parts of startup as UI thread tasks
This CL splits the later parts of startup, from thread creation onwards,
into multiple UI thread tasks. Depending on the StartupTaskRunner passed
to CreateThreads the tasks are all run immediately, or are queued one at
a time on the UI thread. This, on platforms where the UI is
already running, allows the UI to remain interactive while Chrome is
initialized.

BUG=231856

Review URL: https://chromiumcodereview.appspot.com/19957002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@215042 0039d316-1c4b-4281-b951-d872f2087c98
2013-08-01 16:01:51 +00:00