BASELINE: Update Chromium to 88.0.4324.208

Change-Id: I3ae87d23e4eff4b4a469685658740a213600c667
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

12 files changed, 136 insertions, 108 deletions

diff --git a/chromium/components/update_client/component.cc b/chromium/components/update_client/component.cc
index c014a3011b0..5973357da2a 100644
--- a/chromium/components/update_client/component.cc
+++ b/chromium/components/update_client/component.cc
@@ -8,8 +8,8 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/callback_helpers.h"
 #include "base/check_op.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -386,9 +386,6 @@ void Component::SetUpdateCheckResult(
 
   if (result)
     SetParseResult(result.value());
-
-  base::SequencedTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, std::move(update_check_complete_));
 }
 
 void Component::NotifyWait() {
@@ -593,6 +590,23 @@ void Component::StateNew::DoHandle() {
   auto& component = State::component();
   if (component.crx_component()) {
     TransitionState(std::make_unique<StateChecking>(&component));
+
+    // Notify that the component is being checked for updates after the
+    // transition to `StateChecking` occurs. This event indicates the start
+    // of the update check. The component receives the update check results when
+    // the update checks completes, and after that, `UpdateEngine` invokes the
+    // function `StateChecking::DoHandle` to transition the component out of
+    // the `StateChecking`. The current design allows for notifying observers
+    // on state transitions but it does not allow such notifications when a
+    // new state is entered. Hence, posting the task below is a workaround for
+    // this design oversight.
+    base::SequencedTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            [](Component& component) {
+              component.NotifyObservers(Events::COMPONENT_CHECKING_FOR_UPDATES);
+            },
+            std::ref(component)));
   } else {
     component.error_code_ = static_cast<int>(Error::CRX_NOT_FOUND);
     component.error_category_ = ErrorCategory::kService;
@@ -601,47 +615,37 @@ void Component::StateNew::DoHandle() {
 }
 
 Component::StateChecking::StateChecking(Component* component)
-    : State(component, ComponentState::kChecking) {}
+    : State(component, ComponentState::kChecking) {
+  component->last_check_ = base::TimeTicks::Now();
+}
 
 Component::StateChecking::~StateChecking() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
-// Unlike how other states are handled, this function does not change the
-// state right away. The state transition happens when the UpdateChecker
-// calls Component::UpdateCheckComplete and |update_check_complete_| is invoked.
-// This is an artifact of how multiple components must be checked for updates
-// together but the state machine defines the transitions for one component
-// at a time.
 void Component::StateChecking::DoHandle() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   auto& component = State::component();
   DCHECK(component.crx_component());
 
-  component.last_check_ = base::TimeTicks::Now();
-  component.update_check_complete_ = base::BindOnce(
-      &Component::StateChecking::UpdateCheckComplete, base::Unretained(this));
-
-  component.NotifyObservers(Events::COMPONENT_CHECKING_FOR_UPDATES);
-}
+  if (component.error_code_) {
+    TransitionState(std::make_unique<StateUpdateError>(&component));
+    return;
+  }
 
