Bug 138690

Summary: Support multiple signatures of diagnostic logging.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bweinstein, calvaris, commit-queue, darin, d-r, eric.carlson, esprehn+autocc, fmalita, glenn, gyuyoung.kim, japhet, pdr, philipj, schenney, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
andersca: review+
Patch none

Description Jer Noble 2014-11-13 01:02:04 PST 
Support multiple signatures of diagnostic logging.
Comment 1 Jer Noble 2014-11-13 01:11:31 PST 
Created attachment 241477 [details]
Patch
Comment 2 Anders Carlsson 2014-11-13 08:26:44 PST 
I think this client should live on MainFrame instead of Page. Darin, what do you think?
Comment 3 Darin Adler 2014-11-13 20:30:36 PST 
I agree. Almost anything new we think about adding to Page we could instead add to MainFrame.
Comment 4 Jer Noble 2014-11-13 21:31:32 PST 
Frame/MainFrame doesn't seem to have the same kind of "client" infrastructure as page does. In fact, it looks like Page passes its own loaderClientForMainFrame into MainFrame when it creates it.

Should MainFrame take a MainFrameClients struct (I guess passed into it inside of or part of PageClients) in its constructor?

And what makes this all weirder is that WebPage is created first, then Page, then MainFrame, then WebFrame. So if a client is going to exist at construction time, it's got to be WebPage which creates it.

We could have WebFrame set its client on MainFrame after they're all constructed, but that might result in logging messages getting dropped if we add logs which fire in between when Page is constructed and when WebFrame sets its clients on it.

And WebFrame currently doesn't have any of its own clients. It's given a client by WebPage.

So I guess I don't understand what makes MainFrame the best place to put these new clients?
Comment 5 Jer Noble 2014-11-13 21:42:51 PST 
    // FIXME: Rename this to PageConfiguration and move it to its own class.
    struct PageClients { ... };

This would be a lot easier if that FIXME was fixed.  As it is, the class-member-class PageClients can't be forward declared.
Comment 6 Jer Noble 2014-11-14 09:01:44 PST 
Created attachment 241592 [details]
Patch
Comment 7 Jer Noble 2014-11-14 13:26:57 PST 
Created attachment 241619 [details]
Patch
Comment 8 WebKit Commit Bot 2014-11-14 13:29:05 PST 
Attachment 241619 [details] did not pass style-queue:


ERROR: Source/WebCore/WebCore.exp.in:0:  Source/WebCore/WebCore.exp.in should be sorted, use Tools/Scripts/sort-export-file script  [list/order] [5]
Total errors found: 1 in 27 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Jer Noble 2014-11-14 14:18:52 PST 
Created attachment 241624 [details]
Patch
Comment 10 Jer Noble 2014-11-14 15:06:29 PST 
Created attachment 241629 [details]
Patch
Comment 11 Jer Noble 2014-11-14 15:38:31 PST 
Created attachment 241634 [details]
Patch
Comment 12 Jer Noble 2014-11-17 11:24:18 PST 
Created attachment 241728 [details]
Patch
Comment 13 Radar WebKit Bug Importer 2014-11-19 09:25:50 PST 
<rdar://problem/19031034>
Comment 14 Jer Noble 2014-11-21 15:58:23 PST 
Created attachment 242091 [details]
Patch
Comment 15 Jer Noble 2014-11-21 16:43:00 PST 
Created attachment 242093 [details]
Patch
Comment 16 Jer Noble 2014-11-21 21:27:05 PST 
Committed r176499: <http://trac.webkit.org/changeset/176499>