Bug 83257 - Add notifications for frame loader deferring and resuming
Summary: Add notifications for frame loader deferring and resuming
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Leo Yang
URL:
Keywords:
Depends on:
Blocks: 73144
  Show dependency tree / graph
 
Reported: 2012-04-05 01:06 PDT by Leo Yang
Modified: 2014-02-05 10:52 PST (History)
13 users (show)

See Also:

Attachments
Patch (4.49 KB, patch)
2012-04-05 01:52 PDT, Leo Yang 		no flags Details | Formatted Diff | Diff
Patch v2 (4.90 KB, patch)
2012-04-05 20:05 PDT, Leo Yang 		abarth: review-
Details | Formatted Diff | Diff
Patch v3 (5.88 KB, patch)
2012-04-09 18:53 PDT, Leo Yang 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Leo Yang 2012-04-05 01:06:28 PDT 
Some platforms (so far blackberry) are interested in notifications before a frame loader is deferred and after it is resumed.
Comment 1 Leo Yang 2012-04-05 01:52:05 PDT 
Created attachment 135783 [details]
Patch
Comment 2 Antonio Gomes 2012-04-05 07:40:42 PDT 
It looks good to me, but I will wait for some "FrameLoader guy" to say something. Btw, it would be nice to say a few words about the usage in the BlackBerry port.
Comment 3 Alexey Proskuryakov 2012-04-05 10:36:13 PDT 
This looks quite redundant - deferring doesn't happen all by itself, the client already knows about conditions that cause it.
Comment 4 Leo Yang 2012-04-05 19:19:16 PDT 
(In reply to comment #3)
> This looks quite redundant - deferring doesn't happen all by itself, the client already knows about conditions that cause it.

The caller does know the conditions that cause deferring and resuming but the notifications are not redundant. The notifications make caller's life easier, otherwise on every call sites the caller need to iterate the frame loaders for the page twice one for deferring and another one for resuming.
Comment 5 Leo Yang 2012-04-05 20:05:00 PDT 
Created attachment 135971 [details]
Patch v2

Adding explain that the notifications are used for the blackberry porting.
Comment 6 Alexey Proskuryakov 2012-04-05 20:51:38 PDT 
This change still doesn't make sense to me. Load deferring is an implementation detail of WebCore, clients needn't know about it.
Comment 7 Leo Yang 2012-04-05 21:02:08 PDT 
(In reply to comment #6)
> This change still doesn't make sense to me. Load deferring is an implementation detail of WebCore, clients needn't know about it.

Do you think page load is an implementation detail of WebCore? Is it FrameLoaderClient::dispatchDidCommitLoad() necessary for example?
Comment 8 Adam Barth 2012-04-06 11:10:48 PDT 
Comment on attachment 135971 [details]
Patch v2

I'm not sure it makes sense to notify the client here.  Can you explain more about how you're using this notification?
Comment 9 Leo Yang 2012-04-09 18:53:42 PDT 
Created attachment 136369 [details]
Patch v3

Adding more explaining.
Comment 10 Anders Carlsson 2014-02-05 10:52:45 PST 
Comment on attachment 136369 [details]
Patch v3

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.