Skip to content

Commit 8078848

Browse files
kblokkblok
authored
[BREAKING] Refactor tree manager (#2187)
* Some progress * Refactor tree manager * Fix some tests * Initialize FrameTree * Fix MainFrame * Some fixes
1 parent 5018c83 commit 8078848

File tree

12 files changed

+170
-164
lines changed

12 files changed

+170
-164
lines changed

‎lib/PuppeteerSharp.Tests/OOPIFTests/OOPIFTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ await FrameUtils.AttachFrameAsync(
3838
TestConstants.CrossProcessHttpPrefix + "/empty.html"
3939
);
4040
await frameTask.WithTimeout();
41-
Assert.Equal(2, Page.MainFrame.ChildFrames.Count);
41+
Assert.Equal(2, Page.MainFrame.ChildFrames.Count());
4242
}
4343

4444
[PuppeteerTest("oopif.spec.ts", "OOPIF", "should track navigations within OOP iframes")]
@@ -258,7 +258,7 @@ await FrameUtils.AttachFrameAsync(
258258
"frame1",
259259
TestConstants.CrossProcessHttpPrefix + "/empty.html"
260260
).WithTimeout();
261-
var frame1 = oopIframe.ChildFrames[0];
261+
var frame1 = oopIframe.ChildFrames.First();
262262
Assert.Contains("empty.html", frame1.Url);
263263
await FrameUtils.NavigateFrameAsync(
264264
oopIframe,

‎lib/PuppeteerSharp/ElementHandle.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ public async Task<IFrame> ContentFrameAsync()
345345
ObjectId = RemoteObject.ObjectId,
346346
}).ConfigureAwait(false);
347347

348-
return string.IsNullOrEmpty(nodeInfo.Node.FrameId) ? null : await _frameManager.GetFrameAsync(nodeInfo.Node.FrameId).ConfigureAwait(false);
348+
return string.IsNullOrEmpty(nodeInfo.Node.FrameId) ? null : await _frameManager.FrameTree.GetFrameAsync(nodeInfo.Node.FrameId).ConfigureAwait(false);
349349
}
350350

351351
/// <inheritdoc/>

‎lib/PuppeteerSharp/Frame.cs‎

Lines changed: 6 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Linq;
43
using System.Threading.Tasks;
54
using Newtonsoft.Json.Linq;
65
using PuppeteerSharp.Helpers;
@@ -11,33 +10,20 @@ namespace PuppeteerSharp
1110
/// <inheritdoc/>
1211
public class Frame : IFrame
1312
{
14-
private readonly List<IFrame> _childFrames = new();
15-
16-
internal Frame(FrameManager frameManager, Frame parentFrame, string frameId, CDPSession client)
13+
internal Frame(FrameManager frameManager, string frameId, string parentFrameId, CDPSession client)
1714
{
1815
FrameManager = frameManager;
19-
ParentFrame = parentFrame;
2016
Id = frameId;
2117
Client = client;
18+
ParentId = parentFrameId;
2219

2320
LifecycleEvents = new List<string>();
2421

25-
parentFrame?.AddChildFrame(this);
26-
2722
UpdateClient(client);
2823
}
2924

3025
/// <inheritdoc/>
31-
public List<IFrame> ChildFrames
32-
{
33-
get
34-
{
35-
lock (_childFrames)
36-
{
37-
return _childFrames.ToList();
38-
}
39-
}
40-
}
26+
public IEnumerable<IFrame> ChildFrames => FrameManager.FrameTree.GetChildFrames(Id);
4127

4228
/// <inheritdoc/>
4329
public string Name { get; private set; }
@@ -49,14 +35,16 @@ public List<IFrame> ChildFrames
4935
public bool Detached { get; set; }
5036

5137
/// <inheritdoc/>
52-
public IFrame ParentFrame { get; private set; }
38+
public IFrame ParentFrame => FrameManager.FrameTree.GetParentFrame(Id);
5339

5440
/// <inheritdoc/>
5541
public bool IsOopFrame => Client != FrameManager.Client;
5642

5743
/// <inheritdoc/>
5844
public string Id { get; internal set; }
5945

46+
internal string ParentId { get; }
47+
6048
internal FrameManager FrameManager { get; }
6149

6250
internal string LoaderId { get; set; }
@@ -333,22 +321,6 @@ public Task ClickAsync(string selector, ClickOptions options = null)
333321
public Task TypeAsync(string selector, string text, TypeOptions options = null)
334322
=> PuppeteerWorld.TypeAsync(selector, text, options);
335323

336-
internal void AddChildFrame(Frame frame)
337-
{
338-
lock (_childFrames)
339-
{
340-
_childFrames.Add(frame);
341-
}
342-
}
343-
344-
internal void RemoveChildFrame(Frame frame)
345-
{
346-
lock (_childFrames)
347-
{
348-
_childFrames.Remove(frame);
349-
}
350-
}
351-
352324
internal void OnLoadingStarted() => HasStartedLoading = true;
353325

354326
internal void OnLoadingStopped()
@@ -382,12 +354,6 @@ internal void Detach()
382354
Detached = true;
383355
MainWorld.Detach();
384356
PuppeteerWorld.Detach();
385-
if (ParentFrame != null)
386-
{
387-
((Frame)ParentFrame).RemoveChildFrame(this);
388-
}
389-
390-
ParentFrame = null;
391357
}
392358

