Support creating the WorkerNode before the final script URL is available

This CL adds WorkerNodeImpl::OnFinalResponseURLDetermined() so that the
|url_| field can be set when it's value is available.

Bug: 1048332
Change-Id: Ida299a8b587aa7753d30023e0d36d11907037d4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036366
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738323}

chrome/browser/ui/webui/discards/graph_dump_impl.cc

@@ -287,6 +287,11 @@ void DiscardsGraphDumpImpl::OnBeforeWorkerNodeRemoved(
   SendDeletionNotification(worker_node);
 }
 
+void DiscardsGraphDumpImpl::OnFinalResponseURLDetermined(
+    const performance_manager::WorkerNode* worker_node) {
+  SendWorkerNotification(worker_node, false);
+}
+
 void DiscardsGraphDumpImpl::OnClientFrameAdded(
     const performance_manager::WorkerNode* worker_node,
     const performance_manager::FrameNode* client_frame_node) {

chrome/browser/ui/webui/discards/graph_dump_impl.h

@@ -153,6 +153,8 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
       const performance_manager::WorkerNode* worker_node) override;
   void OnBeforeWorkerNodeRemoved(
       const performance_manager::WorkerNode* worker_node) override;
+  void OnFinalResponseURLDetermined(
+      const performance_manager::WorkerNode* worker_node) override;
   void OnClientFrameAdded(
       const performance_manager::WorkerNode* worker_node,
       const performance_manager::FrameNode* client_frame_node) override;

components/performance_manager/graph/worker_node_impl.cc

@@ -13,13 +13,11 @@ WorkerNodeImpl::WorkerNodeImpl(GraphImpl* graph,
                                const std::string& browser_context_id,
                                WorkerType worker_type,
                                ProcessNodeImpl* process_node,
-                               const GURL& url,
                                const base::UnguessableToken& dev_tools_token)
     : TypedNodeBase(graph),
       browser_context_id_(browser_context_id),
       worker_type_(worker_type),
       process_node_(process_node),
-      url_(url),
       dev_tools_token_(dev_tools_token) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
   DCHECK(process_node);
@@ -95,6 +93,15 @@ void WorkerNodeImpl::RemoveClientWorker(WorkerNodeImpl* worker_node) {
   DCHECK_EQ(removed, 1u);
 }
 
+void WorkerNodeImpl::OnFinalResponseURLDetermined(const GURL& url) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(url_.is_empty());
+  url_ = url;
+
+  for (auto* observer : GetObservers())
+    observer->OnFinalResponseURLDetermined(this);
+}
+
 const std::string& WorkerNodeImpl::browser_context_id() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return browser_context_id_;

components/performance_manager/graph/worker_node_impl.h

@@ -31,7 +31,6 @@ class WorkerNodeImpl
                  const std::string& browser_context_id,
                  WorkerType worker_type,
                  ProcessNodeImpl* process_node,
-                 const GURL& url,
                  const base::UnguessableToken& dev_tools_token);
   ~WorkerNodeImpl() override;
 
@@ -43,14 +42,18 @@ class WorkerNodeImpl
   void AddClientWorker(WorkerNodeImpl* worker_node);
   void RemoveClientWorker(WorkerNodeImpl* worker_node);
 
+  // Invoked when the worker script was fetched and the final response URL is
+  // available.
+  void OnFinalResponseURLDetermined(const GURL& url);
+
   // Getters for const properties. These can be called from any thread.
   const std::string& browser_context_id() const;
   WorkerType worker_type() const;
   ProcessNodeImpl* process_node() const;
-  const GURL& url() const;
   const base::UnguessableToken& dev_tools_token() const;
 
   // Getters for non-const properties. These are not thread safe.
+  const GURL& url() const;
   const base::flat_set<FrameNodeImpl*>& client_frames() const;
   const base::flat_set<WorkerNodeImpl*>& client_workers() const;
   const base::flat_set<WorkerNodeImpl*>& child_workers() const;
@@ -64,8 +67,8 @@ class WorkerNodeImpl
   WorkerType GetWorkerType() const override;
   const std::string& GetBrowserContextID() const override;
   const ProcessNode* GetProcessNode() const override;
-  const GURL& GetURL() const override;
   const base::UnguessableToken& GetDevToolsToken() const override;
+  const GURL& GetURL() const override;
   const base::flat_set<const FrameNode*> GetClientFrames() const override;
   const base::flat_set<const WorkerNode*> GetClientWorkers() const override;
   const base::flat_set<const WorkerNode*> GetChildWorkers() const override;
@@ -83,14 +86,16 @@ class WorkerNodeImpl
   // The process in which this worker lives.
   ProcessNodeImpl* const process_node_;
 
-  // The URL of the worker script.
-  const GURL url_;
-
   // A unique identifier shared with all representations of this node across
   // content and blink. The token is only defined by the browser process and
   // is never sent back from the renderer in control calls.
   const base::UnguessableToken dev_tools_token_;
 