-void Component::StateChecking::UpdateCheckComplete() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  auto& component = State::component();
-  if (!component.error_code_) {
-    if (component.status_ == "ok") {
-      TransitionState(std::make_unique<StateCanUpdate>(&component));
-      return;
-    }
+  if (component.status_ == "ok") {
+    TransitionState(std::make_unique<StateCanUpdate>(&component));
+    return;
+  }
 
-    if (component.status_ == "noupdate") {
-      if (component.action_run_.empty())
-        TransitionState(std::make_unique<StateUpToDate>(&component));
-      else
-        TransitionState(std::make_unique<StateRun>(&component));
-      return;
+  if (component.status_ == "noupdate") {
+    if (component.action_run_.empty()) {
+      TransitionState(std::make_unique<StateUpToDate>(&component));
+    } else {
+      TransitionState(std::make_unique<StateRun>(&component));
     }
+    return;
   }
 
   TransitionState(std::make_unique<StateUpdateError>(&component));
diff --git a/chromium/components/update_client/component.h b/chromium/components/update_client/component.h
index 1b371dad2ac..46b38607d87 100644
--- a/chromium/components/update_client/component.h
+++ b/chromium/components/update_client/component.h
@@ -206,8 +206,6 @@ class Component {
    private:
     // State overrides.
     void DoHandle() override;
-
-    void UpdateCheckComplete();
   };
 
   class StateUpdateError : public State {
@@ -489,8 +487,6 @@ class Component {
   std::unique_ptr<State> state_;
   const UpdateContext& update_context_;
 
-  base::OnceClosure update_check_complete_;
-
   ComponentState previous_state_ = ComponentState::kLastStatus;
 
   // True if this component has reached a final state because all its states
diff --git a/chromium/components/update_client/crx_downloader_unittest.cc b/chromium/components/update_client/crx_downloader_unittest.cc
index b2a98b78027..5ffb3373b07 100644
--- a/chromium/components/update_client/crx_downloader_unittest.cc
+++ b/chromium/components/update_client/crx_downloader_unittest.cc
@@ -12,7 +12,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
-#include "base/test/bind_test_util.h"
+#include "base/test/bind.h"
 #include "base/test/task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
diff --git a/chromium/components/update_client/net/url_loader_post_interceptor.cc b/chromium/components/update_client/net/url_loader_post_interceptor.cc
index f6a48340963..778307197b4 100644
--- a/chromium/components/update_client/net/url_loader_post_interceptor.cc
+++ b/chromium/components/update_client/net/url_loader_post_interceptor.cc
@@ -9,7 +9,7 @@
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
-#include "base/test/bind_test_util.h"
+#include "base/test/bind.h"
 #include "components/update_client/test_configurator.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
diff --git a/chromium/components/update_client/ping_manager_unittest.cc b/chromium/components/update_client/ping_manager_unittest.cc
index 1d5994c50c3..b38196f9f74 100644
--- a/chromium/components/update_client/ping_manager_unittest.cc
+++ b/chromium/components/update_client/ping_manager_unittest.cc
@@ -103,7 +103,7 @@ void PingManagerTest::PingSentCallback(int error, const std::string& response) {
 scoped_refptr<UpdateContext> PingManagerTest::MakeMockUpdateContext() const {
   return base::MakeRefCounted<UpdateContext>(
       config_, false, std::vector<std::string>(),
-      UpdateClient::CrxDataCallback(), UpdateClient::CrxStateChangeCallback(),
+      UpdateClient::CrxStateChangeCallback(),
       UpdateEngine::NotifyObserversCallback(), UpdateEngine::Callback(),
       nullptr);
 }
diff --git a/chromium/components/update_client/request_sender.cc b/chromium/components/update_client/request_sender.cc
index 9e8fee89862..3fc7cf7aea2 100644
--- a/chromium/components/update_client/request_sender.cc
+++ b/chromium/components/update_client/request_sender.cc
@@ -8,7 +8,7 @@
 
 #include "base/base64.h"
 #include "base/bind.h"
-#include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
diff --git a/chromium/components/update_client/update_checker_unittest.cc b/chromium/components/update_client/update_checker_unittest.cc
index 7717154d9ba..8ed5ffb688f 100644
--- a/chromium/components/update_client/update_checker_unittest.cc
+++ b/chromium/components/update_client/update_checker_unittest.cc
@@ -21,7 +21,7 @@
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/task/post_task.h"
-#include "base/test/bind_test_util.h"
+#include "base/test/bind.h"
 #include "base/test/task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/version.h"
@@ -227,7 +227,7 @@ void UpdateCheckerTest::UpdateCheckComplete(
 scoped_refptr<UpdateContext> UpdateCheckerTest::MakeMockUpdateContext() const {
   return base::MakeRefCounted<UpdateContext>(
       config_, false, std::vector<std::string>(),
-      UpdateClient::CrxDataCallback(), UpdateClient::CrxStateChangeCallback(),
+      UpdateClient::CrxStateChangeCallback(),
       UpdateEngine::NotifyObserversCallback(), UpdateEngine::Callback(),
       nullptr);
 }
diff --git a/chromium/components/update_client/update_client.h b/chromium/components/update_client/update_client.h
index be4f70211ba..f462b297243 100644
--- a/chromium/components/update_client/update_client.h
+++ b/chromium/components/update_client/update_client.h
@@ -331,6 +331,12 @@ using Callback = base::OnceCallback<void(Error error)>;
 // the browser process has gone single-threaded.
 class UpdateClient : public base::RefCountedThreadSafe<UpdateClient> {
  public:
+  // Returns `CrxComponent` instances corresponding to the component ids
+  // passed as an argument to the callback. The order of components in the input
+  // and output vectors must match. If the instance of the `CrxComponent` is not
+  // available for some reason, implementors of the callback must not skip
+  // skip the component, and instead, they must insert a `nullopt` value in
+  // the output vector.
   using CrxDataCallback =
       base::OnceCallback<std::vector<base::Optional<CrxComponent>>(
           const std::vector<std::string>& ids)>;
diff --git a/chromium/components/update_client/update_client_errors.h b/chromium/components/update_client/update_client_errors.h
index 1cf13a8c05f..61d70b20c91 100644
--- a/chromium/components/update_client/update_client_errors.h
+++ b/chromium/components/update_client/update_client_errors.h
@@ -18,6 +18,7 @@ enum class Error {
   UPDATE_CHECK_ERROR = 5,
   CRX_NOT_FOUND = 6,
   INVALID_ARGUMENT = 7,
+  BAD_CRX_DATA_CALLBACK = 8,
   MAX_VALUE,
 };
 
diff --git a/chromium/components/update_client/update_client_unittest.cc b/chromium/components/update_client/update_client_unittest.cc
index 31b6a55c2b2..5907ba967e8 100644
--- a/chromium/components/update_client/update_client_unittest.cc
+++ b/chromium/components/update_client/update_client_unittest.cc
@@ -4587,4 +4587,60 @@ TEST_F(UpdateClientTest, CustomAttributeNoUpdate) {
 
   EXPECT_EQ(1, observer.calls);
 }
+
+// Tests the scenario where `CrxDataCallback` returns a vector whose elements
+// don't include a value for one of the component ids specified by the `ids`
+// parameter of the `UpdateClient::Update` function. Expects the completion
+// callback to include a specific error, and no other events and pings be
+// generated, since the update engine rejects the UpdateClient::Update call.
+TEST_F(UpdateClientTest, BadCrxDataCallback) {
+  class CompletionCallbackMock {
+   public:
+    static void Callback(base::OnceClosure quit_closure, Error error) {
+      EXPECT_EQ(Error::BAD_CRX_DATA_CALLBACK, error);
+      std::move(quit_closure).Run();
+    }
+  };
+
+  class MockPingManager : public MockPingManagerImpl {
+   public:
+    explicit MockPingManager(scoped_refptr<Configurator> config)
+        : MockPingManagerImpl(config) {}
+
+   protected:
+    ~MockPingManager() override { EXPECT_TRUE(ping_data().empty()); }
+  };
+
+  scoped_refptr<UpdateClient> update_client =
+      base::MakeRefCounted<UpdateClientImpl>(
+          config(), base::MakeRefCounted<MockPingManager>(config()),
+          UpdateChecker::Factory{});
+
+  MockObserver observer;
+
+  std::vector<CrxUpdateItem> items;
+  auto receiver = base::MakeRefCounted<MockCrxStateChangeReceiver>();
+  EXPECT_CALL(*receiver, Receive(_))
+      .WillRepeatedly(
+          [&items](const CrxUpdateItem& item) { items.push_back(item); });
+
+  update_client->AddObserver(&observer);
+  const std::vector<std::string> ids = {"jebgalgnebhfojomionfpkfelancnnkf",
+                                        "gjpmebpgbhcamgdgjcmnjfhggjpgcimm"};
+  // The `CrxDataCallback` argument only returns a value for the first
+  // component id. This means that its result is ill formed, and the `Update`
+  // call completes with an error.
+  update_client->Update(
+      ids, base::BindOnce([](const std::vector<std::string>& ids) {
+        EXPECT_EQ(ids.size(), size_t{2});
+        return std::vector<base::Optional<CrxComponent>>{base::nullopt};
+      }),
+      base::BindRepeating(&MockCrxStateChangeReceiver::Receive, receiver), true,
+      base::BindOnce(&CompletionCallbackMock::Callback, quit_closure()));
+  RunThreads();
+
+  EXPECT_TRUE(items.empty());
+  update_client->RemoveObserver(&observer);
+}
+
 }  // namespace update_client
diff --git a/chromium/components/update_client/update_engine.cc b/chromium/components/update_client/update_engine.cc
index 57816ebc2f3..0a4c2d615ea 100644
--- a/chromium/components/update_client/update_engine.cc
+++ b/chromium/components/update_client/update_engine.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/callback_helpers.h"
 #include "base/check_op.h"
 #include "base/guid.h"
 #include "base/location.h"
@@ -32,7 +33,6 @@ UpdateContext::UpdateContext(
     scoped_refptr<Configurator> config,
     bool is_foreground,
     const std::vector<std::string>& ids,
-    UpdateClient::CrxDataCallback crx_data_callback,
     UpdateClient::CrxStateChangeCallback crx_state_change_callback,
     const UpdateEngine::NotifyObserversCallback& notify_observers_callback,
     UpdateEngine::Callback callback,
@@ -41,7 +41,6 @@ UpdateContext::UpdateContext(
       is_foreground(is_foreground),
       enabled_component_updates(config->EnabledComponentUpdates()),
       ids(ids),
-      crx_data_callback(std::move(crx_data_callback)),
       crx_state_change_callback(crx_state_change_callback),
       notify_observers_callback(notify_observers_callback),
       callback(std::move(callback)),
@@ -93,22 +92,25 @@ void UpdateEngine::Update(
     return;
   }
 
+  // Calls out to get the corresponding CrxComponent data for the components.
+  const std::vector<base::Optional<CrxComponent>> crx_components =
+      std::move(crx_data_callback).Run(ids);
+  if (crx_components.size() < ids.size()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(std::move(callback), Error::BAD_CRX_DATA_CALLBACK));
+    return;
+  }
+
   const auto update_context = base::MakeRefCounted<UpdateContext>(
-      config_, is_foreground, ids, std::move(crx_data_callback),
-      crx_state_change_callback, notify_observers_callback_,
-      std::move(callback), metadata_.get());
+      config_, is_foreground, ids, crx_state_change_callback,
+      notify_observers_callback_, std::move(callback), metadata_.get());
   DCHECK(!update_context->session_id.empty());
 
   const auto result = update_contexts_.insert(
       std::make_pair(update_context->session_id, update_context));
   DCHECK(result.second);
 
-  // Calls out to get the corresponding CrxComponent data for the CRXs in this
-  // update context.
-  const auto crx_components =
-      std::move(update_context->crx_data_callback).Run(update_context->ids);
-  DCHECK_EQ(update_context->ids.size(), crx_components.size());
-
   for (size_t i = 0; i != update_context->ids.size(); ++i) {
     const auto& id = update_context->ids[i];
 
@@ -139,33 +141,6 @@ void UpdateEngine::Update(
     return;
   }
 
-  for (const auto& id : update_context->components_to_check_for_updates)
-    update_context->components[id]->Handle(
-        base::BindOnce(&UpdateEngine::ComponentCheckingForUpdatesStart, this,
-                       update_context, id));
-}
-
-void UpdateEngine::ComponentCheckingForUpdatesStart(
-    scoped_refptr<UpdateContext> update_context,
-    const std::string& id) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(update_context);
-
-  DCHECK_EQ(1u, update_context->components.count(id));
-  DCHECK(update_context->components.at(id));
-
-  // Handle |kChecking| state.
-  auto& component = *update_context->components.at(id);
-  component.Handle(
-      base::BindOnce(&UpdateEngine::ComponentCheckingForUpdatesComplete, this,
-                     update_context));
-
-  ++update_context->num_components_ready_to_check;
-  if (update_context->num_components_ready_to_check <
-      update_context->components_to_check_for_updates.size()) {
-    return;
-  }
-
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(&UpdateEngine::DoUpdateCheck, this, update_context));
@@ -175,6 +150,10 @@ void UpdateEngine::DoUpdateCheck(scoped_refptr<UpdateContext> update_context) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(update_context);
 
+  // Make the components transition from |kNew| to |kChecking| state.
+  for (const auto& id : update_context->components_to_check_for_updates)
+    update_context->components[id]->Handle(base::DoNothing());
+
   update_context->update_checker =
       update_checker_factory_(config_, metadata_.get());
 
@@ -221,6 +200,9 @@ void UpdateEngine::UpdateCheckResultsAvailable(
       component->SetUpdateCheckResult(base::nullopt,
                                       ErrorCategory::kUpdateCheck, error);
     }
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(&UpdateEngine::UpdateCheckComplete, this,
+                                  update_context));
     return;
   }
 
@@ -261,18 +243,6 @@ void UpdateEngine::UpdateCheckResultsAvailable(
           static_cast<int>(ProtocolError::UPDATE_RESPONSE_NOT_FOUND));
     }
   }
-}
-
-void UpdateEngine::ComponentCheckingForUpdatesComplete(
-    scoped_refptr<UpdateContext> update_context) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(update_context);
-
-  ++update_context->num_components_checked;
-  if (update_context->num_components_checked <
-      update_context->components_to_check_for_updates.size()) {
-    return;
-  }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
@@ -284,9 +254,17 @@ void UpdateEngine::UpdateCheckComplete(
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(update_context);
 
-  for (const auto& id : update_context->components_to_check_for_updates)
+  for (const auto& id : update_context->components_to_check_for_updates) {
     update_context->component_queue.push(id);
 
+    // Handle the |kChecking| state and transition the component to the
+    // next state, depending on the update check results.
+    DCHECK_EQ(1u, update_context->components.count(id));
+    auto& component = update_context->components.at(id);
+    DCHECK_EQ(component->state(), ComponentState::kChecking);
+    component->Handle(base::DoNothing());
+  }
+
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(&UpdateEngine::HandleComponent, this, update_context));
@@ -407,7 +385,7 @@ void UpdateEngine::SendUninstallPing(const std::string& id,
 
   const auto update_context = base::MakeRefCounted<UpdateContext>(
       config_, false, std::vector<std::string>{id},
-      UpdateClient::CrxDataCallback(), UpdateClient::CrxStateChangeCallback(),
+      UpdateClient::CrxStateChangeCallback(),
       UpdateEngine::NotifyObserversCallback(), std::move(callback),
       metadata_.get());
   DCHECK(!update_context->session_id.empty());
@@ -437,7 +415,7 @@ void UpdateEngine::SendRegistrationPing(const std::string& id,
 
   const auto update_context = base::MakeRefCounted<UpdateContext>(
       config_, false, std::vector<std::string>{id},
-      UpdateClient::CrxDataCallback(), UpdateClient::CrxStateChangeCallback(),
+      UpdateClient::CrxStateChangeCallback(),
       UpdateEngine::NotifyObserversCallback(), std::move(callback),
       metadata_.get());
   DCHECK(!update_context->session_id.empty());
diff --git a/chromium/components/update_client/update_engine.h b/chromium/components/update_client/update_engine.h
index 6adba5d2620..a6f291cf5f0 100644
--- a/chromium/components/update_client/update_engine.h
+++ b/chromium/components/update_client/update_engine.h
@@ -80,13 +80,6 @@ class UpdateEngine : public base::RefCountedThreadSafe<UpdateEngine> {
 
   void UpdateComplete(scoped_refptr<UpdateContext> update_context, Error error);
 
-  void ComponentCheckingForUpdatesStart(
-      scoped_refptr<UpdateContext> update_context,
-      const std::string& id);
-  void ComponentCheckingForUpdatesComplete(
-      scoped_refptr<UpdateContext> update_context);
-  void UpdateCheckComplete(scoped_refptr<UpdateContext> update_context);
-
   void DoUpdateCheck(scoped_refptr<UpdateContext> update_context);
   void UpdateCheckResultsAvailable(
       scoped_refptr<UpdateContext> update_context,
@@ -94,6 +87,7 @@ class UpdateEngine : public base::RefCountedThreadSafe<UpdateEngine> {
       ErrorCategory error_category,
       int error,
       int retry_after_sec);
+  void UpdateCheckComplete(scoped_refptr<UpdateContext> update_context);
 
   void HandleComponent(scoped_refptr<UpdateContext> update_context);
   void HandleComponentComplete(scoped_refptr<UpdateContext> update_context);
@@ -127,7 +121,6 @@ struct UpdateContext : public base::RefCountedThreadSafe<UpdateContext> {
       scoped_refptr<Configurator> config,
       bool is_foreground,
       const std::vector<std::string>& ids,
-      UpdateClient::CrxDataCallback crx_data_callback,
       UpdateClient::CrxStateChangeCallback crx_state_change_callback,
       const UpdateEngine::NotifyObserversCallback& notify_observers_callback,
       UpdateEngine::Callback callback,
@@ -150,9 +143,6 @@ struct UpdateContext : public base::RefCountedThreadSafe<UpdateContext> {
   // Contains the map of ids to components for all the CRX in this context.
   IdToComponentPtrMap components;
 
-  // Called before an update check, when update metadata is needed.
-  UpdateEngine::CrxDataCallback crx_data_callback;
-
   // Called when the observable state of the CRX component has changed.
   UpdateClient::CrxStateChangeCallback crx_state_change_callback;
 
@@ -177,9 +167,6 @@ struct UpdateContext : public base::RefCountedThreadSafe<UpdateContext> {
   // The error reported by the update checker.
   int update_check_error = 0;
 
-  size_t num_components_ready_to_check = 0;
-  size_t num_components_checked = 0;
-
   // Contains the ids of the components that the state machine must handle.
   base::queue<std::string> component_queue;