393359
internal void UpdateClient(CDPSession client)

‎lib/PuppeteerSharp/FrameManager.cs‎

Lines changed: 53 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,19 @@ internal class FrameManager
1818

1919
private readonly ConcurrentDictionary<string, ExecutionContext> _contextIdToContext;
2020
private readonly ILogger _logger;
21-
private readonly ConcurrentDictionary<string, Frame> _frames;
22-
private readonly AsyncDictionaryHelper<string, Frame> _asyncFrames;
2321
private readonly List<string> _isolatedWorlds = new();
22+
private readonly List<string> _frameNavigatedReceived = new();
2423
private readonly TaskQueue _eventsQueue = new();
2524
private bool _ensureNewDocumentNavigation;
2625

2726
internal FrameManager(CDPSession client, Page page, bool ignoreHTTPSErrors, TimeoutSettings timeoutSettings)
2827
{
2928
Client = client;
3029
Page = page;
31-
_frames = new ConcurrentDictionary<string, Frame>();
3230
_contextIdToContext = new ConcurrentDictionary<string, ExecutionContext>();
3331
_logger = Client.Connection.LoggerFactory.CreateLogger<FrameManager>();
3432
NetworkManager = new NetworkManager(client, ignoreHTTPSErrors, this);
3533
TimeoutSettings = timeoutSettings;
36-
_asyncFrames = new AsyncDictionaryHelper<string, Frame>(_frames, "Frame {0} not found");
3734

3835
Client.MessageReceived += Client_MessageReceived;
3936
}
@@ -54,12 +51,14 @@ internal FrameManager(CDPSession client, Page page, bool ignoreHTTPSErrors, Time
5451

5552
internal NetworkManager NetworkManager { get; }
5653

57-
internal Frame MainFrame { get; set; }
54+
internal Frame MainFrame => FrameTree.MainFrame;
5855

5956
internal Page Page { get; }
6057

6158
internal TimeoutSettings TimeoutSettings { get; }
6259

60+
internal FrameTree FrameTree { get; private set; } = new();
61+
6362
public async Task<IResponse> NavigateFrameAsync(Frame frame, string url, NavigationOptions options)
6463
{
6564
var referrer = string.IsNullOrEmpty(options.Referer)
@@ -129,7 +128,7 @@ await Task.WhenAll(
129128
getFrameTreeTask,
130129
autoAttachTask).ConfigureAwait(false);
131130

132-
await HandleFrameTreeAsync(client, new FrameTree(getFrameTreeTask.Result.FrameTree)).ConfigureAwait(false);
131+
await HandleFrameTreeAsync(client, getFrameTreeTask.Result.FrameTree).ConfigureAwait(false);
133132

134133
await Task.WhenAll(
135134
client.SendAsync("Page.setLifecycleEventsEnabled", new PageSetLifecycleEventsEnabledRequest { Enabled = true }),
@@ -173,21 +172,16 @@ internal void OnAttachedToTarget(TargetChangedArgs e)
173172
return;
174173
}
175174

176-
_frames.TryGetValue(e.TargetInfo.TargetId, out var frame);
177-
if (frame != null)
178-
{
179-
frame.UpdateClient(e.Target.Session);
180-
}
175+
var frame = GetFrame(e.TargetInfo.TargetId);
176+
frame?.UpdateClient(e.Target.Session);
181177

182178
e.Target.Session.MessageReceived += Client_MessageReceived;
183179
_ = InitializeAsync(e.Target.Session);
184180
}
185181

186-
internal Frame[] GetFrames() => _frames.Values.ToArray();
187-
188-
internal Task<Frame> GetFrameAsync(string frameId) => _asyncFrames.GetItemAsync(frameId);
182+
internal Frame GetFrame(string frameid) => FrameTree.GetById(frameid);
189183

190-
internal Task<Frame> TryGetFrameAsync(string frameId) => _asyncFrames.TryGetItemAsync(frameId);
184+
internal Frame[] GetFrames() => FrameTree.Frames;
191185

192186
private async Task NavigateAsync(CDPSession client, string url, string referrer, string frameId)
193187
{
@@ -266,15 +260,14 @@ private void Client_MessageReceived(object sender, MessageEventArgs e)
266260

267261
private void OnFrameStartedLoading(BasicFrameResponse e)
268262
{
269-
if (_frames.TryGetValue(e.FrameId, out var frame))
270-
{
271-
frame.OnLoadingStarted();
272-
}
263+
var frame = GetFrame(e.FrameId);
264+
frame?.OnLoadingStarted();
273265
}
274266

275267
private void OnFrameStoppedLoading(BasicFrameResponse e)
276268
{
277-
if (_frames.TryGetValue(e.FrameId, out var frame))
269+
var frame = GetFrame(e.FrameId);
270+
if (frame != null)
278271
{
279272
frame.OnLoadingStopped();
280273
LifecycleEvent?.Invoke(this, new FrameEventArgs(frame));
@@ -283,7 +276,8 @@ private void OnFrameStoppedLoading(BasicFrameResponse e)
283276

284277
private void OnLifeCycleEvent(LifecycleEventResponse e)
285278
{
286-
if (_frames.TryGetValue(e.FrameId, out var frame))
279+
var frame = GetFrame(e.FrameId);
280+
if (frame != null)
287281
{
288282
frame.OnLifecycleEvent(e.LoaderId, e.Name);
289283
LifecycleEvent?.Invoke(this, new FrameEventArgs(frame));
@@ -318,7 +312,7 @@ private void OnExecutionContextDestroyed(int contextId, CDPSession session)
318312
private async Task OnExecutionContextCreatedAsync(ContextPayload contextPayload, CDPSession session)
319313
{
320314
var frameId = contextPayload.AuxData?.FrameId;
321-
var frame = !string.IsNullOrEmpty(frameId) ? await GetFrameAsync(frameId).ConfigureAwait(false) : null;
315+
var frame = !string.IsNullOrEmpty(frameId) ? await FrameTree.GetFrameAsync(frameId).ConfigureAwait(false) : null;
322316
IsolatedWorld world = null;
323317

324318
if (frame != null)
@@ -350,7 +344,8 @@ private async Task OnExecutionContextCreatedAsync(ContextPayload contextPayload,
350344

351345
private void OnFrameDetached(PageFrameDetachedResponse e)
352346
{
353-
if (_frames.TryGetValue(e.FrameId, out var frame))
347+
var frame = GetFrame(e.FrameId);
348+
if (frame != null)
354349
{
355350
if (e.Reason == FrameDetachedReason.Remove)
356351
{
@@ -365,17 +360,21 @@ private void OnFrameDetached(PageFrameDetachedResponse e)
365360

366361
private async Task OnFrameNavigatedAsync(FramePayload framePayload)
367362
{
363+
// This is in the event handler upstream.
364+
// It's more consistent having this here.
365+
_frameNavigatedReceived.Add(framePayload.Id);
366+
368367
var isMainFrame = string.IsNullOrEmpty(framePayload.ParentId);
369-
var frame = isMainFrame ? MainFrame : await GetFrameAsync(framePayload.Id).ConfigureAwait(false);
368+
var frame = isMainFrame ? MainFrame : await FrameTree.GetFrameAsync(framePayload.Id).ConfigureAwait(false);
370369

371370
Contract.Assert(isMainFrame || frame != null, "We either navigate top level or have old version of the navigated frame");
372371

373372
// Detach all child frames first.
374373
if (frame != null)
375374
{
376-
while (frame.ChildFrames.Count > 0)
375+
while (frame.ChildFrames.Any())
377376
{
378-
RemoveFramesRecursively(frame.ChildFrames[0]);
377+
RemoveFramesRecursively(frame.ChildFrames.First() as Frame);
379378
}
380379
}
381380

@@ -384,22 +383,16 @@ private async Task OnFrameNavigatedAsync(FramePayload framePayload)
384383
{
385384
if (frame != null)
386385
{
387-
// Update frame id to retain frame identity on cross-process navigation.
388-
if (frame.Id != null)
389-
{
390-
_frames.TryRemove(frame.Id, out _);
391-
}
392-
386+
FrameTree.RemoveFrame(frame);
393387
frame.Id = framePayload.Id;
394388
}
395389
else
396390
{
397391
// Initial main frame navigation.
398-
frame = new Frame(this, null, framePayload.Id, Client);
392+
frame = new Frame(this, framePayload.Id, null, Client);
399393
}
400394

401-
_asyncFrames.AddItem(framePayload.Id, frame);
402-
MainFrame = frame;
395+
FrameTree.AddFrame(frame);
403396
}
404397

405398
// Update frame payload.
@@ -410,7 +403,8 @@ private async Task OnFrameNavigatedAsync(FramePayload framePayload)
410403

411404
private void OnFrameNavigatedWithinDocument(NavigatedWithinDocumentResponse e)
412405
{
413-
if (_frames.TryGetValue(e.FrameId, out var frame))
406+
var frame = GetFrame(e.FrameId);
407+
if (frame != null)
414408
{
415409
frame.NavigatedWithinDocument(e.Url);
416410

@@ -420,15 +414,15 @@ private void OnFrameNavigatedWithinDocument(NavigatedWithinDocumentResponse e)
420414
}
421415
}
422416

423-
private void RemoveFramesRecursively(IFrame frame)
417+
private void RemoveFramesRecursively(Frame frame)
424418
{
425-
while (frame.ChildFrames.Count > 0)
419+
while (frame.ChildFrames.Any())
426420
{
427-
RemoveFramesRecursively(frame.ChildFrames[0]);
421+
RemoveFramesRecursively(frame.ChildFrames.First() as Frame);
428422
}
429423

430-
((Frame)frame).Detach();
431-
_frames.TryRemove(frame.Id, out _);
424+
frame.Detach();
425+
FrameTree.RemoveFrame(frame);
432426
FrameDetached?.Invoke(this, new FrameEventArgs(frame));
433427
}
434428

@@ -437,37 +431,41 @@ private void OnFrameAttached(CDPSession session, PageFrameAttachedResponse frame
437431

438432
private void OnFrameAttached(CDPSession session, string frameId, string parentFrameId)
439433
{
440-
if (_frames.TryGetValue(frameId, out var existingFrame))
434+
var frame = GetFrame(frameId);
435+
if (frame != null)
441436
{
442-
if (session != null && existingFrame.IsOopFrame)
437+
if (session != null && frame.IsOopFrame)
443438
{
444-
existingFrame.UpdateClient(session);
439+
frame.UpdateClient(session);
445440
}
446441

447442
return;
448443
}
449444

450-
if (!_frames.ContainsKey(frameId) && _frames.ContainsKey(parentFrameId))
451-
{
452-
var parentFrame = _frames[parentFrameId];
453-
var frame = new Frame(this, parentFrame, frameId, session);
454-
_asyncFrames.AddItem(frame.Id, frame);
455-
FrameAttached?.Invoke(this, new FrameEventArgs(frame));
456-
}
445+
frame = new Frame(this, frameId, parentFrameId, session);
446+
FrameTree.AddFrame(frame);
447+
FrameAttached?.Invoke(this, new FrameEventArgs(frame));
457448
}
458449

459-
private async Task HandleFrameTreeAsync(CDPSession session, FrameTree frameTree)
450+
private async Task HandleFrameTreeAsync(CDPSession session, PageGetFrameTree frameTree)
460451
{
461452
if (!string.IsNullOrEmpty(frameTree.Frame.ParentId))
462453
{
463454
OnFrameAttached(session, frameTree.Frame.Id, frameTree.Frame.ParentId);
464455
}
465456

466-
await OnFrameNavigatedAsync(frameTree.Frame).ConfigureAwait(false);
457+
if (!_frameNavigatedReceived.Contains(frameTree.Frame.Id))
458+
{
459+
await OnFrameNavigatedAsync(frameTree.Frame).ConfigureAwait(false);
460+
}
461+
else
462+
{
463+
_frameNavigatedReceived.Remove(frameTree.Frame.Id);
464+
}
467465

468-
if (frameTree.Childs != null)
466+
if (frameTree.ChildFrames != null)
469467
{
470-
foreach (var child in frameTree.Childs)
468+
foreach (var child in frameTree.ChildFrames)
471469
{
472470
await HandleFrameTreeAsync(session, child).ConfigureAwait(false);
473471
}

0 commit comments

Comments
 (0)