Bug 191853

Summary: Remove @no-unify of InjectedBundleRangeHandle.cpp and InjectedBundleNodeHandle.cpp
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: WebKit2Assignee: Fujii Hironori <Hironori.Fujii>
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

Description Fujii Hironori 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>
Comment 1 Fujii Hironori 2018-11-19 23:57:16 PST
Created attachment 355316 [details]
Patch
Comment 2 EWS Watchlist 2018-11-20 01:59:57 PST Comment hidden (obsolete)
Comment 3 EWS Watchlist 2018-11-20 01:59:59 PST Comment hidden (obsolete)
Comment 4 Tim Horton 2018-11-20 10:39:44 PST
Nice
Comment 5 Fujii Hironori 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>
Comment 6 Fujii Hironori 2018-11-21 17:28:41 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2018-11-21 17:29:30 PST
<rdar://problem/46205899>
Comment 8 Mark Lam 2018-11-22 09:34:43 PST
r238432 was rolled out in r238446: <http://trac.webkit.org/r238446>.
Comment 9 Mark Lam 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.
Comment 10 zalan 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.
Comment 11 Michael Catanzaro 2018-11-23 09:03:58 PST
Can you post a build log?
Comment 12 Michael Catanzaro 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....
Comment 13 Tim Horton 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".
Comment 14 Tim Horton 2018-11-23 13:35:25 PST
Could be WKBundlePageOverlay
Comment 15 Fujii Hironori 2018-11-27 17:46:24 PST
Created attachment 355829 [details]
Patch for landing

Bug 191995 has addressed. Can I try again?
Comment 16 Tim Horton 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).
Comment 17 Fujii Hironori 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.
Comment 18 Mark Lam 2018-11-28 09:50:53 PST
Landing it now.
Comment 19 WebKit Commit Bot 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>
Comment 20 WebKit Commit Bot 2018-11-28 10:16:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 21 Ryan Haddad 2018-11-28 11:05:27 PST
Reverted r238620 for reason:

Broke internal builds again.

Committed r238625: <https://trac.webkit.org/changeset/238625>
Comment 22 Ryan Haddad 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.