Bug 44913

Summary: add missing parts of didFinishDocumentLoadForFrame
Product: WebKit Reporter: Alice Liu <alice.barraclough>
Component: WebKit2Assignee: Alice Liu <alice.barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, aroben, kbalazs, ossy, sam, webkit-ews, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
patch
darin: review-
patch with changelog, WKTR, MiniBrowser, and qt changes
none
(for real this time) patch with changelog, WKTR, MiniBrowser, and qt changes darin: review+

Description Alice Liu 2010-08-30 17:37:47 PDT
Created attachment 65984 [details]
patch

Support for didFinishDocumentLoadForFrame is currently only in InjectedBundlePageLoaderClient.  Add the rest of it.
Comment 1 Darin Adler 2010-08-30 17:42:10 PDT
Comment on attachment 65984 [details]
patch

Changes are good. But need a change log.
Comment 2 Adam Roben (:aroben) 2010-08-30 17:47:06 PDT
I think you also need to update DRT (and maybe MiniBrowser) to account for the new callback.
Comment 3 Early Warning System Bot 2010-08-30 17:50:03 PDT
Attachment 65984 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3815184
Comment 4 Sam Weinig 2010-08-30 17:53:55 PDT
(In reply to comment #2)
> I think you also need to update DRT (and maybe MiniBrowser) to account for the new callback.

And qwkpage.cpp.
Comment 5 Alice Liu 2010-08-30 18:45:45 PDT
Created attachment 65997 [details]
patch with changelog, WKTR, MiniBrowser, and qt changes

thanks everyone!
Comment 6 WebKit Review Bot 2010-08-30 18:47:14 PDT
Attachment 65997 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:91:  Extra space between WKPageDidFinishDocumentLoadForFrameCallback and didFinishDocumentLoadForFrame  [whitespace/declaration] [3]
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:77:  Extra space between WKBundlePageDidFinishDocumentLoadForFrameCallback and didFinishDocumentLoadForFrame  [whitespace/declaration] [3]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Alice Liu 2010-08-30 18:48:44 PDT
Created attachment 65998 [details]
(for real this time) patch with changelog, WKTR, MiniBrowser, and qt changes

lol what is wrong with me today
Comment 8 WebKit Review Bot 2010-08-30 18:50:06 PDT
Attachment 65998 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:91:  Extra space between WKPageDidFinishDocumentLoadForFrameCallback and didFinishDocumentLoadForFrame  [whitespace/declaration] [3]
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:77:  Extra space between WKBundlePageDidFinishDocumentLoadForFrameCallback and didFinishDocumentLoadForFrame  [whitespace/declaration] [3]
Total errors found: 2 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Early Warning System Bot 2010-08-30 18:56:24 PDT
Attachment 65997 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3854159
Comment 10 Csaba Osztrogonác 2010-08-30 22:54:10 PDT
http://trac.webkit.org/changeset/66448 broke Qt build:

../../../WebKit2/UIProcess/API/qt/qwkpage.cpp: In constructor ‘QWKPage::QWKPage(OpaqueWKPageNamespace*)’:
../../../WebKit2/UIProcess/API/qt/qwkpage.cpp:228: error: ‘qt_wk_didFinishDocumentLoadForFrame’ was not declared in this scope
Comment 11 Csaba Osztrogonác 2010-08-31 01:48:49 PDT
(In reply to comment #10)
> http://trac.webkit.org/changeset/66448 broke Qt build:
> 
> ../../../WebKit2/UIProcess/API/qt/qwkpage.cpp: In constructor ‘QWKPage::QWKPage(OpaqueWKPageNamespace*)’:
> ../../../WebKit2/UIProcess/API/qt/qwkpage.cpp:228: error: ‘qt_wk_didFinishDocumentLoadForFrame’ was not declared in this scope

Fix landed in http://trac.webkit.org/changeset/66466
Comment 12 Alice Liu 2010-08-31 11:40:41 PDT
my apologies.  thank you for the qt build fix. 

(In reply to comment #11)