+  // The URL of the worker script. This is the final response URL which takes
+  // into account redirections. This is initially empty and it is set when
+  // OnFinalResponseURLDetermined() is invoked.
+  GURL url_;
+
   // Frames that are clients of this worker.
   base::flat_set<FrameNodeImpl*> client_frames_;
 

components/performance_manager/graph/worker_node_impl_unittest.cc

@@ -49,19 +49,16 @@ TEST_F(WorkerNodeImplTest, ConstProperties) {
   const std::string kTestBrowserContextId =
       base::UnguessableToken::Create().ToString();
   auto process = CreateNode<ProcessNodeImpl>();
-  static const GURL kTestUrl("testurl.com");
   static const base::UnguessableToken kTestDevToolsToken =
       base::UnguessableToken::Create();
 
-  auto worker_impl = CreateNode<WorkerNodeImpl>(kWorkerType, process.get(),
-                                                kTestBrowserContextId, kTestUrl,
-                                                kTestDevToolsToken);
+  auto worker_impl = CreateNode<WorkerNodeImpl>(
+      kWorkerType, process.get(), kTestBrowserContextId, kTestDevToolsToken);
 
   // Test private interface.
   EXPECT_EQ(worker_impl->browser_context_id(), kTestBrowserContextId);
   EXPECT_EQ(worker_impl->worker_type(), kWorkerType);
   EXPECT_EQ(worker_impl->process_node(), process.get());
-  EXPECT_EQ(worker_impl->url(), kTestUrl);
   EXPECT_EQ(worker_impl->dev_tools_token(), kTestDevToolsToken);
 
   // Test public interface.
@@ -70,10 +67,24 @@ TEST_F(WorkerNodeImplTest, ConstProperties) {
   EXPECT_EQ(worker->GetBrowserContextID(), kTestBrowserContextId);
   EXPECT_EQ(worker->GetWorkerType(), kWorkerType);
   EXPECT_EQ(worker->GetProcessNode(), process.get());
-  EXPECT_EQ(worker->GetURL(), kTestUrl);
   EXPECT_EQ(worker->GetDevToolsToken(), kTestDevToolsToken);
 }
 
+TEST_F(WorkerNodeImplTest, OnFinalResponseURLDetermined) {
+  auto process = CreateNode<ProcessNodeImpl>();
+  static const GURL kTestUrl("testurl.com");
+
+  auto worker_impl = CreateNode<WorkerNodeImpl>(WorkerNode::WorkerType::kShared,
+                                                process.get());
+
+  // Initially empty.
+  EXPECT_TRUE(worker_impl->url().is_empty());
+
+  // Set when OnFinalResponseURLDetermined() is called.
+  worker_impl->OnFinalResponseURLDetermined(kTestUrl);
+  EXPECT_EQ(worker_impl->url(), kTestUrl);
+}
+
 // Create a worker of each type and register the frame as a client of each.
 TEST_F(WorkerNodeImplTest, AddWorkerNodes) {
   auto process = CreateNode<ProcessNodeImpl>();
@@ -199,6 +210,10 @@ class TestWorkerNodeObserver : public WorkerNodeObserver {
     EXPECT_EQ(client_workers_.erase(worker_node), 1u);
   }
 
+  void OnFinalResponseURLDetermined(const WorkerNode* worker_node) override {
+    on_final_response_url_determined_called_ = true;
+  }
+
   void OnClientFrameAdded(const WorkerNode* worker_node,
                           const FrameNode* client_frame_node) override {
     auto& client_frames = client_frames_.find(worker_node)->second;
@@ -221,16 +236,22 @@ class TestWorkerNodeObserver : public WorkerNodeObserver {
     EXPECT_EQ(client_workers.erase(client_worker_node), 1u);
   }
 
+  bool on_final_response_url_determined_called() const {
+    return on_final_response_url_determined_called_;
+  }
+
   const base::flat_map<const WorkerNode*, base::flat_set<const FrameNode*>>&
-  client_frames() {
+  client_frames() const {
     return client_frames_;
   }
   const base::flat_map<const WorkerNode*, base::flat_set<const WorkerNode*>>&
-  client_workers() {
+  client_workers() const {
     return client_workers_;
   }
 
  private:
+  bool on_final_response_url_determined_called_ = false;
+
   base::flat_map<const WorkerNode*, base::flat_set<const FrameNode*>>
       client_frames_;
   base::flat_map<const WorkerNode*, base::flat_set<const WorkerNode*>>
@@ -319,4 +340,19 @@ TEST_F(WorkerNodeImplTest, Observer_ClientsOfServiceWorkers) {
   graph()->RemoveWorkerNodeObserver(&worker_node_observer);
 }
 
+TEST_F(WorkerNodeImplTest, Observer_OnFinalResponseURLDetermined) {
+  TestWorkerNodeObserver worker_node_observer;
+  graph()->AddWorkerNodeObserver(&worker_node_observer);
+
+  auto process = CreateNode<ProcessNodeImpl>();
+  auto worker = CreateNode<WorkerNodeImpl>(WorkerNode::WorkerType::kDedicated,
+                                           process.get());
+
+  EXPECT_FALSE(worker_node_observer.on_final_response_url_determined_called());
+  worker->OnFinalResponseURLDetermined(GURL("testurl.com"));
+  EXPECT_TRUE(worker_node_observer.on_final_response_url_determined_called());
+
+  graph()->RemoveWorkerNodeObserver(&worker_node_observer);
+}
+
 }  // namespace performance_manager

components/performance_manager/performance_manager_impl.cc

@@ -133,11 +133,10 @@ std::unique_ptr<WorkerNodeImpl> PerformanceManagerImpl::CreateWorkerNode(
     const std::string& browser_context_id,
     WorkerNode::WorkerType worker_type,
     ProcessNodeImpl* process_node,
-    const GURL& url,
     const base::UnguessableToken& dev_tools_token) {
   return CreateNodeImpl<WorkerNodeImpl>(
       base::OnceCallback<void(WorkerNodeImpl*)>(), browser_context_id,
-      worker_type, process_node, url, dev_tools_token);
+      worker_type, process_node, dev_tools_token);
 }
 
 void PerformanceManagerImpl::BatchDeleteNodes(

components/performance_manager/performance_manager_impl.h

@@ -96,7 +96,6 @@ class PerformanceManagerImpl : public PerformanceManager {
       const std::string& browser_context_id,
       WorkerNode::WorkerType worker_type,
       ProcessNodeImpl* process_node,
-      const GURL& url,
       const base::UnguessableToken& dev_tools_token);
 
   // Destroys a node returned from the creation functions above.

components/performance_manager/public/graph/worker_node.h

@@ -70,12 +70,13 @@ class WorkerNode : public Node {
   // over the lifetime of the frame.
   virtual const ProcessNode* GetProcessNode() const = 0;
 
-  // Returns the URL of the worker script.
-  virtual const GURL& GetURL() const = 0;
-
   // Returns the dev tools token for this worker.
   virtual const base::UnguessableToken& GetDevToolsToken() const = 0;
 
+  // Returns the URL of the worker script. This is the final response URL which
+  // takes into account redirections.
+  virtual const GURL& GetURL() const = 0;
+
   // Returns the frames that are clients of this worker.
   virtual const base::flat_set<const FrameNode*> GetClientFrames() const = 0;
 
@@ -116,6 +117,10 @@ class WorkerNodeObserver {
 
   // Notifications of property changes.
 
+  // Invoked when the final url of the worker script has been determined, which
+  // happens when the script has finished loading.
+  virtual void OnFinalResponseURLDetermined(const WorkerNode* worker_node) = 0;
+
   // Invoked when |client_frame_node| becomes a client of |worker_node|.
   virtual void OnClientFrameAdded(const WorkerNode* worker_node,
                                   const FrameNode* client_frame_node) = 0;
@@ -151,6 +156,7 @@ class WorkerNode::ObserverDefaultImpl : public WorkerNodeObserver {
   // Called when a |worker_node| is added to the graph.
   void OnWorkerNodeAdded(const WorkerNode* worker_node) override {}
   void OnBeforeWorkerNodeRemoved(const WorkerNode* worker_node) override {}
+  void OnFinalResponseURLDetermined(const WorkerNode* worker_node) override {}
   void OnClientFrameAdded(const WorkerNode* worker_node,
                           const FrameNode* client_frame_node) override {}
   void OnBeforeClientFrameRemoved(const WorkerNode* worker_node,

components/performance_manager/test_support/graph_test_harness.h

@@ -125,10 +125,9 @@ struct TestNodeWrapper<WorkerNodeImpl>::Factory {
       WorkerNode::WorkerType worker_type,
       ProcessNodeImpl* process_node,
       const std::string& browser_context_id = std::string(),
-      const GURL& url = GURL(),
       const base::UnguessableToken& token = base::UnguessableToken::Create()) {
-    return std::make_unique<WorkerNodeImpl>(
-        graph, browser_context_id, worker_type, process_node, url, token);
+    return std::make_unique<WorkerNodeImpl>(graph, browser_context_id,
+                                            worker_type, process_node, token);
   }
 };
 

components/performance_manager/worker_watcher.cc

@@ -103,8 +103,7 @@ void WorkerWatcher::OnWorkerStarted(
     const base::UnguessableToken& dev_tools_token) {
   auto worker_node = PerformanceManagerImpl::GetInstance()->CreateWorkerNode(
       browser_context_id_, WorkerNode::WorkerType::kShared,
-      process_node_source_->GetProcessNode(worker_process_id), instance.url(),
-      dev_tools_token);
+      process_node_source_->GetProcessNode(worker_process_id), dev_tools_token);
   bool inserted =
       shared_worker_nodes_.emplace(instance, std::move(worker_node)).second;
   DCHECK(inserted);