Bug 191853

Summary: Remove @no-unify of InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp
Product: WebKit Reporter: Fujii Hironori <fujii.hironori>
Component: WebKit2Assignee: Fujii Hironori <fujii.hironori>
Status: REOPENED    
Severity: Normal CC: aakash_jain, commit-queue, ews-watchlist, keith_miller, mark.lam, mcatanzaro, ryanhaddad, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=189467
https://bugs.webkit.org/show_bug.cgi?id=189400
Bug Depends on: 191909, 191995    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch for landing none

Fujii Hironori
Reported 2018-11-19 23:35:08 PST
Remove @no-unify of InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp In r235845, I excluded InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp from unify source builds in order to work around a MSVC bug. <https://trac.webkit.org/changeset/235845> I commited a different workaround for the MSVC bug in r238386. Now, we can include InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp in unified source builds. <https://trac.webkit.org/changeset/238386>
Attachments
Patch (4.37 KB, patch)
2018-11-19 23:57 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.43 MB, application/zip)
2018-11-20 01:59 PST, EWS Watchlist 		no flags
Details
Patch for landing (4.39 KB, patch)
2018-11-27 17:46 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2018-11-19 23:57:16 PST
Created attachment 355316 [details] Patch
EWS Watchlist
Comment 2 2018-11-20 01:59:57 PST Comment hidden (obsolete)
EWS Watchlist
Comment 3 2018-11-20 01:59:59 PST Comment hidden (obsolete)
Tim Horton
Comment 4 2018-11-20 10:39:44 PST
Nice
Fujii Hironori
Comment 5 2018-11-21 17:28:37 PST
Comment on attachment 355316 [details] Patch Clearing flags on attachment: 355316 Committed r238432: <https://trac.webkit.org/changeset/238432>
Fujii Hironori
Comment 6 2018-11-21 17:28:41 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2018-11-21 17:29:30 PST
<rdar://problem/46205899>
Mark Lam
Comment 8 2018-11-22 09:34:43 PST
r238432 was rolled out in r238446: <http://trac.webkit.org/r238446>.
Mark Lam
Comment 9 2018-11-22 09:35:45 PST
I'm not sure we actually need to land this patch (since it doesn't add much value). We rolled it out because it introduced a build breakage in internal Mac builds.
alan
Comment 10 2018-11-22 10:09:38 PST
(In reply to Mark Lam from comment #9) > I'm not sure we actually need to land this patch (since it doesn't add much > value). We rolled it out because it introduced a build breakage in internal > Mac builds. Thanks Mark, I was about to roll this out.
Michael Catanzaro
Comment 11 2018-11-23 09:03:58 PST
Can you post a build log?
Michael Catanzaro
Comment 12 2018-11-23 09:05:23 PST
Maybe we need an EWS for the internal builds; otherwise it's not really possible to avoid breaking it. Especially with unified builds. If you have different unified source bundles in the internal build, all bets are off....
Tim Horton
Comment 13 2018-11-23 13:33:08 PST
(In reply to Michael Catanzaro from comment #12) > Maybe we need an EWS for the internal builds; otherwise it's not really > possible to avoid breaking it. Especially with unified builds. If you have > different unified source bundles in the internal build, all bets are off.... That's a bit tricky to do safely. Anyway, in this particular case it looks like the usual problem of "someone's got a 'using namespace WebCore' nearby the changed file".
Tim Horton
Comment 14 2018-11-23 13:35:25 PST
Could be WKBundlePageOverlay
Fujii Hironori
Comment 15 2018-11-27 17:46:24 PST
Created attachment 355829 [details] Patch for landing Bug 191995 has addressed. Can I try again?
Tim Horton
Comment 16 2018-11-27 20:00:16 PST
(In reply to Fujii Hironori from comment #15) > Created attachment 355829 [details] > Patch for landing > > Bug 191995 has addressed. Can I try again? I don’t see why not! (Though I will note that morning PST would probably be a better bet if you want someone on #webkit to check on the internal builds).
Fujii Hironori
Comment 17 2018-11-27 21:58:34 PST
Hmm, it doesn't sound good to me (I'll be sleeping in the time). I'd like to ask Apple folks to land my patch in morning.
Mark Lam
Comment 18 2018-11-28 09:50:53 PST
Landing it now.
WebKit Commit Bot
Comment 19 2018-11-28 10:16:19 PST
Comment on attachment 355829 [details] Patch for landing Clearing flags on attachment: 355829 Committed r238620: <https://trac.webkit.org/changeset/238620>
WebKit Commit Bot
Comment 20 2018-11-28 10:16:21 PST
All reviewed patches have been landed. Closing bug.
Ryan Haddad
Comment 21 2018-11-28 11:05:27 PST
Reverted r238620 for reason: Broke internal builds again. Committed r238625: <https://trac.webkit.org/changeset/238625>
Ryan Haddad
Comment 22 2018-11-28 11:18:58 PST
Sorry for the churn here. Someone will work with you in the next few days to get this sorted out.
Note You need to log in before you can comment on or make changes to this bug.