Bug 210182 - Make more use of FrameLoader pageID/frameID getters
Summary: Make more use of FrameLoader pageID/frameID getters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-08 04:35 PDT by Rob Buis
Modified: 2020-04-08 13:15 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.84 KB, patch)
2020-04-08 04:37 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (10.31 KB, patch)
2020-04-08 09:59 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2020-04-08 04:35:56 PDT
Remove pageID/frameID getters from FrameLoader since these
are not called, instead FrameLoaderClient API is used.
Comment 1 Rob Buis 2020-04-08 04:37:00 PDT
Created attachment 395791 [details]
Patch
Comment 2 Chris Dumez 2020-04-08 08:18:30 PDT
Comment on attachment 395791 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395791&action=review

> Source/WebCore/loader/FrameLoader.cpp:154
> +#define PAGE_ID ((client().pageID().valueOr(PageIdentifier())).toUInt64())

Well it was used here and the getter did not do any harm. I don't understand why we want to drop these getters.
Comment 3 Rob Buis 2020-04-08 08:23:50 PDT
FrameLoader is quite big and complicated, I am trying to slim it down (will take a long time obviously). If you do not like this particular patch too much, how about at least making it private and not exported? I'll rename the bug of course.
Comment 4 Chris Dumez 2020-04-08 08:25:32 PDT
(In reply to Rob Buis from comment #3)
> FrameLoader is quite big and complicated, I am trying to slim it down (will
> take a long time obviously). If you do not like this particular patch too
> much, how about at least making it private and not exported? I'll rename the
> bug of course.

If anything, I think that if anything that is using FrameLoader::client().frameID() should be fixed to use FrameLoader::frameID() instead.
Comment 5 Rob Buis 2020-04-08 09:59:19 PDT
Created attachment 395824 [details]
Patch
Comment 6 Rob Buis 2020-04-08 11:40:20 PDT
(In reply to Chris Dumez from comment #4)
> (In reply to Rob Buis from comment #3)
> > FrameLoader is quite big and complicated, I am trying to slim it down (will
> > take a long time obviously). If you do not like this particular patch too
> > much, how about at least making it private and not exported? I'll rename the
> > bug of course.
> 
> If anything, I think that if anything that is using
> FrameLoader::client().frameID() should be fixed to use
> FrameLoader::frameID() instead.

Ah, I had not considered that. Done!
Comment 7 EWS 2020-04-08 13:14:55 PDT
Committed r259752: <https://trac.webkit.org/changeset/259752>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395824 [details].
Comment 8 Radar WebKit Bug Importer 2020-04-08 13:15:14 PDT
<rdar://problem/61472882>